Messages in this thread | | | Subject | Re: [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request | From | Andy Lutomirski <> | Date | Wed, 7 Nov 2018 06:06:43 -0800 |
| |
> 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.)
>
| |