* [PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request @ 2018-11-25 1:21 Elvira Khabirova 2018-11-25 1:22 ` [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message Elvira Khabirova ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Elvira Khabirova @ 2018-11-25 1:21 UTC (permalink / raw) To: oleg, rostedt, mingo Cc: linux-kernel, ldv, esyr, luto, strace-devel, linux-api Resent with linux-api@ Cc'ed. PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall the tracee is blocked in. The request succeeds when the tracee is in a syscall-enter-stop, syscall-exit-stop or PTRACE_EVENT_SECCOMP stop, and fails with -EINVAL otherwise. There are two reasons for a special syscall-related ptrace request. Firstly, with the current ptrace API there are cases when ptracer cannot retrieve necessary information about syscalls. Some examples include: * The notorious int-0x80-from-64-bit-task issue. See [1] for details. 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. Common practice is to keep track of the sequence of ptrace-stops in order not to mix the two syscall-stops up. But it is not as simple as it looks; for example, strace had a (just recently fixed) long-standing bug where attaching strace to a tracee that is performing the execve system call led to the tracer identifying the following syscall-exit-stop as syscall-enter-stop, which messed up all the state tracking. * Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3 ("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA and process_vm_readv become unavailable when the process dumpable flag is cleared. On such architectures as ia64 this results in all syscall arguments being unavailable. 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. PTRACE_GET_SYSCALL_INFO returns the following structure: struct ptrace_syscall_info { __u8 op; /* PTRACE_SYSCALL_INFO_* */ __u8 __pad0[3]; __u32 arch; union { struct { __u64 nr; __u64 instruction_pointer; __u64 stack_pointer; __u64 frame_pointer; __u64 args[6]; } entry; struct { __s64 rval; __u8 is_error; __u8 __pad1[7]; } exit; }; }; The structure was chosen according to [2], except for the following changes: * arch is returned unconditionally to aid with tracing system calls such as execve(); * the type of nr field was changed from int to __u64 because syscall numbers are, as a practical matter, 64 bits; * stack_pointer and frame_pointer fields were added along with instruction_pointer field since they are readily available and can save the tracer from extra PTRACE_GETREGSET calls; * a boolean is_error field was added along with rval field, this way the tracer can more reliably distinguish a return value from an error value. This changeset should be applied on top of [3] and [4]. [1] https://lore.kernel.org/lkml/CA+55aFzcSVmdDj9Lh_gdbz1OzHyEm6ZrGPBDAJnywm2LF_eVyg@mail.gmail.com/ [2] https://lore.kernel.org/lkml/CAObL_7GM0n80N7J_DFw_eQyfLyzq+sf4y2AvsCCV88Tb3AwEHA@mail.gmail.com/ [3] https://lore.kernel.org/lkml/20181119210139.GA8360@altlinux.org/ [4] https://lore.kernel.org/lkml/20181120001128.GA11300@altlinux.org/ v3: Split into three changes. Change struct ptrace_syscall_info. Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct. Add proper defines for ptrace_syscall_info.op values. Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT and move them to uapi. Elvira Khabirova (3): ptrace: pass type of a syscall-stop in ptrace_message ptrace: add PTRACE_GET_SYSCALL_INFO request ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO include/linux/ptrace.h | 1 + include/linux/sched.h | 1 + include/linux/tracehook.h | 10 ++++--- include/uapi/linux/ptrace.h | 34 ++++++++++++++++++++++++ kernel/ptrace.c | 53 +++++++++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 3 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message 2018-11-25 1:21 [PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request Elvira Khabirova @ 2018-11-25 1:22 ` Elvira Khabirova 2018-11-25 1:54 ` Joey Pabalinas 2018-11-26 14:56 ` Oleg Nesterov 2018-11-25 1:23 ` [PATCH RESEND v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request Elvira Khabirova 2018-11-25 1:23 ` [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO Elvira Khabirova 2 siblings, 2 replies; 18+ messages in thread From: Elvira Khabirova @ 2018-11-25 1:22 UTC (permalink / raw) To: oleg, rostedt, mingo Cc: linux-kernel, ldv, esyr, luto, strace-devel, linux-api Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message for the duration of syscall-stops. This way ptracers can distinguish syscall-enter-stops from syscall-exit-stops using PTRACE_GETEVENTMSG request. Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- include/linux/tracehook.h | 9 ++++++--- include/uapi/linux/ptrace.h | 10 ++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h index 40b0b4c1bf7b..633a83fe7051 100644 --- a/include/linux/tracehook.h +++ b/include/linux/tracehook.h @@ -57,13 +57,15 @@ struct linux_binprm; /* * ptrace report for syscall entry and exit looks identical. */ -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_message = message; ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0)); /* @@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs) current->exit_code = 0; } + current->ptrace_message = 0; return fatal_signal_pending(current); } @@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs) static inline __must_check int tracehook_report_syscall_entry( struct pt_regs *regs) { - return ptrace_report_syscall(regs); + return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY); } /** @@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct pt_regs *regs, int step) if (step) user_single_step_report(regs); else - ptrace_report_syscall(regs); + ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT); } /** diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h index d5a1b8a492b9..cb138902d042 100644 --- a/include/uapi/linux/ptrace.h +++ b/include/uapi/linux/ptrace.h @@ -104,6 +104,16 @@ struct seccomp_metadata { #define PTRACE_O_MASK (\ 0x000000ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP) +/* + * 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 + #include <asm/ptrace.h> -- 2.19.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message 2018-11-25 1:22 ` [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message Elvira Khabirova @ 2018-11-25 1:54 ` Joey Pabalinas 2018-11-25 2:10 ` Dmitry V. Levin 2018-11-26 14:56 ` Oleg Nesterov 1 sibling, 1 reply; 18+ messages in thread From: Joey Pabalinas @ 2018-11-25 1:54 UTC (permalink / raw) To: Elvira Khabirova Cc: oleg, rostedt, mingo, linux-api, esyr, linux-kernel, luto, ldv, strace-devel, Joey Pabalinas [-- Attachment #1: Type: text/plain, Size: 516 bytes --] On Sun, Nov 25, 2018 at 02:22:27AM +0100, Elvira Khabirova wrote: > Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and > PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message > for the duration of syscall-stops. > This way ptracers can distinguish syscall-enter-stops > from syscall-exit-stops using PTRACE_GETEVENTMSG request. Is there an advantage to using two constants instead of a single sys_exit bit (set/unset for syscall-exit-stop/syscall-enter-stop)? -- Cheers, Joey Pabalinas [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message 2018-11-25 1:54 ` Joey Pabalinas @ 2018-11-25 2:10 ` Dmitry V. Levin 2018-11-25 6:17 ` Joey Pabalinas 0 siblings, 1 reply; 18+ messages in thread From: Dmitry V. Levin @ 2018-11-25 2:10 UTC (permalink / raw) To: Joey Pabalinas Cc: Elvira Khabirova, Oleg Nesterov, Steven Rostedt, Ingo Molnar, Andy Lutomirski, Eugene Syromyatnikov, linux-api, linux-kernel, strace-devel [-- Attachment #1: Type: text/plain, Size: 1064 bytes --] On Sat, Nov 24, 2018 at 03:54:02PM -1000, Joey Pabalinas wrote: > On Sun, Nov 25, 2018 at 02:22:27AM +0100, Elvira Khabirova wrote: > > Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and > > PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message > > for the duration of syscall-stops. > > This way ptracers can distinguish syscall-enter-stops > > from syscall-exit-stops using PTRACE_GETEVENTMSG request. > > Is there an advantage to using two constants instead of a single > sys_exit bit (set/unset for syscall-exit-stop/syscall-enter-stop)? 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. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message 2018-11-25 2:10 ` Dmitry V. Levin @ 2018-11-25 6:17 ` Joey Pabalinas 0 siblings, 0 replies; 18+ messages in thread From: Joey Pabalinas @ 2018-11-25 6:17 UTC (permalink / raw) To: Dmitry V. Levin Cc: Joey Pabalinas, Elvira Khabirova, Oleg Nesterov, Steven Rostedt, Ingo Molnar, Andy Lutomirski, Eugene Syromyatnikov, linux-api, linux-kernel, strace-devel [-- Attachment #1: Type: text/plain, Size: 644 bytes --] On Sun, Nov 25, 2018 at 05:10:59AM +0300, Dmitry V. Levin wrote: > 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. Ah, I see, I agree that definitely makes sense. -- Cheers, Joey Pabalinas [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message 2018-11-25 1:22 ` [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message Elvira Khabirova 2018-11-25 1:54 ` Joey Pabalinas @ 2018-11-26 14:56 ` Oleg Nesterov 2018-11-27 2:53 ` Elvira Khabirova 1 sibling, 1 reply; 18+ messages in thread From: Oleg Nesterov @ 2018-11-26 14:56 UTC (permalink / raw) To: Elvira Khabirova Cc: rostedt, mingo, linux-kernel, ldv, esyr, luto, strace-devel, linux-api On 11/25, Elvira Khabirova wrote: > > + * 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 Stupid question, why not #define PTRACE_EVENT_SYSCALL_ENTRY 8 #define PTRACE_EVENT_SYSCALL_EXIT 9 right after other PTRACE_EVENT_* constants? The next 3/3 tries to detect PTRACE_EVENT_SECCOMP case anyway, so I am not sure I understand what "do not appear in task->ptrace_message by other means" actually means... Oleg. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message 2018-11-26 14:56 ` Oleg Nesterov @ 2018-11-27 2:53 ` Elvira Khabirova 2018-11-27 3:48 ` Dmitry V. Levin 0 siblings, 1 reply; 18+ messages in thread From: Elvira Khabirova @ 2018-11-27 2:53 UTC (permalink / raw) To: Oleg Nesterov Cc: rostedt, mingo, linux-kernel, ldv, esyr, luto, strace-devel, linux-api On Mon, 26 Nov 2018 15:56:43 +0100 Oleg Nesterov <oleg@redhat.com> wrote: > On 11/25, Elvira Khabirova wrote: > > > > + * 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 > > Stupid question, why not > > #define PTRACE_EVENT_SYSCALL_ENTRY 8 > #define PTRACE_EVENT_SYSCALL_EXIT 9 > > right after other PTRACE_EVENT_* constants? I thought about adding new events for syscall {entry,exit}. For tracers, using new events means setting new options and checking for new values after waitpid(). They will also have to switch from using PTRACE_SYSCALL to PTRACE_CONT. Right now (with this version of the patch) tracers can use PTRACE_GETEVENTMSG without doing any additional configuration. More importantly, adding these events would require much more complex modifications of kernel code than this patch does. The only benefit I see from adding these events instead of letting syscall-stops put a value in ptrace_message is an ability to subscribe to syscall entries, but not to exits, and vice-versa, and I don't think it is worth it. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message 2018-11-27 2:53 ` Elvira Khabirova @ 2018-11-27 3:48 ` Dmitry V. Levin 0 siblings, 0 replies; 18+ messages in thread From: Dmitry V. Levin @ 2018-11-27 3:48 UTC (permalink / raw) To: Elvira Khabirova, Oleg Nesterov Cc: Steven Rostedt, Ingo Molnar, Andy Lutomirski, Eugene Syromyatnikov, linux-kernel, strace-devel, linux-api [-- Attachment #1: Type: text/plain, Size: 1667 bytes --] On Tue, Nov 27, 2018 at 03:53:57AM +0100, Elvira Khabirova wrote: > On Mon, 26 Nov 2018 15:56:43 +0100, Oleg Nesterov wrote: > > On 11/25, Elvira Khabirova wrote: > > > > > > + * 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 > > > > Stupid question, why not > > > > #define PTRACE_EVENT_SYSCALL_ENTRY 8 > > #define PTRACE_EVENT_SYSCALL_EXIT 9 > > > > right after other PTRACE_EVENT_* constants? > > I thought about adding new events for syscall {entry,exit}. > For tracers, using new events means setting new options and checking > for new values after waitpid(). They will also have to switch from using > PTRACE_SYSCALL to PTRACE_CONT. > Right now (with this version of the patch) tracers can use > PTRACE_GETEVENTMSG without doing any additional configuration. > More importantly, adding these events would require much more complex > modifications of kernel code than this patch does. To be honest, we don't see any way of introducing PTRACE_EVENT_* support in PTRACE_GET_SYSCALL_INFO without adding new fields to struct task_struct. > The only benefit I see from adding these events instead of letting > syscall-stops put a value in ptrace_message is an ability to subscribe > to syscall entries, but not to exits, and vice-versa, and I don't think > it is worth it. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RESEND v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request 2018-11-25 1:21 [PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request Elvira Khabirova 2018-11-25 1:22 ` [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message Elvira Khabirova @ 2018-11-25 1:23 ` Elvira Khabirova 2018-11-25 1:23 ` [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO Elvira Khabirova 2 siblings, 0 replies; 18+ messages in thread From: Elvira Khabirova @ 2018-11-25 1:23 UTC (permalink / raw) To: oleg, rostedt, mingo Cc: linux-kernel, ldv, esyr, luto, strace-devel, linux-api PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall the tracee is blocked in. The request succeeds when the tracee is in a syscall-enter-stop or syscall-exit-stop, and fails with -EINVAL otherwise. A subsequent change may extend PTRACE_GET_SYSCALL_INFO for the case of PTRACE_EVENT_SECCOMP stop. There are two reasons for a special syscall-related ptrace request. Firstly, with the current ptrace API there are cases when ptracer cannot retrieve necessary information about syscalls. Some examples include: * The notorious int-0x80-from-64-bit-task issue. See [1] for details. 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. Common practice is to keep track of the sequence of ptrace-stops in order not to mix the two syscall-stops up. But it is not as simple as it looks; for example, strace had a (just recently fixed) long-standing bug where attaching strace to a tracee that is performing the execve system call led to the tracer identifying the following syscall-exit-stop as syscall-enter-stop, which messed up all the state tracking. * Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3 ("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA and process_vm_readv become unavailable when the process dumpable flag is cleared. On such architectures as ia64 this results in all syscall arguments being unavailable. 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. ptrace(2) man page: long ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data); ... PTRACE_GET_SYSCALL_INFO Retrieve information about the syscall that caused the stop. The information is placed into the buffer pointed by "data" argument, which should be a pointer to a buffer of type "struct ptrace_syscall_info". The "addr" argument contains the size of the buffer pointed to by "data" (i.e., sizeof(struct ptrace_syscall_info)). The return value contains the number of bytes available to be written by the kernel. If the size of data to be written by the kernel exceeds the size specified by "addr" argument, the output is truncated. This operation fails with EINVAL if the tracee is not in a syscall-enter-stop, a syscall-exit-stop, or a PTRACE_EVENT_SECCOMP stop. Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- include/uapi/linux/ptrace.h | 24 ++++++++++++++++++ kernel/ptrace.c | 50 +++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h index cb138902d042..49b0b1b41943 100644 --- a/include/uapi/linux/ptrace.h +++ b/include/uapi/linux/ptrace.h @@ -73,6 +73,30 @@ struct seccomp_metadata { __u64 flags; /* Output: filter's flags */ }; +#define PTRACE_GET_SYSCALL_INFO 0x420f +#define PTRACE_SYSCALL_INFO_ENTRY 0 +#define PTRACE_SYSCALL_INFO_EXIT 1 + +struct ptrace_syscall_info { + __u8 op; /* PTRACE_SYSCALL_INFO_* */ + __u8 __pad0[3]; + __u32 arch; + union { + struct { + __u64 nr; + __u64 instruction_pointer; + __u64 stack_pointer; + __u64 frame_pointer; + __u64 args[6]; + } entry; + struct { + __s64 rval; + __u8 is_error; + __u8 __pad1[7]; + } exit; + }; +}; + /* Read signals from a shared (process wide) queue */ #define PTRACE_PEEKSIGINFO_SHARED (1 << 0) diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 80b34dffdfb9..92c47cd5ad84 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -30,6 +30,10 @@ #include <linux/cn_proc.h> #include <linux/compat.h> +#ifdef CONFIG_HAVE_ARCH_TRACEHOOK +#include <asm/syscall.h> /* For syscall_get_* */ +#endif + /* * Access another process' address space via ptrace. * Source/target buffer must be kernel space, @@ -890,6 +894,46 @@ static int ptrace_regset(struct task_struct *task, int req, unsigned int type, EXPORT_SYMBOL_GPL(task_user_regset_view); #endif +#ifdef CONFIG_HAVE_ARCH_TRACEHOOK +static int ptrace_get_syscall(struct task_struct *child, + unsigned long user_size, void __user *datavp) +{ + struct ptrace_syscall_info info; + struct pt_regs *regs = task_pt_regs(child); + unsigned long args[ARRAY_SIZE(info.entry.args)]; + unsigned long actual_size; + unsigned long write_size; + + if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) { + int i; + + info.op = PTRACE_SYSCALL_INFO_ENTRY; + info.arch = syscall_get_arch(child); + info.entry.nr = syscall_get_nr(child, regs); + info.entry.instruction_pointer = instruction_pointer(regs); + info.entry.stack_pointer = user_stack_pointer(regs); + info.entry.frame_pointer = frame_pointer(regs); + syscall_get_arguments(child, regs, 0, ARRAY_SIZE(args), args); + for (i = 0; i < ARRAY_SIZE(args); i++) + info.entry.args[i] = args[i]; + actual_size = offsetofend(struct ptrace_syscall_info, entry); + } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) { + info.op = PTRACE_SYSCALL_INFO_EXIT; + info.arch = syscall_get_arch(child); + info.exit.rval = syscall_get_error(child, regs); + info.exit.is_error = !!info.exit.rval; + if (!info.exit.is_error) + info.exit.rval = syscall_get_return_value(child, regs); + actual_size = offsetofend(struct ptrace_syscall_info, exit); + } else { + return -EINVAL; + } + + write_size = min(actual_size, user_size); + return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size; +} +#endif + int ptrace_request(struct task_struct *child, long request, unsigned long addr, unsigned long data) { @@ -1105,6 +1149,12 @@ int ptrace_request(struct task_struct *child, long request, ret = seccomp_get_metadata(child, addr, datavp); break; +#ifdef CONFIG_HAVE_ARCH_TRACEHOOK + case PTRACE_GET_SYSCALL_INFO: + ret = ptrace_get_syscall(child, addr, datavp); + break; +#endif + default: break; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-25 1:21 [PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request Elvira Khabirova 2018-11-25 1:22 ` [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message Elvira Khabirova 2018-11-25 1:23 ` [PATCH RESEND v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request Elvira Khabirova @ 2018-11-25 1:23 ` Elvira Khabirova 2018-11-26 14:35 ` Oleg Nesterov 2 siblings, 1 reply; 18+ messages in thread From: Elvira Khabirova @ 2018-11-25 1:23 UTC (permalink / raw) To: oleg, rostedt, mingo Cc: linux-kernel, ldv, esyr, luto, strace-devel, linux-api Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops. The information returned is the same as for syscall-enter-stops. Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- include/linux/ptrace.h | 1 + include/linux/sched.h | 1 + include/linux/tracehook.h | 1 + kernel/ptrace.c | 7 +++++-- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index 6c2ffed907f5..a993d0fde865 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long message) { if (unlikely(ptrace_event_enabled(current, event))) { current->ptrace_message = message; + current->ptrace_event = event; ptrace_notify((event << 8) | SIGTRAP); } else if (event == PTRACE_EVENT_EXEC) { /* legacy EXEC report via SIGTRAP */ diff --git a/include/linux/sched.h b/include/linux/sched.h index a51c13c2b1a0..86215fb654d6 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -964,6 +964,7 @@ struct task_struct { /* Ptrace state: */ unsigned long ptrace_message; + int ptrace_event; kernel_siginfo_t *last_siginfo; struct task_io_accounting ioac; diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h index 633a83fe7051..5d2e5aa07a5c 100644 --- a/include/linux/tracehook.h +++ b/include/linux/tracehook.h @@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs, return 0; current->ptrace_message = message; + current->ptrace_event = 0; ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0)); /* diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 92c47cd5ad84..74a37e74c7f1 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child, unsigned long actual_size; unsigned long write_size; - if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) { + if ((child->ptrace_event == 0 && + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) || + child->ptrace_event == PTRACE_EVENT_SECCOMP) { int i; info.op = PTRACE_SYSCALL_INFO_ENTRY; @@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child, for (i = 0; i < ARRAY_SIZE(args); i++) info.entry.args[i] = args[i]; actual_size = offsetofend(struct ptrace_syscall_info, entry); - } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) { + } else if (child->ptrace_event == 0 && + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) { info.op = PTRACE_SYSCALL_INFO_EXIT; info.arch = syscall_get_arch(child); info.exit.rval = syscall_get_error(child, regs); -- 2.19.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-25 1:23 ` [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO Elvira Khabirova @ 2018-11-26 14:35 ` Oleg Nesterov 2018-11-27 3:07 ` Elvira Khabirova 0 siblings, 1 reply; 18+ messages in thread From: Oleg Nesterov @ 2018-11-26 14:35 UTC (permalink / raw) To: Elvira Khabirova Cc: rostedt, mingo, linux-kernel, ldv, esyr, luto, strace-devel, linux-api On 11/25, Elvira Khabirova wrote: > > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops. > The information returned is the same as for syscall-enter-stops. Oh, this is not nice ;) there must be a better option, I hope... Plus Can't ptrace_get_syscall() check child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP; to detect the PTRACE_EVENT_SECCOMP case? Oleg. > Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> > --- > include/linux/ptrace.h | 1 + > include/linux/sched.h | 1 + > include/linux/tracehook.h | 1 + > kernel/ptrace.c | 7 +++++-- > 4 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h > index 6c2ffed907f5..a993d0fde865 100644 > --- a/include/linux/ptrace.h > +++ b/include/linux/ptrace.h > @@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long message) > { > if (unlikely(ptrace_event_enabled(current, event))) { > current->ptrace_message = message; > + current->ptrace_event = event; > ptrace_notify((event << 8) | SIGTRAP); > } else if (event == PTRACE_EVENT_EXEC) { > /* legacy EXEC report via SIGTRAP */ > diff --git a/include/linux/sched.h b/include/linux/sched.h > index a51c13c2b1a0..86215fb654d6 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -964,6 +964,7 @@ struct task_struct { > > /* Ptrace state: */ > unsigned long ptrace_message; > + int ptrace_event; > kernel_siginfo_t *last_siginfo; > > struct task_io_accounting ioac; > diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h > index 633a83fe7051..5d2e5aa07a5c 100644 > --- a/include/linux/tracehook.h > +++ b/include/linux/tracehook.h > @@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs, > return 0; > > current->ptrace_message = message; > + current->ptrace_event = 0; > ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0)); > > /* > diff --git a/kernel/ptrace.c b/kernel/ptrace.c > index 92c47cd5ad84..74a37e74c7f1 100644 > --- a/kernel/ptrace.c > +++ b/kernel/ptrace.c > @@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child, > unsigned long actual_size; > unsigned long write_size; > > - if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) { > + if ((child->ptrace_event == 0 && > + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) || > + child->ptrace_event == PTRACE_EVENT_SECCOMP) { > int i; > > info.op = PTRACE_SYSCALL_INFO_ENTRY; > @@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child, > for (i = 0; i < ARRAY_SIZE(args); i++) > info.entry.args[i] = args[i]; > actual_size = offsetofend(struct ptrace_syscall_info, entry); > - } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) { > + } else if (child->ptrace_event == 0 && > + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) { > info.op = PTRACE_SYSCALL_INFO_EXIT; > info.arch = syscall_get_arch(child); > info.exit.rval = syscall_get_error(child, regs); > -- > 2.19.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-26 14:35 ` Oleg Nesterov @ 2018-11-27 3:07 ` Elvira Khabirova 2018-11-27 3:21 ` Dmitry V. Levin 2018-11-27 12:31 ` Oleg Nesterov 0 siblings, 2 replies; 18+ messages in thread From: Elvira Khabirova @ 2018-11-27 3:07 UTC (permalink / raw) To: Oleg Nesterov Cc: rostedt, mingo, linux-kernel, ldv, esyr, luto, strace-devel, linux-api On Mon, 26 Nov 2018 15:35:24 +0100 Oleg Nesterov <oleg@redhat.com> wrote: > On 11/25, Elvira Khabirova wrote: > > > > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops. > > The information returned is the same as for syscall-enter-stops. > > Oh, this is not nice ;) there must be a better option, I hope... Plus > > > Can't ptrace_get_syscall() check > > child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP; > > to detect the PTRACE_EVENT_SECCOMP case? Nope; looks like exit_code is zeroed after wait(). > Oleg. > > > Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> > > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> > > --- > > include/linux/ptrace.h | 1 + > > include/linux/sched.h | 1 + > > include/linux/tracehook.h | 1 + > > kernel/ptrace.c | 7 +++++-- > > 4 files changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h > > index 6c2ffed907f5..a993d0fde865 100644 > > --- a/include/linux/ptrace.h > > +++ b/include/linux/ptrace.h > > @@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long message) > > { > > if (unlikely(ptrace_event_enabled(current, event))) { > > current->ptrace_message = message; > > + current->ptrace_event = event; > > ptrace_notify((event << 8) | SIGTRAP); > > } else if (event == PTRACE_EVENT_EXEC) { > > /* legacy EXEC report via SIGTRAP */ > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > index a51c13c2b1a0..86215fb654d6 100644 > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -964,6 +964,7 @@ struct task_struct { > > > > /* Ptrace state: */ > > unsigned long ptrace_message; > > + int ptrace_event; > > kernel_siginfo_t *last_siginfo; > > > > struct task_io_accounting ioac; > > diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h > > index 633a83fe7051..5d2e5aa07a5c 100644 > > --- a/include/linux/tracehook.h > > +++ b/include/linux/tracehook.h > > @@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs, > > return 0; > > > > current->ptrace_message = message; > > + current->ptrace_event = 0; > > ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0)); > > > > /* > > diff --git a/kernel/ptrace.c b/kernel/ptrace.c > > index 92c47cd5ad84..74a37e74c7f1 100644 > > --- a/kernel/ptrace.c > > +++ b/kernel/ptrace.c > > @@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child, > > unsigned long actual_size; > > unsigned long write_size; > > > > - if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) { > > + if ((child->ptrace_event == 0 && > > + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) || > > + child->ptrace_event == PTRACE_EVENT_SECCOMP) { > > int i; > > > > info.op = PTRACE_SYSCALL_INFO_ENTRY; > > @@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child, > > for (i = 0; i < ARRAY_SIZE(args); i++) > > info.entry.args[i] = args[i]; > > actual_size = offsetofend(struct ptrace_syscall_info, entry); > > - } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) { > > + } else if (child->ptrace_event == 0 && > > + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) { > > info.op = PTRACE_SYSCALL_INFO_EXIT; > > info.arch = syscall_get_arch(child); > > info.exit.rval = syscall_get_error(child, regs); > > -- > > 2.19.1 > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-27 3:07 ` Elvira Khabirova @ 2018-11-27 3:21 ` Dmitry V. Levin 2018-11-27 12:31 ` Oleg Nesterov 1 sibling, 0 replies; 18+ messages in thread From: Dmitry V. Levin @ 2018-11-27 3:21 UTC (permalink / raw) To: Elvira Khabirova, Oleg Nesterov Cc: Steven Rostedt, Ingo Molnar, Andy Lutomirski, Eugene Syromyatnikov, linux-kernel, strace-devel, linux-api [-- Attachment #1: Type: text/plain, Size: 839 bytes --] On Tue, Nov 27, 2018 at 04:07:32AM +0100, Elvira Khabirova wrote: > On Mon, 26 Nov 2018 15:35:24 +0100, Oleg Nesterov wrote: > > On 11/25, Elvira Khabirova wrote: > > > > > > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops. > > > The information returned is the same as for syscall-enter-stops. > > > > Oh, this is not nice ;) there must be a better option, I hope... Plus > > > > Can't ptrace_get_syscall() check > > > > child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP; > > > > to detect the PTRACE_EVENT_SECCOMP case? > > Nope; looks like exit_code is zeroed after wait(). It's explicitly reset to zero in wait_task_stopped() unless WNOWAIT wait option is set. When strace requests PTRACE_GET_SYSCALL_INFO after wait4(), child->exit_code is already set to zero. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-27 3:07 ` Elvira Khabirova 2018-11-27 3:21 ` Dmitry V. Levin @ 2018-11-27 12:31 ` Oleg Nesterov 2018-11-27 23:27 ` Dmitry V. Levin 1 sibling, 1 reply; 18+ messages in thread From: Oleg Nesterov @ 2018-11-27 12:31 UTC (permalink / raw) To: Elvira Khabirova Cc: rostedt, mingo, linux-kernel, ldv, esyr, luto, strace-devel, linux-api On 11/27, Elvira Khabirova wrote: > > On Mon, 26 Nov 2018 15:35:24 +0100 > Oleg Nesterov <oleg@redhat.com> wrote: > > > On 11/25, Elvira Khabirova wrote: > > > > > > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops. > > > The information returned is the same as for syscall-enter-stops. > > > > Oh, this is not nice ;) there must be a better option, I hope... Plus > > > > > > Can't ptrace_get_syscall() check > > > > child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP; > > > > to detect the PTRACE_EVENT_SECCOMP case? > > Nope; looks like exit_code is zeroed after wait(). Yes, thanks for correcting me, but we can use child->last_siginfo->si_code. Just like ptrace_request(PTRACE_LISTEN) does but you can do this lockless (no need to lock_task_sighand()). And if we require that the user of ptrace_get_syscall() should also use TRACESYSGOOD then ptrace_get_syscall() can probably do something like int entry; if (!child->last_siginfo) return -EINVAL; else if (child->last_siginfo->si_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP) entry = 1; else if (child->last_siginfo->si_code == SIGTRAP | 0x80) entry = child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY; else return -EINVAL; and this way PTRACE_EVENTMSG_SYSCALL_ENTRY/EXIT can't confict with seccomp or anything else. No? Of course, debugger can do PTRACE_SETSIGINFO and confuse itself but probably we do not care? Oleg. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-27 12:31 ` Oleg Nesterov @ 2018-11-27 23:27 ` Dmitry V. Levin 2018-11-28 12:35 ` Oleg Nesterov 0 siblings, 1 reply; 18+ messages in thread From: Dmitry V. Levin @ 2018-11-27 23:27 UTC (permalink / raw) To: Oleg Nesterov Cc: Elvira Khabirova, Steven Rostedt, Ingo Molnar, Eugene Syromyatnikov, Andy Lutomirski, strace-devel, linux-api, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1969 bytes --] On Tue, Nov 27, 2018 at 01:31:17PM +0100, Oleg Nesterov wrote: > On 11/27, Elvira Khabirova wrote: > > On Mon, 26 Nov 2018 15:35:24 +0100, Oleg Nesterov wrote: > > > On 11/25, Elvira Khabirova wrote: > > > > > > > > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops. > > > > The information returned is the same as for syscall-enter-stops. > > > > > > Oh, this is not nice ;) there must be a better option, I hope... Plus > > > > > > > > > Can't ptrace_get_syscall() check > > > > > > child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP; > > > > > > to detect the PTRACE_EVENT_SECCOMP case? > > > > Nope; looks like exit_code is zeroed after wait(). > > Yes, thanks for correcting me, > > but we can use child->last_siginfo->si_code. Yes, this approach works, thanks! > Just like ptrace_request(PTRACE_LISTEN) > does but you can do this lockless (no need to lock_task_sighand()). Why this can be done lockless? All other places in that file do the locking, so I'd rather add a comment in the new code. > And if we require that the user of ptrace_get_syscall() should also use TRACESYSGOOD > then ptrace_get_syscall() can probably do something like > > int entry; > > if (!child->last_siginfo) > return -EINVAL; > else if (child->last_siginfo->si_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP) > entry = 1; > else if (child->last_siginfo->si_code == SIGTRAP | 0x80) > entry = child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY; > else > return -EINVAL; > > and this way PTRACE_EVENTMSG_SYSCALL_ENTRY/EXIT can't confict with seccomp or > anything else. > > No? > > Of course, debugger can do PTRACE_SETSIGINFO and confuse itself but probably we > do not care? The only potential issue I could think of is whether PTRACE_SETSIGINFO could be used this way to cause an information leak by making PTRACE_GET_SYSCALL_INFO access some unrelated data. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-27 23:27 ` Dmitry V. Levin @ 2018-11-28 12:35 ` Oleg Nesterov 2018-11-28 12:44 ` Dmitry V. Levin 0 siblings, 1 reply; 18+ messages in thread From: Oleg Nesterov @ 2018-11-28 12:35 UTC (permalink / raw) To: Dmitry V. Levin Cc: Elvira Khabirova, Steven Rostedt, Ingo Molnar, Eugene Syromyatnikov, Andy Lutomirski, strace-devel, linux-api, linux-kernel On 11/28, Dmitry V. Levin wrote: > > > Just like ptrace_request(PTRACE_LISTEN) > > does but you can do this lockless (no need to lock_task_sighand()). > > Why this can be done lockless? All other places in that file do > the locking, PTRACE_LISTEN too doesn't need lock_task_sighand() to access ->last_siginfo, this code predates ptrace_freeze_traced() which ensures that the tracee can't go away and clear ->last_siginfo. However, unlike ptrace_get_syscall(), PTRACE_LISTEN needs spin_lock_irq(siglock), it modifies ->jobctl and calls signal_wake_up(). > > Of course, debugger can do PTRACE_SETSIGINFO and confuse itself but probably we > > do not care? > > The only potential issue I could think of is whether PTRACE_SETSIGINFO > could be used this way to cause an information leak by making > PTRACE_GET_SYSCALL_INFO access some unrelated data. Well, afaics ptrace_get_syscall() does nothing "special", debugger can use other PTRACE_ requests to get the same info? Oleg. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-28 12:35 ` Oleg Nesterov @ 2018-11-28 12:44 ` Dmitry V. Levin 2018-11-28 13:13 ` Oleg Nesterov 0 siblings, 1 reply; 18+ messages in thread From: Dmitry V. Levin @ 2018-11-28 12:44 UTC (permalink / raw) To: Oleg Nesterov Cc: Elvira Khabirova, Eugene Syromyatnikov, Steven Rostedt, Andy Lutomirski, Ingo Molnar, linux-api, linux-kernel, strace-devel [-- Attachment #1: Type: text/plain, Size: 1270 bytes --] On Wed, Nov 28, 2018 at 01:35:46PM +0100, Oleg Nesterov wrote: > On 11/28, Dmitry V. Levin wrote: > > > > > Just like ptrace_request(PTRACE_LISTEN) > > > does but you can do this lockless (no need to lock_task_sighand()). > > > > Why this can be done lockless? All other places in that file do > > the locking, > > PTRACE_LISTEN too doesn't need lock_task_sighand() to access ->last_siginfo, > this code predates ptrace_freeze_traced() which ensures that the tracee can't > go away and clear ->last_siginfo. > > However, unlike ptrace_get_syscall(), PTRACE_LISTEN needs spin_lock_irq(siglock), > it modifies ->jobctl and calls signal_wake_up(). What about PTRACE_GETSIGINFO? Can it also be done lockless because ptrace_check_attach() has already called ptrace_freeze_traced()? > > > Of course, debugger can do PTRACE_SETSIGINFO and confuse itself but probably we > > > do not care? > > > > The only potential issue I could think of is whether PTRACE_SETSIGINFO > > could be used this way to cause an information leak by making > > PTRACE_GET_SYSCALL_INFO access some unrelated data. > > Well, afaics ptrace_get_syscall() does nothing "special", debugger can use other > PTRACE_ requests to get the same info? I agree. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO 2018-11-28 12:44 ` Dmitry V. Levin @ 2018-11-28 13:13 ` Oleg Nesterov 0 siblings, 0 replies; 18+ messages in thread From: Oleg Nesterov @ 2018-11-28 13:13 UTC (permalink / raw) To: Dmitry V. Levin Cc: Elvira Khabirova, Eugene Syromyatnikov, Steven Rostedt, Andy Lutomirski, Ingo Molnar, linux-api, linux-kernel, strace-devel On 11/28, Dmitry V. Levin wrote: > > What about PTRACE_GETSIGINFO? Can it also be done lockless because > ptrace_check_attach() has already called ptrace_freeze_traced()? Yes, only PTRACE_INTERRUPT needs lock_task_sighand() and it can actually fail because the tracee is not necessarily stopped. Oleg. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-11-28 13:13 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-25 1:21 [PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request Elvira Khabirova 2018-11-25 1:22 ` [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message Elvira Khabirova 2018-11-25 1:54 ` Joey Pabalinas 2018-11-25 2:10 ` Dmitry V. Levin 2018-11-25 6:17 ` Joey Pabalinas 2018-11-26 14:56 ` Oleg Nesterov 2018-11-27 2:53 ` Elvira Khabirova 2018-11-27 3:48 ` Dmitry V. Levin 2018-11-25 1:23 ` [PATCH RESEND v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request Elvira Khabirova 2018-11-25 1:23 ` [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO Elvira Khabirova 2018-11-26 14:35 ` Oleg Nesterov 2018-11-27 3:07 ` Elvira Khabirova 2018-11-27 3:21 ` Dmitry V. Levin 2018-11-27 12:31 ` Oleg Nesterov 2018-11-27 23:27 ` Dmitry V. Levin 2018-11-28 12:35 ` Oleg Nesterov 2018-11-28 12:44 ` Dmitry V. Levin 2018-11-28 13:13 ` Oleg Nesterov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).