Re: [PATCH v4 1/2] ptrace: save the type of syscall-stop in ptrace_message
From: Andy Lutomirski <luto@kernel.org>
Date: 2018-11-28 23:18:13
Also in:
lkml
On Wed, Nov 28, 2018 at 2:11 PM Dmitry V. Levin [off-list ref] wrote:
On Wed, Nov 28, 2018 at 06:23:46PM +0300, Dmitry V. Levin wrote:quoted
On Wed, Nov 28, 2018 at 03:20:06PM +0100, Oleg Nesterov wrote:quoted
On 11/28, Dmitry V. Levin wrote:quoted
On Wed, Nov 28, 2018 at 02:49:14PM +0100, Oleg Nesterov wrote:quoted
On 11/28, Dmitry V. Levin wrote:quoted
+/* + * These values are stored in task->ptrace_message by tracehook_report_syscall_* + * to describe current syscall-stop. + * + * Values for these constants are chosen so that they do not appear + * in task->ptrace_message by other means. + */ +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 0x80000000U +#define PTRACE_EVENTMSG_SYSCALL_EXIT 0x90000000UAgain, I do not really understand the comment... Why should we care about "do not appear in task->ptrace_message by other means" ? 2/2 should detect ptrace_report_syscall() case correctly, so we can use any numbers, say, 1 and 2? If debugger does PTRACE_GETEVENTMSG it should know how to interpet the value anyway after wait(status).Given that without this patch the value returned by PTRACE_GETEVENTMSG during syscall stop is undefined, we need two different ptrace_message values that cannot be set by other ptrace events to enable reliable identification of syscall-enter-stop and syscall-exit-stop in userspace: if we make PTRACE_GETEVENTMSG return 0 or any other value routinely set by other ptrace events, it would be hard for userspace to find out whether the kernel implements new semantics or not.Hmm, why? Debugger can just do ptrace(PTRACE_GET_SYSCALL_INFO, NULL), if it returns EIO then it is not implemented?The debugger that uses PTRACE_GET_SYSCALL_INFO does not need to call PTRACE_GETEVENTMSG for syscall stops. My concern here is the PTRACE_GETEVENTMSG interface itself. If we use ptrace_message to implement PTRACE_GET_SYSCALL_INFO and expose PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} for regular PTRACE_GETEVENTMSG users, it should have clear semantics.Since our implementation of PTRACE_GET_SYSCALL_INFO uses ptrace_message to distinguish syscall-enter-stop from syscall-exit-stop, we could choose one of the following approaches: 1. Do not document the values saved into ptrace_message during syscall stops (and exposed via PTRACE_GETEVENTMSG) as a part of ptrace API, leaving the value returned by PTRACE_GETEVENTMSG during syscall stops as undefined. 2. Document these values chosen to avoid collisions with ptrace_message values set by other ptrace events so that PTRACE_GETEVENTMSG users can easily tell whether this new semantics is supported by the kernel or not.
I don't like any of this at all. Can we please choose a sensible API design and let the API drive the implementation instead of vice versa? ISTM the correct solution is to add some new state to task_struct for this. If we're concerned about making task_struct bigger, I have a half-finished patch to factor all the ptrace tracee state into a separate struct.