Re: [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request

Andy Lutomirski
2018-11-07T14:06:49Z

> On Nov 7, 2018, at 3:21 AM, Oleg Nesterov <oleg@redhat.com> 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.)

>