Re: [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request

[Andy Lutomirski]

> On Nov 7, 2018, at 3:21 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> 
>> On 11/07, Elvira Khabirova wrote:
>> 
>> In short, if a 64-bit task performs a syscall through int 0x80, its tracer
>> has no reliable means to find out that the syscall was, in fact,
>> a compat syscall, and misidentifies it.
>> * Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
> 
> Yes, this was discussed many times...
> 
> So perhaps it makes sense to encode compat/is_enter in ->ptrace_message,
> debugger can use PTRACE_GETEVENTMSG to get this info.

As I said before, I strongly object to the use of âcompatâ here.  Compat meant ânot the kernelâs native syscall API â uses the 32-bit structure format insteadâ.  This does not have a sensible meaning to user code, especially in the case where the tracer is 32-bit.

> 
>> Secondly, ptracers also have to support a lot of arch-specific code for
>> obtaining information about the tracee. For some architectures, this
>> requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
>> argument and return value.
> 
> I am not sure about this change... I won't really argue, but imo this
> needs a separate patch.

Why?  Having a single struct that the tracer can read to get the full state is extremely helpful.

Also, we really want it to work for seccomp events as well as PTRACE_SYSCALL, and the event info trick doesnât make sense for seccomp events.

> 
>> +#define PT_IN_SYSCALL_STOP    0x00000004    /* task is in a syscall-stop */
> ...
>> -static inline int ptrace_report_syscall(struct pt_regs *regs)
>> +static inline int ptrace_report_syscall(struct pt_regs *regs,
>> +                    unsigned long message)
>> {
>>    int ptrace = current->ptrace;
>> 
>>    if (!(ptrace & PT_PTRACED))
>>        return 0;
>> +    current->ptrace |= PT_IN_SYSCALL_STOP;
>> 
>> +    current->ptrace_message = message;
>>    ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
>> 
>>    /*
>> @@ -76,6 +79,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
>>        current->exit_code = 0;
>>    }
>> 
>> +    current->ptrace &= ~PT_IN_SYSCALL_STOP;
>>    return fatal_signal_pending(current);
> ...
> 
>> +    case PTRACE_GET_SYSCALL_INFO:
>> +        if (child->ptrace & PT_IN_SYSCALL_STOP)
>> +            ret = ptrace_get_syscall(child, datavp);
>> +        break;
> 
> Why? If debugger uses PTRACE_O_TRACESYSGOOD it can know if the tracee reported
> syscall entry/exit or not. PTRACE_GET_SYSCALL_INFO is pointless if not, but
> nothing bad can happen.
> 
> 

I think itâs considerably nicer to the user to avoid reporting garbage if the user misused the API.  (And Elvira got this right in the patch â I just missed it.)

>