Re: [PATCH 1/5] ptrace: add PTRACE_SYSCALL_INFO_SECCOMP_SKIP

[Oleg Nesterov <oleg@redhat.com>]

On 07/02, Renzo Davoli wrote:
>
> Hi Oleg,
>
> > Rather than add the new PTRACE_SYSCALL_INFO_SECCOMP_SKIP, can't we teach
> > ptrace_set_syscall_info_seccomp() to treat info->entry.nr == -1 as "skip" ?
> it already does
> > Note that ptrace_set_syscall_info_seccomp() -> ptrace_set_syscall_info_entry()
> > already does syscall_set_nr().
> Syscall skipping is useless if there is not a way to set the return value/errno.
>
> As I explain in the cover letter
> + The tracer can skip the system call by setting the system call number
> + to -1. However, the current PTRACE_SET_SYSCALL_INFO interface does not
> + provide a way to specify the return value or error code that should be
> + reported to the tracee after skipping the call.
>
> currently retvalue/errno can be set only at PTRACE_SYSCALL_INFO_EXIT

I meant something like below. This way both PTRACE_SYSCALL_INFO_ENTRY and
__SECCOMP can skip the syscall and set the return/errr value.

Oleg.
---


diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index 5f8ef6156752..4ee7870f3291 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -90,7 +90,13 @@ struct ptrace_syscall_info {
 	union {
 		struct {
 			__u64 nr;
-			__u64 args[6];
+			union {
+				__u64 args[6];
+				struct {
+					__s64 rval;
+					__u8 is_error;
+				};
+			};
 		} entry;
 		struct {
 			__s64 rval;
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 130043bfc209..1daac0e62cfa 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -1031,6 +1031,28 @@ ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
 	return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
 }
 
+static int
+__set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
+			__s64 __rval, __u8 __is_error)
+{
+	long rval = __rval;
+
+	/*
+	 * Check that the return value specified in info->exit.rval
+	 * is either a value of type "long" or a sign-extended value
+	 * of type "long".
+	 */
+	if (rval != __rval)
+		return -ERANGE;
+
+	if (__is_error)
+		syscall_set_return_value(child, regs, rval, 0);
+	else
+		syscall_set_return_value(child, regs, 0, rval);
+
+	return 0;
+}
+
 static int
 ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
 			      struct ptrace_syscall_info *info)
@@ -1047,6 +1069,11 @@ ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
 	if (nr != info->entry.nr)
 		return -ERANGE;
 
+	syscall_set_nr(child, regs, nr);
+	if (nr == -1)
+		return __set_syscall_info_exit(child, regs,
+						info->entry.rval, info->entry.is_error);
+
 	for (i = 0; i < ARRAY_SIZE(args); i++) {
 		args[i] = info->entry.args[i];
 		/*
@@ -1058,16 +1085,7 @@ ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
 			return -ERANGE;
 	}
 
-	syscall_set_nr(child, regs, nr);
-	/*
-	 * If the syscall number is set to -1, setting syscall arguments is not
-	 * just pointless, it would also clobber the syscall return value on
-	 * those architectures that share the same register both for the first
-	 * argument of syscall and its return value.
-	 */
-	if (nr != -1)
-		syscall_set_arguments(child, regs, args);
-
+	syscall_set_arguments(child, regs, args);
 	return 0;
 }
 
@@ -1086,22 +1104,8 @@ static int
 ptrace_set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
 			     struct ptrace_syscall_info *info)
 {
-	long rval = info->exit.rval;
-
-	/*
-	 * Check that the return value specified in info->exit.rval
-	 * is either a value of type "long" or a sign-extended value
-	 * of type "long".
-	 */
-	if (rval != info->exit.rval)
-		return -ERANGE;
-
-	if (info->exit.is_error)
-		syscall_set_return_value(child, regs, rval, 0);
-	else
-		syscall_set_return_value(child, regs, 0, rval);
-
-	return 0;
+	return __set_syscall_info_exit(child, regs,
+		info->exit.rval, info->exit.is_error);
 }
 
 static int