* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request [not found] <20181121165806.07da7c98@akathisia> @ 2018-11-21 22:56 ` Andy Lutomirski [not found] ` <CALCETrV3TPCkYyhoLLcikXVeF-RdZUuLTCvKReK3Qb9LSS9Csw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Andy Lutomirski @ 2018-11-21 22:56 UTC (permalink / raw) To: Elvira Khabirova, Kees Cook, Sasha Levin, Linux API, Jann Horn Cc: Oleg Nesterov, Steven Rostedt, Ingo Molnar, LKML, Dmitry V. Levin, Eugene Syromiatnikov, Andrew Lutomirski, strace-devel Please cc linux-api@vger.kernel.org for future versions. On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova <lineprinter@altlinux.org> wrote: > > struct ptrace_syscall_info { > __u8 op; /* 0 for entry, 1 for exit */ Can you add proper defines, like: #define PTRACE_SYSCALL_ENTRY 0 #define PTRACE_SYSCALL_EXIT 1 #define PTRACE_SYSCALL_SECCOMP 2 and make seccomp work from the start? I'd rather we don't merge an implementation that doesn't work for seccomp and then have to rework it later. > __u8 __pad0[7]; > union { > struct { > __s32 nr; __u64 please. Syscall numbers are, as a practical matter, 64 bits. Admittedly, the actual effects of setting the high bits are unclear, and seccomp has issues with it, but let's not perpetuate the problem. > __u32 arch; > __u64 instruction_pointer; > __u64 args[6]; > } entry_info; > struct { > __s64 rval; > __u8 is_error; > __u8 __pad1[7]; > } exit_info; > }; > }; Should seccomp events use entry_info or should they just literally supply seccomp_data? ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CALCETrV3TPCkYyhoLLcikXVeF-RdZUuLTCvKReK3Qb9LSS9Csw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request [not found] ` <CALCETrV3TPCkYyhoLLcikXVeF-RdZUuLTCvKReK3Qb9LSS9Csw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-11-21 23:56 ` Dmitry V. Levin 2018-11-22 14:55 ` Andy Lutomirski 0 siblings, 1 reply; 8+ messages in thread From: Dmitry V. Levin @ 2018-11-21 23:56 UTC (permalink / raw) To: Andy Lutomirski Cc: Eugene Syromiatnikov, Kees Cook, Jann Horn, Linux API, Oleg Nesterov, Steven Rostedt, LKML, Sasha Levin, Ingo Molnar, strace-devel-3+4lAyCyj6AWlMsSdNXQLw [-- Attachment #1.1: Type: text/plain, Size: 2194 bytes --] On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote: > Please cc linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for future versions. > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote: > > > > struct ptrace_syscall_info { > > __u8 op; /* 0 for entry, 1 for exit */ > > Can you add proper defines, like: > > #define PTRACE_SYSCALL_ENTRY 0 > #define PTRACE_SYSCALL_EXIT 1 > #define PTRACE_SYSCALL_SECCOMP 2 > > and make seccomp work from the start? I'd rather we don't merge an > implementation that doesn't work for seccomp and then have to rework > it later. What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the same entry_info to return. As long as implementation (ab)uses ptrace_message to tell one kind of stop from another, it can distinguish syscall-entry-stop and syscall-exit-stop from each other and from many other kinds of stops, but it cannot distinguish PTRACE_EVENT_SECCOMP from e.g. PTRACE_EVENT_EXIT. > > __u8 __pad0[7]; > > union { > > struct { > > __s32 nr; > > __u64 please. Syscall numbers are, as a practical matter, 64 bits. > Admittedly, the actual effects of setting the high bits are unclear, > and seccomp has issues with it, but let's not perpetuate the problem. I agree. Although the implementation uses syscall_get_nr() which returns int, this could potentially be fixed in the future. > > __u32 arch; > > __u64 instruction_pointer; > > __u64 args[6]; > > } entry_info; > > struct { > > __s64 rval; > > __u8 is_error; > > __u8 __pad1[7]; > > } exit_info; > > }; > > }; > > Should seccomp events use entry_info or should they just literally > supply seccomp_data? It certainly can use entry_info. I'd prefer to avoid using in uapi/linux/ptrace.h those types that are defined in uapi/linux/seccomp.h. -- ldv [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] [-- Attachment #2: Type: text/plain, Size: 137 bytes --] -- Strace-devel mailing list Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org https://lists.strace.io/mailman/listinfo/strace-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request 2018-11-21 23:56 ` Dmitry V. Levin @ 2018-11-22 14:55 ` Andy Lutomirski [not found] ` <CALCETrXea_uRAqw_srW5CWgOzeM=RubaDbjnxZ=cUMy5Zv1TsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Andy Lutomirski @ 2018-11-22 14:55 UTC (permalink / raw) To: Dmitry V. Levin Cc: Andrew Lutomirski, Elvira Khabirova, Kees Cook, Sasha Levin, Linux API, Jann Horn, Oleg Nesterov, Steven Rostedt, Ingo Molnar, LKML, Eugene Syromiatnikov, strace-devel On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin <ldv@altlinux.org> wrote: > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote: > > Please cc linux-api@vger.kernel.org for future versions. > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote: > > > > > > struct ptrace_syscall_info { > > > __u8 op; /* 0 for entry, 1 for exit */ > > > > Can you add proper defines, like: > > > > #define PTRACE_SYSCALL_ENTRY 0 > > #define PTRACE_SYSCALL_EXIT 1 > > #define PTRACE_SYSCALL_SECCOMP 2 > > > > and make seccomp work from the start? I'd rather we don't merge an > > implementation that doesn't work for seccomp and then have to rework > > it later. > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the > same entry_info to return. I'm not sure there's any material difference. > > As long as implementation (ab)uses ptrace_message to tell one kind of stop > from another, it can distinguish syscall-entry-stop and syscall-exit-stop > from each other and from many other kinds of stops, but it cannot > distinguish PTRACE_EVENT_SECCOMP from e.g. PTRACE_EVENT_EXIT. Hmm. PTRACE_GET_SYSCALL_INFO should fail for PTRACE_EVENT_EXIT, I think. > > > > __u8 __pad0[7]; > > > union { > > > struct { > > > __s32 nr; > > > > __u64 please. Syscall numbers are, as a practical matter, 64 bits. > > Admittedly, the actual effects of setting the high bits are unclear, > > and seccomp has issues with it, but let's not perpetuate the problem. > > I agree. Although the implementation uses syscall_get_nr() > which returns int, this could potentially be fixed in the future. Agreed. Although if we ever start using those high bits, things will get confusing. > > > > __u32 arch; > > > __u64 instruction_pointer; > > > __u64 args[6]; > > > } entry_info; > > > struct { > > > __s64 rval; > > > __u8 is_error; > > > __u8 __pad1[7]; > > > } exit_info; > > > }; > > > }; > > > > Should seccomp events use entry_info or should they just literally > > supply seccomp_data? > > It certainly can use entry_info. > I'd prefer to avoid using in uapi/linux/ptrace.h those types > that are defined in uapi/linux/seccomp.h. Makes sense to me. Also, it's possible in principle to extend seccomp_data with other fields that are only generated if they're read, so passing struct seccomp_data to userspace as a struct may be the wrong thing to do. ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CALCETrXea_uRAqw_srW5CWgOzeM=RubaDbjnxZ=cUMy5Zv1TsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request [not found] ` <CALCETrXea_uRAqw_srW5CWgOzeM=RubaDbjnxZ=cUMy5Zv1TsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-11-22 19:15 ` Dmitry V. Levin [not found] ` <20181122191504.GB27204-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Dmitry V. Levin @ 2018-11-22 19:15 UTC (permalink / raw) To: Andy Lutomirski Cc: Eugene Syromiatnikov, Kees Cook, Jann Horn, Linux API, Oleg Nesterov, Steven Rostedt, LKML, Ingo Molnar, strace-devel-3+4lAyCyj6AWlMsSdNXQLw [-- Attachment #1.1: Type: text/plain, Size: 1928 bytes --] On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote: > On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote: > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote: > > > Please cc linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for future versions. > > > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote: > > > > > > > > struct ptrace_syscall_info { > > > > __u8 op; /* 0 for entry, 1 for exit */ > > > > > > Can you add proper defines, like: > > > > > > #define PTRACE_SYSCALL_ENTRY 0 > > > #define PTRACE_SYSCALL_EXIT 1 > > > #define PTRACE_SYSCALL_SECCOMP 2 > > > > > > and make seccomp work from the start? I'd rather we don't merge an > > > implementation that doesn't work for seccomp and then have to rework > > > it later. > > > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop > > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the > > same entry_info to return. > > I'm not sure there's any material difference. In that case we don't really need PTRACE_SYSCALL_SECCOMP: op field describes the structure inside the union to use, not the ptrace stop. > > As long as implementation (ab)uses ptrace_message to tell one kind of stop > > from another, it can distinguish syscall-entry-stop and syscall-exit-stop > > from each other and from many other kinds of stops, but it cannot > > distinguish PTRACE_EVENT_SECCOMP from e.g. PTRACE_EVENT_EXIT. > > Hmm. PTRACE_GET_SYSCALL_INFO should fail for PTRACE_EVENT_EXIT, I think. Unless we can change PTRACE_EVENT_SECCOMP to set some higher bits of ptrace_message (beyond SECCOMP_RET_DATA) which is very unlikely because it would qualify as an ABI change, this would require an additional field in struct task_struct because ptrace_message wouldn't be enough to distinguish PTRACE_EVENT_SECCOMP from PTRACE_EVENT_EXIT. -- ldv [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] [-- Attachment #2: Type: text/plain, Size: 137 bytes --] -- Strace-devel mailing list Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org https://lists.strace.io/mailman/listinfo/strace-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20181122191504.GB27204-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>]
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request [not found] ` <20181122191504.GB27204-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org> @ 2018-11-23 0:19 ` Andy Lutomirski [not found] ` <CALCETrXvXzRzoEqwEY_VZ7Vpt-sLwaF+rZPg+y_eG2xyzubXtw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Andy Lutomirski @ 2018-11-23 0:19 UTC (permalink / raw) To: Dmitry V. Levin Cc: Eugene Syromiatnikov, Kees Cook, Jann Horn, Linux API, Oleg Nesterov, Steven Rostedt, LKML, Andrew Lutomirski, Ingo Molnar, strace-devel-3+4lAyCyj6AWlMsSdNXQLw On Thu, Nov 22, 2018 at 11:15 AM Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org> wrote: > > On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote: > > On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote: > > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote: > > > > Please cc linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for future versions. > > > > > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote: > > > > > > > > > > struct ptrace_syscall_info { > > > > > __u8 op; /* 0 for entry, 1 for exit */ > > > > > > > > Can you add proper defines, like: > > > > > > > > #define PTRACE_SYSCALL_ENTRY 0 > > > > #define PTRACE_SYSCALL_EXIT 1 > > > > #define PTRACE_SYSCALL_SECCOMP 2 > > > > > > > > and make seccomp work from the start? I'd rather we don't merge an > > > > implementation that doesn't work for seccomp and then have to rework > > > > it later. > > > > > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop > > > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the > > > same entry_info to return. > > > > I'm not sure there's any material difference. > > In that case we don't really need PTRACE_SYSCALL_SECCOMP: op field > describes the structure inside the union to use, not the ptrace stop. Unless we think the structures might diverge in the future. > > > > As long as implementation (ab)uses ptrace_message to tell one kind of stop > > > from another, it can distinguish syscall-entry-stop and syscall-exit-stop > > > from each other and from many other kinds of stops, but it cannot > > > distinguish PTRACE_EVENT_SECCOMP from e.g. PTRACE_EVENT_EXIT. > > > > Hmm. PTRACE_GET_SYSCALL_INFO should fail for PTRACE_EVENT_EXIT, I think. > > Unless we can change PTRACE_EVENT_SECCOMP to set some higher bits of > ptrace_message (beyond SECCOMP_RET_DATA) which is very unlikely because > it would qualify as an ABI change, this would require an additional field > in struct task_struct because ptrace_message wouldn't be enough > to distinguish PTRACE_EVENT_SECCOMP from PTRACE_EVENT_EXIT. At the risk of making the patch more complicated, there's room to massively clean up the ptrace state. We could add a struct ptrace_tracee and put a struct ptrace_tracee *ptrace_tracee into task_struct. The struct would contain a pointer to the task_struct as well as ptrace (the flag field, I think), ptrace_entry, ptracer_cred, ptrace_message, and last_siginfo. And then we could add a field for the ptrace stop state that would indicate the actual reason for the current stop. We'd only allocate ptrace_tracee when someone attaches with ptrace, thus saving quite a few bytes for each task. It's a bit unfortunate if we allow PTRACE_GET_SYSCALL_INFO to success if the event is PTRACE_EVENT_EXIT. I'd also be a bit nervous about info leaks if we start calling the syscall accessors for tasks that aren't in syscalls. --Andy -- Strace-devel mailing list Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org https://lists.strace.io/mailman/listinfo/strace-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CALCETrXvXzRzoEqwEY_VZ7Vpt-sLwaF+rZPg+y_eG2xyzubXtw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request [not found] ` <CALCETrXvXzRzoEqwEY_VZ7Vpt-sLwaF+rZPg+y_eG2xyzubXtw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-11-23 4:01 ` Dmitry V. Levin [not found] ` <20181123040139.GB2572-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Dmitry V. Levin @ 2018-11-23 4:01 UTC (permalink / raw) To: Andy Lutomirski Cc: Eugene Syromiatnikov, Kees Cook, Jann Horn, Linux API, Oleg Nesterov, Steven Rostedt, LKML, Ingo Molnar, strace-devel-3+4lAyCyj6AWlMsSdNXQLw [-- Attachment #1.1: Type: text/plain, Size: 2300 bytes --] On Thu, Nov 22, 2018 at 04:19:10PM -0800, Andy Lutomirski wrote: > On Thu, Nov 22, 2018 at 11:15 AM Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org> wrote: > > > > On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote: > > > On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote: > > > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote: > > > > > Please cc linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for future versions. > > > > > > > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote: > > > > > > > > > > > > struct ptrace_syscall_info { > > > > > > __u8 op; /* 0 for entry, 1 for exit */ > > > > > > > > > > Can you add proper defines, like: > > > > > > > > > > #define PTRACE_SYSCALL_ENTRY 0 > > > > > #define PTRACE_SYSCALL_EXIT 1 > > > > > #define PTRACE_SYSCALL_SECCOMP 2 > > > > > > > > > > and make seccomp work from the start? I'd rather we don't merge an > > > > > implementation that doesn't work for seccomp and then have to rework > > > > > it later. > > > > > > > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop > > > > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the > > > > same entry_info to return. > > > > > > I'm not sure there's any material difference. > > > > In that case we don't really need PTRACE_SYSCALL_SECCOMP: op field > > describes the structure inside the union to use, not the ptrace stop. > > Unless we think the structures might diverge in the future. If these structures ever diverge, then a seccomp structure will be added to the union, and a portable userspace code will likely look this way: #include <linux/ptrace.h> ... struct ptrace_syscall_info info; long rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid, (void *) sizeof(info), &info); ... switch (info.op) { case PTRACE_SYSCALL_INFO_ENTRY: /* handle info.entry */ case PTRACE_SYSCALL_INFO_EXIT: /* handle info.exit */ #ifdef PTRACE_SYSCALL_INFO_SECCOMP case PTRACE_SYSCALL_INFO_SECCOMP: /* handle info.seccomp */ #endif default: /* handle unknown info.op */ } In other words, it would be better if PTRACE_SYSCALL_INFO_* selector constants were introduced along with corresponding structures in the union. -- ldv [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] [-- Attachment #2: Type: text/plain, Size: 137 bytes --] -- Strace-devel mailing list Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org https://lists.strace.io/mailman/listinfo/strace-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20181123040139.GB2572-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>]
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request [not found] ` <20181123040139.GB2572-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org> @ 2018-11-25 4:10 ` Dmitry V. Levin 2018-11-27 22:28 ` Kees Cook 0 siblings, 1 reply; 8+ messages in thread From: Dmitry V. Levin @ 2018-11-25 4:10 UTC (permalink / raw) To: Andy Lutomirski Cc: Eugene Syromiatnikov, Kees Cook, Jann Horn, Linux API, Oleg Nesterov, Steven Rostedt, LKML, Ingo Molnar, strace-devel-3+4lAyCyj6AWlMsSdNXQLw [-- Attachment #1.1: Type: text/plain, Size: 3266 bytes --] On Fri, Nov 23, 2018 at 07:01:39AM +0300, Dmitry V. Levin wrote: > On Thu, Nov 22, 2018 at 04:19:10PM -0800, Andy Lutomirski wrote: > > On Thu, Nov 22, 2018 at 11:15 AM Dmitry V. Levin wrote: > > > On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote: > > > > On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote: > > > > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote: > > > > > > Please cc linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for future versions. > > > > > > > > > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote: > > > > > > > > > > > > > > struct ptrace_syscall_info { > > > > > > > __u8 op; /* 0 for entry, 1 for exit */ > > > > > > > > > > > > Can you add proper defines, like: > > > > > > > > > > > > #define PTRACE_SYSCALL_ENTRY 0 > > > > > > #define PTRACE_SYSCALL_EXIT 1 > > > > > > #define PTRACE_SYSCALL_SECCOMP 2 > > > > > > > > > > > > and make seccomp work from the start? I'd rather we don't merge an > > > > > > implementation that doesn't work for seccomp and then have to rework > > > > > > it later. > > > > > > > > > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop > > > > > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the > > > > > same entry_info to return. > > > > > > > > I'm not sure there's any material difference. > > > > > > In that case we don't really need PTRACE_SYSCALL_SECCOMP: op field > > > describes the structure inside the union to use, not the ptrace stop. > > > > Unless we think the structures might diverge in the future. > > If these structures ever diverge, then a seccomp structure will be added > to the union, and a portable userspace code will likely look this way: > > #include <linux/ptrace.h> > ... > struct ptrace_syscall_info info; > long rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid, (void *) sizeof(info), &info); > ... > switch (info.op) { > case PTRACE_SYSCALL_INFO_ENTRY: > /* handle info.entry */ > case PTRACE_SYSCALL_INFO_EXIT: > /* handle info.exit */ > #ifdef PTRACE_SYSCALL_INFO_SECCOMP > case PTRACE_SYSCALL_INFO_SECCOMP: > /* handle info.seccomp */ > #endif > default: > /* handle unknown info.op */ > } > > In other words, it would be better if PTRACE_SYSCALL_INFO_* selector > constants were introduced along with corresponding structures in the > union. However, the approach I suggested doesn't provide forward compatibility: if userspace is compiled with kernel headers that don't define PTRACE_SYSCALL_INFO_SECCOMP, it will break when the kernel starts to use PTRACE_SYSCALL_INFO_SECCOMP instead of PTRACE_SYSCALL_INFO_ENTRY for PTRACE_EVENT_SECCOMP support in PTRACE_GET_SYSCALL_INFO. The solution is to introduce PTRACE_SYSCALL_INFO_SECCOMP and struct ptrace_syscall_info.seccomp along with PTRACE_EVENT_SECCOMP support in PTRACE_GET_SYSCALL_INFO. The initial revision of the seccomp structure could be made the same as the entry structure, or it can diverge from the beginning, e.g., by adding ret_data field containing SECCOMP_RET_DATA return value stored in ptrace_message, this would save ptracers an extra PTRACE_GETEVENTMSG call currently required to obtain it. -- ldv [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] [-- Attachment #2: Type: text/plain, Size: 137 bytes --] -- Strace-devel mailing list Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org https://lists.strace.io/mailman/listinfo/strace-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request 2018-11-25 4:10 ` Dmitry V. Levin @ 2018-11-27 22:28 ` Kees Cook 0 siblings, 0 replies; 8+ messages in thread From: Kees Cook @ 2018-11-27 22:28 UTC (permalink / raw) To: Dmitry V. Levin Cc: Andy Lutomirski, Elvira Khabirova, Linux API, Jann Horn, Oleg Nesterov, Steven Rostedt, Ingo Molnar, LKML, Eugene Syromiatnikov, strace-devel On Sat, Nov 24, 2018 at 8:10 PM, Dmitry V. Levin <ldv@altlinux.org> wrote: > On Fri, Nov 23, 2018 at 07:01:39AM +0300, Dmitry V. Levin wrote: >> On Thu, Nov 22, 2018 at 04:19:10PM -0800, Andy Lutomirski wrote: >> > On Thu, Nov 22, 2018 at 11:15 AM Dmitry V. Levin wrote: >> > > On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote: >> > > > On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote: >> > > > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote: >> > > > > > Please cc linux-api@vger.kernel.org for future versions. >> > > > > > >> > > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote: >> > > > > > > >> > > > > > > struct ptrace_syscall_info { >> > > > > > > __u8 op; /* 0 for entry, 1 for exit */ >> > > > > > >> > > > > > Can you add proper defines, like: >> > > > > > >> > > > > > #define PTRACE_SYSCALL_ENTRY 0 >> > > > > > #define PTRACE_SYSCALL_EXIT 1 >> > > > > > #define PTRACE_SYSCALL_SECCOMP 2 >> > > > > > >> > > > > > and make seccomp work from the start? I'd rather we don't merge an >> > > > > > implementation that doesn't work for seccomp and then have to rework >> > > > > > it later. Yes, please. >> > > > > >> > > > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop >> > > > > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the >> > > > > same entry_info to return. >> > > > >> > > > I'm not sure there's any material difference. >> > > >> > > In that case we don't really need PTRACE_SYSCALL_SECCOMP: op field >> > > describes the structure inside the union to use, not the ptrace stop. >> > >> > Unless we think the structures might diverge in the future. Yes, I want to make sure we have a way to expand this, especially for seccomp: we've come close a few times to adding new fields to struct seccomp_data, for example. >> >> If these structures ever diverge, then a seccomp structure will be added >> to the union, and a portable userspace code will likely look this way: >> >> #include <linux/ptrace.h> >> ... >> struct ptrace_syscall_info info; >> long rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid, (void *) sizeof(info), &info); >> ... >> switch (info.op) { >> case PTRACE_SYSCALL_INFO_ENTRY: >> /* handle info.entry */ >> case PTRACE_SYSCALL_INFO_EXIT: >> /* handle info.exit */ >> #ifdef PTRACE_SYSCALL_INFO_SECCOMP >> case PTRACE_SYSCALL_INFO_SECCOMP: >> /* handle info.seccomp */ >> #endif >> default: >> /* handle unknown info.op */ >> } >> >> In other words, it would be better if PTRACE_SYSCALL_INFO_* selector >> constants were introduced along with corresponding structures in the >> union. > > However, the approach I suggested doesn't provide forward compatibility: > if userspace is compiled with kernel headers that don't define > PTRACE_SYSCALL_INFO_SECCOMP, it will break when the kernel > starts to use PTRACE_SYSCALL_INFO_SECCOMP instead of > PTRACE_SYSCALL_INFO_ENTRY for PTRACE_EVENT_SECCOMP support > in PTRACE_GET_SYSCALL_INFO. > > The solution is to introduce PTRACE_SYSCALL_INFO_SECCOMP and struct > ptrace_syscall_info.seccomp along with PTRACE_EVENT_SECCOMP support > in PTRACE_GET_SYSCALL_INFO. The initial revision of the seccomp > structure could be made the same as the entry structure, or it can > diverge from the beginning, e.g., by adding ret_data field containing > SECCOMP_RET_DATA return value stored in ptrace_message, this would save > ptracers an extra PTRACE_GETEVENTMSG call currently required to obtain it. Yup, that'd be a nice addition. -- Kees Cook ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-27 22:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20181121165806.07da7c98@akathisia>
2018-11-21 22:56 ` [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request Andy Lutomirski
[not found] ` <CALCETrV3TPCkYyhoLLcikXVeF-RdZUuLTCvKReK3Qb9LSS9Csw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-11-21 23:56 ` Dmitry V. Levin
2018-11-22 14:55 ` Andy Lutomirski
[not found] ` <CALCETrXea_uRAqw_srW5CWgOzeM=RubaDbjnxZ=cUMy5Zv1TsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-11-22 19:15 ` Dmitry V. Levin
[not found] ` <20181122191504.GB27204-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
2018-11-23 0:19 ` Andy Lutomirski
[not found] ` <CALCETrXvXzRzoEqwEY_VZ7Vpt-sLwaF+rZPg+y_eG2xyzubXtw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-11-23 4:01 ` Dmitry V. Levin
[not found] ` <20181123040139.GB2572-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
2018-11-25 4:10 ` Dmitry V. Levin
2018-11-27 22:28 ` Kees Cook
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).