selftests/seccomp: Enhance per-arch ptrace syscall skip tests

Passing EPERM during syscall skipping was confusing since the test wasn't
actually exercising the errno evaluation -- it was just passing a literal
"1" (EPERM). Instead, expand the tests to check both direct value returns
(positive, 45000 in this case), and errno values (negative, -ESRCH in this
case) to check both fake success and fake failure during syscall skipping.

Reported-by: Colin Ian King <colin.king@canonical.com>
Fixes: a33b2d0359a0 ("selftests/seccomp: Add tests for basic ptrace actions")
Cc: stable@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Shuah Khan <shuah@kernel.org>

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 496a9a8c773aba088f0e88a752a08614d3750ec7..7e632b465ab48eea860ab32e2a2cb9b2ea1d0a4f 100644 (file)
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -1608,7 +1608,16 @@ TEST_F(TRACE_poke, getpid_runs_normally)
 #ifdef SYSCALL_NUM_RET_SHARE_REG
 # define EXPECT_SYSCALL_RETURN(val, action)    EXPECT_EQ(-1, action)
 #else
-# define EXPECT_SYSCALL_RETURN(val, action)    EXPECT_EQ(val, action)
+# define EXPECT_SYSCALL_RETURN(val, action)            \
+       do {                                            \
+               errno = 0;                              \
+               if (val < 0) {                          \
+                       EXPECT_EQ(-1, action);          \
+                       EXPECT_EQ(-(val), errno);       \
+               } else {                                \
+                       EXPECT_EQ(val, action);         \
+               }                                       \
+       } while (0)
 #endif
 
 /* Use PTRACE_GETREGS and PTRACE_SETREGS when available. This is useful for
@@ -1647,7 +1656,7 @@ 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)
+                   pid_t tracee, int syscall, int result)
 {
        int ret;
        ARCH_REGS regs;
@@ -1706,7 +1715,7 @@ void change_syscall(struct __test_metadata *_metadata,
 #ifdef SYSCALL_NUM_RET_SHARE_REG
                TH_LOG("Can't modify syscall return on this architecture");
 #else
-               regs.SYSCALL_RET = EPERM;
+               regs.SYSCALL_RET = result;
 #endif
 
 #ifdef HAVE_GETREGS
@@ -1734,14 +1743,19 @@ void tracer_syscall(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);
+               change_syscall(_metadata, tracee, __NR_getppid, 0);
                break;
        case 0x1003:
-               /* skip gettid. */
+               /* skip gettid with valid return code. */
                EXPECT_EQ(__NR_gettid, get_syscall(_metadata, tracee));
-               change_syscall(_metadata, tracee, -1);
+               change_syscall(_metadata, tracee, -1, 45000);
                break;
        case 0x1004:
+               /* skip openat with error. */
+               EXPECT_EQ(__NR_openat, get_syscall(_metadata, tracee));
+               change_syscall(_metadata, tracee, -1, -ESRCH);
+               break;
+       case 0x1005:
                /* do nothing (allow getppid) */
                EXPECT_EQ(__NR_getppid, get_syscall(_metadata, tracee));
                break;
@@ -1774,9 +1788,11 @@ void tracer_ptrace(struct __test_metadata *_metadata, pid_t tracee,
        nr = get_syscall(_metadata, tracee);
 
        if (nr == __NR_getpid)
-               change_syscall(_metadata, tracee, __NR_getppid);
+               change_syscall(_metadata, tracee, __NR_getppid, 0);
+       if (nr == __NR_gettid)
+               change_syscall(_metadata, tracee, -1, 45000);
        if (nr == __NR_openat)
-               change_syscall(_metadata, tracee, -1);
+               change_syscall(_metadata, tracee, -1, -ESRCH);
 }
 
 FIXTURE_DATA(TRACE_syscall) {
@@ -1793,8 +1809,10 @@ FIXTURE_SETUP(TRACE_syscall)
                BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1002),
                BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_gettid, 0, 1),
                BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1003),
-               BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getppid, 0, 1),
+               BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_openat, 0, 1),
                BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1004),
+               BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getppid, 0, 1),
+               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1005),
                BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
        };
 
@@ -1842,15 +1860,26 @@ TEST_F(TRACE_syscall, ptrace_syscall_redirected)
        EXPECT_NE(self->mypid, syscall(__NR_getpid));
 }
 
-TEST_F(TRACE_syscall, ptrace_syscall_dropped)
+TEST_F(TRACE_syscall, ptrace_syscall_errno)
+{
+       /* Swap SECCOMP_RET_TRACE tracer for PTRACE_SYSCALL tracer. */
+       teardown_trace_fixture(_metadata, self->tracer);
+       self->tracer = setup_trace_fixture(_metadata, tracer_ptrace, NULL,
+                                          true);
+
+       /* Tracer should skip the open syscall, resulting in ESRCH. */
+       EXPECT_SYSCALL_RETURN(-ESRCH, syscall(__NR_openat));
+}
+
+TEST_F(TRACE_syscall, ptrace_syscall_faked)
 {
        /* Swap SECCOMP_RET_TRACE tracer for PTRACE_SYSCALL tracer. */
        teardown_trace_fixture(_metadata, self->tracer);
        self->tracer = setup_trace_fixture(_metadata, tracer_ptrace, NULL,
                                           true);
 
-       /* Tracer should skip the open syscall, resulting in EPERM. */
-       EXPECT_SYSCALL_RETURN(EPERM, syscall(__NR_openat));
+       /* Tracer should skip the gettid syscall, resulting fake pid. */
+       EXPECT_SYSCALL_RETURN(45000, syscall(__NR_gettid));
 }
 
 TEST_F(TRACE_syscall, syscall_allowed)
@@ -1883,7 +1912,21 @@ TEST_F(TRACE_syscall, syscall_redirected)
        EXPECT_NE(self->mypid, syscall(__NR_getpid));
 }
 
-TEST_F(TRACE_syscall, syscall_dropped)
+TEST_F(TRACE_syscall, syscall_errno)
+{
+       long ret;
+
+       ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+       ASSERT_EQ(0, ret);
+
+       ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &self->prog, 0, 0);
+       ASSERT_EQ(0, ret);
+
+       /* openat has been skipped and an errno return. */
+       EXPECT_SYSCALL_RETURN(-ESRCH, syscall(__NR_openat));
+}
+
+TEST_F(TRACE_syscall, syscall_faked)
 {
        long ret;
 
@@ -1894,8 +1937,7 @@ TEST_F(TRACE_syscall, syscall_dropped)
        ASSERT_EQ(0, ret);
 
        /* gettid has been skipped and an altered return value stored. */
-       EXPECT_SYSCALL_RETURN(EPERM, syscall(__NR_gettid));
-       EXPECT_NE(self->mytid, syscall(__NR_gettid));
+       EXPECT_SYSCALL_RETURN(45000, syscall(__NR_gettid));
 }
 
 TEST_F(TRACE_syscall, skip_after_RET_TRACE)