Skip to content

Commit

Permalink
selftests/seccomp: Enhance per-arch ptrace syscall skip tests
Browse files Browse the repository at this point in the history 
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: a33b2d0 ("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>
  • Loading branch information
Kees Cook authored and Shuah Khan committed Jan 25, 2019
1 parent 7e35a59 commit ed5f132
Showing 1 changed file with 57 additions and 15 deletions.
72 changes: 57 additions & 15 deletions tools/testing/selftests/seccomp/seccomp_bpf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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),
};

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;

Expand All @@ -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)
Expand Down

0 comments on commit ed5f132

Please sign in to comment.