diff mbox series

[v2,2/4] selftests/seccomp: Allow syscall nr and ret value to be set separately

Message ID 20200919080637.259478-3-keescook@chromium.org (mailing list archive)
State Not Applicable
Headers show
Series selftests/seccomp: Refactor change_syscall() | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/merge (ace1986562a0814f179ecd2f1e648215ebc6625a)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/next (b5c8a2934eecbba3d688a911b98e92f8670ff462)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linus/master (eb5f95f1593f7c22dac681b19e815828e2af3efd)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/fixes (0460534b532e5518c657c7d6492b9337d975eaa3)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linux-next (b652d2a5f2a4e93d803cc33eb57fdc41ee528500)
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch

Commit Message

Kees Cook Sept. 19, 2020, 8:06 a.m. UTC
In preparation for setting syscall nr and ret values separately, refactor
the helpers to take a pointer to a value, so that a NULL can indicate
"do not change this respective value". This is done to keep the regset
read/write happening once and in one code path.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 59 +++++++++++++++----
 1 file changed, 47 insertions(+), 12 deletions(-)

Comments

Christian Brauner Sept. 21, 2020, 7:50 a.m. UTC | #1
On Sat, Sep 19, 2020 at 01:06:35AM -0700, Kees Cook wrote:
> In preparation for setting syscall nr and ret values separately, refactor
> the helpers to take a pointer to a value, so that a NULL can indicate
> "do not change this respective value". This is done to keep the regset
> read/write happening once and in one code path.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---

Looks good!
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
diff mbox series

Patch

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index c0311b4c736b..98ce5e8a6398 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -1888,27 +1888,47 @@  int get_syscall(struct __test_metadata *_metadata, pid_t tracee)
 }
 
 /* Architecture-specific syscall changing routine. */
-void change_syscall(struct __test_metadata *_metadata,
-		    pid_t tracee, int syscall, int result)
+void __change_syscall(struct __test_metadata *_metadata,
+		    pid_t tracee, long *syscall, long *ret)
 {
 	ARCH_REGS orig, regs;
 
+	/* Do not get/set registers if we have nothing to do. */
+	if (!syscall && !ret)
+		return;
+
 	EXPECT_EQ(0, ARCH_GETREGS(regs)) {
 		return;
 	}
 	orig = regs;
 
-	SYSCALL_NUM_SET(regs, syscall);
+	if (syscall)
+		SYSCALL_NUM_SET(regs, *syscall);
 
-	/* If syscall is skipped, change return value. */
-	if (syscall == -1)
-		SYSCALL_RET_SET(regs, result);
+	if (ret)
+		SYSCALL_RET_SET(regs, *ret);
 
 	/* Flush any register changes made. */
 	if (memcmp(&orig, &regs, sizeof(orig)) != 0)
 		EXPECT_EQ(0, ARCH_SETREGS(regs));
 }
 
+/* Change only syscall number. */
+void change_syscall_nr(struct __test_metadata *_metadata,
+		       pid_t tracee, long syscall)
+{
+	__change_syscall(_metadata, tracee, &syscall, NULL);
+}
+
+/* Change syscall return value (and set syscall number to -1). */
+void change_syscall_ret(struct __test_metadata *_metadata,
+			pid_t tracee, long ret)
+{
+	long syscall = -1;
+
+	__change_syscall(_metadata, tracee, &syscall, &ret);
+}
+
 void tracer_seccomp(struct __test_metadata *_metadata, pid_t tracee,
 		    int status, void *args)
 {
@@ -1924,17 +1944,17 @@  void tracer_seccomp(struct __test_metadata *_metadata, pid_t tracee,
 	case 0x1002:
 		/* change getpid to getppid. */
 		EXPECT_EQ(__NR_getpid, get_syscall(_metadata, tracee));
-		change_syscall(_metadata, tracee, __NR_getppid, 0);
+		change_syscall_nr(_metadata, tracee, __NR_getppid);
 		break;
 	case 0x1003:
 		/* skip gettid with valid return code. */
 		EXPECT_EQ(__NR_gettid, get_syscall(_metadata, tracee));
-		change_syscall(_metadata, tracee, -1, 45000);
+		change_syscall_ret(_metadata, tracee, 45000);
 		break;
 	case 0x1004:
 		/* skip openat with error. */
 		EXPECT_EQ(__NR_openat, get_syscall(_metadata, tracee));
-		change_syscall(_metadata, tracee, -1, -ESRCH);
+		change_syscall_ret(_metadata, tracee, -ESRCH);
 		break;
 	case 0x1005:
 		/* do nothing (allow getppid) */
@@ -1961,6 +1981,8 @@  void tracer_ptrace(struct __test_metadata *_metadata, pid_t tracee,
 	int ret;
 	unsigned long msg;
 	static bool entry;
+	long syscall_nr_val, syscall_ret_val;
+	long *syscall_nr = NULL, *syscall_ret = NULL;
 	FIXTURE_DATA(TRACE_syscall) *self = args;
 
 	/*
@@ -1987,17 +2009,30 @@  void tracer_ptrace(struct __test_metadata *_metadata, pid_t tracee,
 	else
 		return;
 
+	syscall_nr = &syscall_nr_val;
+	syscall_ret = &syscall_ret_val;
+
+	/* Now handle the actual rewriting cases. */
 	switch (self->syscall_nr) {
 	case __NR_getpid:
-		change_syscall(_metadata, tracee, __NR_getppid, 0);
+		syscall_nr_val = __NR_getppid;
+		/* Never change syscall return for this case. */
+		syscall_ret = NULL;
 		break;
 	case __NR_gettid:
-		change_syscall(_metadata, tracee, -1, 45000);
+		syscall_nr_val = -1;
+		syscall_ret_val = 45000;
 		break;
 	case __NR_openat:
-		change_syscall(_metadata, tracee, -1, -ESRCH);
+		syscall_nr_val = -1;
+		syscall_ret_val = -ESRCH;
 		break;
+	default:
+		/* Unhandled, do nothing. */
+		return;
 	}
+
+	__change_syscall(_metadata, tracee, syscall_nr, syscall_ret);
 }
 
 FIXTURE_VARIANT(TRACE_syscall) {