Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request

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:

quoted

On Thu, Nov 22, 2018 at 11:15 AM Dmitry V. Levin wrote:

quoted

On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote:

quoted

On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote:

quoted

On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote:

quoted

Please cc linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for future versions.

On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote:

quoted

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