Commit ed5f1326 authored by Kees Cook's avatar Kees Cook Committed by Shuah Khan
Browse files

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: default avatarColin Ian King <colin.king@canonical.com>
Fixes: a33b2d03 ("selftests/seccomp: Add tests for basic ptrace actions")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Signed-off-by: default avatarShuah Khan <shuah@kernel.org>
parent 7e35a594

tools/testing/selftests/seccomp/seccomp_bpf.c

+57 −15
Original line number Diff line number Diff line
@@ -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)

CRA Git | Maintained and supported by SUSTech CRA and CCSE