ptrace: add PTRACE_GET_SYSCALL_INFO request

struct ptrace_syscall_info {
        __u8 op; /* 0 for entry, 1 for exit */

        __u8 __pad0[7];
        union {
                struct {
                        __s32 nr;

                        __u32 arch;
                        __u64 instruction_pointer;
                        __u64 args[6];
                } entry_info;
                struct {
                        __s64 rval;
                        __u8 is_error;
                        __u8 __pad1[7];
                } exit_info;
        };
};

Please cc [email protected] for future versions.

Quoted 3 lines · Elvira Khabirova

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.

Quoted 4 lines

        __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.

Quoted 11 lines

                        __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?

Quoted 13 lines · Andy Lutomirski

Please cc [email protected] for future versions.

Quoted 3 lines · Elvira Khabirova

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

Quoted 3 lines

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.

Quoted 6 lines

Quoted 5 lines

        __u8 __pad0[7];
        union {
                struct {
                        __s32 nr;

Quoted 3 lines

__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.

Quoted 13 lines

Quoted 12 lines

                        __u32 arch;
                        __u64 instruction_pointer;
                        __u64 args[6];
                } entry_info;
                struct {
                        __s64 rval;
                        __u8 is_error;
                        __u8 __pad1[7];
                } exit_info;
        };
};

Quoted 2 lines

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.

Quoted 18 lines · Dmitry V. Levin

Quoted 16 lines · Andy Lutomirski

Please cc [email protected] for future versions.

Quoted 3 lines · Elvira Khabirova

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.

Quoted 3 lines

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.

Quoted 4 lines

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.

Quoted 19 lines · Andy Lutomirski

Quoted 18 lines · Dmitry V. Levin

Quoted 16 lines · Andy Lutomirski

Please cc [email protected] for future versions.

Quoted 3 lines · Elvira Khabirova

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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 6 lines

Quoted 5 lines

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.

Quoted 1 line

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.

Quoted 25 lines · Dmitry V. Levin

Quoted 23 lines · Andy Lutomirski

Quoted 21 lines · Dmitry V. Levin

Quoted 9 lines · Andy Lutomirski

Please cc [email protected] for future versions.

Quoted 3 lines · Elvira Khabirova

struct ptrace_syscall_info {
        __u8 op; /* 0 for entry, 1 for exit */

Can you add proper defines, like:

Quoted 7 lines

#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.

Quoted 4 lines

Quoted 1 line

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.

Quoted 25 lines · Andy Lutomirski

Quoted 24 lines · Dmitry V. Levin

Quoted 23 lines · Andy Lutomirski

Quoted 21 lines · Dmitry V. Levin

Quoted 9 lines · Andy Lutomirski

Please cc [email protected] for future versions.

Quoted 3 lines · Elvira Khabirova

struct ptrace_syscall_info {
        __u8 op; /* 0 for entry, 1 for exit */

Can you add proper defines, like:

Quoted 7 lines

#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.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

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.

Quoted 14 lines · Dmitry V. Levin

Quoted 13 lines · Andy Lutomirski

Quoted 12 lines · Dmitry V. Levin

Quoted 11 lines · Andy Lutomirski

Quoted 10 lines · Dmitry V. Levin

Quoted 9 lines · Andy Lutomirski

Please cc [email protected] for future versions.

Quoted 3 lines · Elvira Khabirova

struct ptrace_syscall_info {
        __u8 op; /* 0 for entry, 1 for exit */

Can you add proper defines, like:

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

#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.

Quoted 11 lines

Quoted 11 lines

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

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.

Quoted 22 lines

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 */
}

Quoted 3 lines

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.