Thread (17 messages) 17 messages, 3 authors, 2018-11-30

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 0x90000000U
Again, 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help