Re: [PATCH v7 1/2] ptrace: add PTRACE_SET_SYSCALL_INFO syscall skipping support

[Oleg Nesterov <oleg@redhat.com>]

Well, still looks good to me...

But I forgot (sorry, actually didn't really read) the previous discussions
and IIRC Michal had some concerns about this patch. And you don't CC him.

Michal, can you take a look? Do you see anything wrong?

Oleg.

On 08/31, Renzo Davoli wrote:
>
> Extend PTRACE_SET_SYSCALL_INFO to support skipping a system call triggered
> via seccomp.
>
> When a tracer retrieves a ptrace_syscall_info structure with 'op' set to
> PTRACE_SYSCALL_INFO_SECCOMP, it can now choose to skip the system call.
> To do this, the tracer changes 'op' to PTRACE_SYSCALL_INFO_EXIT and
> populates the exit union fields (rval and is_error) to define the return
> value and error status for the tracee.
>
> System call suppression via PTRACE_SYSCALL_INFO_ENTRY is currently not
> implemented.  On some architectures (e.g. MIPS), when a system call is
> skipped by setting the syscall number to -1 at the entry stop, the
> architecture entry path unconditionally overwrites the return value
> register with -ENOSYS, clobbering any custom return value set by the
> tracer at the entry stop.
>
> Signed-off-by: Renzo Davoli <renzo@cs.unibo.it>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> Reviewed-by: Dmitry V. Levin <ldv@strace.io>
> ---
>  kernel/ptrace.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index d041645d9d17..64fd1b455297 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -1099,7 +1099,7 @@ ptrace_set_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
>
>  static int
>  ptrace_set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
> -			     struct ptrace_syscall_info *info)
> +			     struct ptrace_syscall_info *info, bool skip_syscall)
>  {
>  	long rval = info->exit.rval;
>
> @@ -1111,6 +1111,9 @@ ptrace_set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
>  	if (rval != info->exit.rval)
>  		return -ERANGE;
>
> +	if (skip_syscall)
> +		syscall_set_nr(child, regs, -1);
> +
>  	if (info->exit.is_error)
>  		syscall_set_return_value(child, regs, rval, 0);
>  	else
> @@ -1125,6 +1128,8 @@ ptrace_set_syscall_info(struct task_struct *child, unsigned long user_size,
>  {
>  	struct pt_regs *regs = task_pt_regs(child);
>  	struct ptrace_syscall_info info;
> +	int op;
> +	bool skip_syscall = false;
>
>  	if (user_size < sizeof(info))
>  		return -EINVAL;
> @@ -1141,15 +1146,27 @@ ptrace_set_syscall_info(struct task_struct *child, unsigned long user_size,
>  	if (info.flags || info.reserved)
>  		return -EINVAL;
>
> -	/* Changing the type of the system call stop is not supported yet. */
> -	if (ptrace_get_syscall_info_op(child) != info.op)
> -		return -EINVAL;
> +	/*
> +	 * Changing the type of the system call stop is not allowed, with the
> +	 * following exception:
> +	 * PTRACE_SYSCALL_INFO_SECCOMP can be changed to PTRACE_SYSCALL_INFO_EXIT
> +	 * to skip the system call
> +	 */
> +
> +	op = ptrace_get_syscall_info_op(child);
> +	if (op != info.op) {
> +		if (info.op == PTRACE_SYSCALL_INFO_EXIT &&
> +				op == PTRACE_SYSCALL_INFO_SECCOMP)
> +			skip_syscall = true;
> +		else
> +			return -EINVAL;
> +	}
>
>  	switch (info.op) {
>  	case PTRACE_SYSCALL_INFO_ENTRY:
>  		return ptrace_set_syscall_info_entry(child, regs, &info);
>  	case PTRACE_SYSCALL_INFO_EXIT:
> -		return ptrace_set_syscall_info_exit(child, regs, &info);
> +		return ptrace_set_syscall_info_exit(child, regs, &info, skip_syscall);
>  	case PTRACE_SYSCALL_INFO_SECCOMP:
>  		return ptrace_set_syscall_info_seccomp(child, regs, &info);
>  	default:
> --
> 2.55.0
>