linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing
@ 2020-07-05  6:12 Kees Cook
  2020-07-05  6:12 ` [PATCH 1/3] selftests/harness: Clean up kern-doc for fixtures Kees Cook
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Kees Cook @ 2020-07-05  6:12 UTC (permalink / raw)
  To: Will Deacon
  Cc: Kees Cook, Keno Fischer, Andy Lutomirski, Will Drewry,
	Oleg Nesterov, Shuah Khan, linux-kselftest, linux-kernel

Hi,

This expands the seccomp selftest to poke a architectural behavior corner
that Keno Fischer noticed[1]. In the process, I took the opportunity
to do the kselftest harness variant refactoring I'd been meaning to do,
which made adding this test much nicer.

I'd prefer this went via the seccomp tree, as it builds on top of the
other recent seccomp feature addition tests. Testing and reviews are
welcome! :)

Thanks,

-Kees

[1] https://lore.kernel.org/lkml/CABV8kRxA9mXPZwtYrjbAfOfFewhABHddipccgk-LQJO+ZYu4Xg@mail.gmail.com

Kees Cook (3):
  selftests/harness: Clean up kern-doc for fixtures
  selftests/seccomp: Refactor to use fixture variants
  selftests/seccomp: Check ENOSYS under tracing

 tools/testing/selftests/kselftest_harness.h   |  15 +-
 tools/testing/selftests/seccomp/seccomp_bpf.c | 217 ++++++------------
 2 files changed, 72 insertions(+), 160 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] selftests/harness: Clean up kern-doc for fixtures
  2020-07-05  6:12 [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing Kees Cook
@ 2020-07-05  6:12 ` Kees Cook
  2020-07-05 16:53   ` Jakub Kicinski
  2020-07-05  6:12 ` [PATCH 2/3] selftests/seccomp: Refactor to use fixture variants Kees Cook
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2020-07-05  6:12 UTC (permalink / raw)
  To: Will Deacon
  Cc: Kees Cook, Shuah Khan, Jakub Kicinski, Keno Fischer,
	Andy Lutomirski, Will Drewry, Oleg Nesterov, linux-kselftest,
	linux-kernel

The FIXTURE*() macro kern-doc examples had the wrong names for the C code
examples associated with them. Fix those and clarify that FIXTURE_DATA()
usage should be avoided.

Cc: Shuah Khan <shuah@kernel.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Fixes: 74bc7c97fa88 ("kselftest: add fixture variants")
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 tools/testing/selftests/kselftest_harness.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index c9f03ef93338..7f32a7099a81 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -195,8 +195,9 @@
  *
  * .. code-block:: c
  *
- *     FIXTURE_DATA(datatype name)
+ *     FIXTURE_DATA(datatype_name)
  *
+ * Almost always, you want just FIXTURE() instead (see below).
  * This call may be used when the type of the fixture data
  * is needed.  In general, this should not be needed unless
  * the *self* is being passed to a helper directly.
@@ -211,7 +212,7 @@
  *
  * .. code-block:: c
  *
- *     FIXTURE(datatype name) {
+ *     FIXTURE(fixture_name) {
  *       type property1;
  *       ...
  *     };
@@ -238,7 +239,7 @@
  *
  * .. code-block:: c
  *
- *     FIXTURE_SETUP(fixture name) { implementation }
+ *     FIXTURE_SETUP(fixture_name) { implementation }
  *
  * Populates the required "setup" function for a fixture.  An instance of the
  * datatype defined with FIXTURE_DATA() will be exposed as *self* for the
@@ -264,7 +265,7 @@
  *
  * .. code-block:: c
  *
- *     FIXTURE_TEARDOWN(fixture name) { implementation }
+ *     FIXTURE_TEARDOWN(fixture_name) { implementation }
  *
  * Populates the required "teardown" function for a fixture.  An instance of the
  * datatype defined with FIXTURE_DATA() will be exposed as *self* for the
@@ -285,7 +286,7 @@
  *
  * .. code-block:: c
  *
- *     FIXTURE_VARIANT(datatype name) {
+ *     FIXTURE_VARIANT(fixture_name) {
  *       type property1;
  *       ...
  *     };
@@ -305,8 +306,8 @@
  *
  * .. code-block:: c
  *
- *     FIXTURE_ADD(datatype name) {
- *       .property1 = val1;
+ *     FIXTURE_VARIANT_ADD(fixture_name, variant_name) {
+ *       .property1 = val1,
  *       ...
  *     };
  *
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] selftests/seccomp: Refactor to use fixture variants
  2020-07-05  6:12 [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing Kees Cook
  2020-07-05  6:12 ` [PATCH 1/3] selftests/harness: Clean up kern-doc for fixtures Kees Cook
@ 2020-07-05  6:12 ` Kees Cook
  2020-07-05 16:53   ` Jakub Kicinski
  2020-07-05  6:12 ` [PATCH 3/3] selftests/seccomp: Check ENOSYS under tracing Kees Cook
  2020-07-10 12:40 ` [PATCH for-next/seccomp 0/3] " Will Deacon
  3 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2020-07-05  6:12 UTC (permalink / raw)
  To: Will Deacon
  Cc: Kees Cook, Andy Lutomirski, Will Drewry, Jakub Kicinski,
	Keno Fischer, Oleg Nesterov, Shuah Khan, linux-kselftest,
	linux-kernel

Now that the selftest harness has variants, use them to eliminate a
bunch of copy/paste duplication.

Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 199 ++++--------------
 1 file changed, 42 insertions(+), 157 deletions(-)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 6439c031a85d..966dec340ea8 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -1541,6 +1541,7 @@ pid_t setup_trace_fixture(struct __test_metadata *_metadata,
 
 	return tracer_pid;
 }
+
 void teardown_trace_fixture(struct __test_metadata *_metadata,
 			    pid_t tracer)
 {
@@ -1820,7 +1821,7 @@ void change_syscall(struct __test_metadata *_metadata,
 	EXPECT_EQ(0, ret);
 }
 
-void tracer_syscall(struct __test_metadata *_metadata, pid_t tracee,
+void tracer_seccomp(struct __test_metadata *_metadata, pid_t tracee,
 		    int status, void *args)
 {
 	int ret;
@@ -1897,6 +1898,24 @@ FIXTURE(TRACE_syscall) {
 	pid_t tracer, mytid, mypid, parent;
 };
 
+FIXTURE_VARIANT(TRACE_syscall) {
+	/*
+	 * All of the SECCOMP_RET_TRACE behaviors can be tested with either
+	 * SECCOMP_RET_TRACE+PTRACE_CONT or plain ptrace()+PTRACE_SYSCALL.
+	 * This indicates if we should use SECCOMP_RET_TRACE (false), or
+	 * ptrace (true).
+	 */
+	bool use_ptrace;
+};
+
+FIXTURE_VARIANT_ADD(TRACE_syscall, ptrace) {
+	.use_ptrace = true,
+};
+
+FIXTURE_VARIANT_ADD(TRACE_syscall, seccomp) {
+	.use_ptrace = false,
+};
+
 FIXTURE_SETUP(TRACE_syscall)
 {
 	struct sock_filter filter[] = {
@@ -1912,12 +1931,11 @@ FIXTURE_SETUP(TRACE_syscall)
 		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1005),
 		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
 	};
-
-	memset(&self->prog, 0, sizeof(self->prog));
-	self->prog.filter = malloc(sizeof(filter));
-	ASSERT_NE(NULL, self->prog.filter);
-	memcpy(self->prog.filter, filter, sizeof(filter));
-	self->prog.len = (unsigned short)ARRAY_SIZE(filter);
+	struct sock_fprog prog = {
+		.len = (unsigned short)ARRAY_SIZE(filter),
+		.filter = filter,
+	};
+	long ret;
 
 	/* Prepare some testable syscall results. */
 	self->mytid = syscall(__NR_gettid);
@@ -1935,60 +1953,28 @@ FIXTURE_SETUP(TRACE_syscall)
 	ASSERT_NE(self->parent, self->mypid);
 
 	/* Launch tracer. */
-	self->tracer = setup_trace_fixture(_metadata, tracer_syscall, NULL,
-					   false);
-}
-
-FIXTURE_TEARDOWN(TRACE_syscall)
-{
-	teardown_trace_fixture(_metadata, self->tracer);
-	if (self->prog.filter)
-		free(self->prog.filter);
-}
+	self->tracer = setup_trace_fixture(_metadata,
+					   variant->use_ptrace ? tracer_ptrace
+							       : tracer_seccomp,
+					   NULL, variant->use_ptrace);
 
-TEST_F(TRACE_syscall, ptrace_syscall_redirected)
-{
-	/* 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 will redirect getpid to getppid. */
-	EXPECT_NE(self->mypid, syscall(__NR_getpid));
-}
+	ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+	ASSERT_EQ(0, ret);
 
-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);
+	if (variant->use_ptrace)
+		return;
 
-	/* Tracer should skip the open syscall, resulting in ESRCH. */
-	EXPECT_SYSCALL_RETURN(-ESRCH, syscall(__NR_openat));
+	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0);
+	ASSERT_EQ(0, ret);
 }
 
-TEST_F(TRACE_syscall, ptrace_syscall_faked)
+FIXTURE_TEARDOWN(TRACE_syscall)
 {
-	/* 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 gettid syscall, resulting fake pid. */
-	EXPECT_SYSCALL_RETURN(45000, syscall(__NR_gettid));
 }
 
 TEST_F(TRACE_syscall, syscall_allowed)
 {
-	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);
-
 	/* getppid works as expected (no changes). */
 	EXPECT_EQ(self->parent, syscall(__NR_getppid));
 	EXPECT_NE(self->mypid, syscall(__NR_getppid));
@@ -1996,14 +1982,6 @@ TEST_F(TRACE_syscall, syscall_allowed)
 
 TEST_F(TRACE_syscall, syscall_redirected)
 {
-	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);
-
 	/* getpid has been redirected to getppid as expected. */
 	EXPECT_EQ(self->parent, syscall(__NR_getpid));
 	EXPECT_NE(self->mypid, syscall(__NR_getpid));
@@ -2011,33 +1989,17 @@ TEST_F(TRACE_syscall, syscall_redirected)
 
 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. */
+	/* Tracer should skip the open syscall, resulting in ESRCH. */
 	EXPECT_SYSCALL_RETURN(-ESRCH, syscall(__NR_openat));
 }
 
 TEST_F(TRACE_syscall, syscall_faked)
 {
-	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);
-
-	/* gettid has been skipped and an altered return value stored. */
+	/* Tracer skips the gettid syscall and store altered return value. */
 	EXPECT_SYSCALL_RETURN(45000, syscall(__NR_gettid));
 }
 
-TEST_F(TRACE_syscall, skip_after_RET_TRACE)
+TEST_F(TRACE_syscall, skip_after)
 {
 	struct sock_filter filter[] = {
 		BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
@@ -2052,14 +2014,7 @@ TEST_F(TRACE_syscall, skip_after_RET_TRACE)
 	};
 	long ret;
 
-	ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Install fixture filter. */
-	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &self->prog, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Install "errno on getppid" filter. */
+	/* Install additional "errno on getppid" filter. */
 	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0);
 	ASSERT_EQ(0, ret);
 
@@ -2069,7 +2024,7 @@ TEST_F(TRACE_syscall, skip_after_RET_TRACE)
 	EXPECT_EQ(EPERM, errno);
 }
 
-TEST_F_SIGNAL(TRACE_syscall, kill_after_RET_TRACE, SIGSYS)
+TEST_F_SIGNAL(TRACE_syscall, kill_after, SIGSYS)
 {
 	struct sock_filter filter[] = {
 		BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
@@ -2084,77 +2039,7 @@ TEST_F_SIGNAL(TRACE_syscall, kill_after_RET_TRACE, SIGSYS)
 	};
 	long ret;
 
-	ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Install fixture filter. */
-	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &self->prog, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Install "death on getppid" filter. */
-	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Tracer will redirect getpid to getppid, and we should die. */
-	EXPECT_NE(self->mypid, syscall(__NR_getpid));
-}
-
-TEST_F(TRACE_syscall, skip_after_ptrace)
-{
-	struct sock_filter filter[] = {
-		BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
-			offsetof(struct seccomp_data, nr)),
-		BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getppid, 0, 1),
-		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ERRNO | EPERM),
-		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
-	};
-	struct sock_fprog prog = {
-		.len = (unsigned short)ARRAY_SIZE(filter),
-		.filter = filter,
-	};
-	long ret;
-
-	/* 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);
-
-	ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Install "errno on getppid" filter. */
-	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Tracer will redirect getpid to getppid, and we should see EPERM. */
-	EXPECT_EQ(-1, syscall(__NR_getpid));
-	EXPECT_EQ(EPERM, errno);
-}
-
-TEST_F_SIGNAL(TRACE_syscall, kill_after_ptrace, SIGSYS)
-{
-	struct sock_filter filter[] = {
-		BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
-			offsetof(struct seccomp_data, nr)),
-		BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getppid, 0, 1),
-		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
-		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
-	};
-	struct sock_fprog prog = {
-		.len = (unsigned short)ARRAY_SIZE(filter),
-		.filter = filter,
-	};
-	long ret;
-
-	/* 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);
-
-	ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
-	ASSERT_EQ(0, ret);
-
-	/* Install "death on getppid" filter. */
+	/* Install additional "death on getppid" filter. */
 	ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0);
 	ASSERT_EQ(0, ret);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] selftests/seccomp: Check ENOSYS under tracing
  2020-07-05  6:12 [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing Kees Cook
  2020-07-05  6:12 ` [PATCH 1/3] selftests/harness: Clean up kern-doc for fixtures Kees Cook
  2020-07-05  6:12 ` [PATCH 2/3] selftests/seccomp: Refactor to use fixture variants Kees Cook
@ 2020-07-05  6:12 ` Kees Cook
  2020-07-05  7:01   ` Kees Cook
  2020-07-10 12:40 ` [PATCH for-next/seccomp 0/3] " Will Deacon
  3 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2020-07-05  6:12 UTC (permalink / raw)
  To: Will Deacon
  Cc: Kees Cook, Andy Lutomirski, Will Drewry, Keno Fischer,
	Oleg Nesterov, Shuah Khan, linux-kselftest, linux-kernel

There should be no difference between -1 and other negative syscalls
while tracing.

Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: Will Deacon <will@kernel.org>
Cc: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 966dec340ea8..bf6aa06c435c 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -1973,6 +1973,32 @@ FIXTURE_TEARDOWN(TRACE_syscall)
 	teardown_trace_fixture(_metadata, self->tracer);
 }
 
+TEST(negative_ENOSYS)
+{
+	/* Untraced negative syscalls should return ENOSYS. */
+	errno = 0;
+	EXPECT_EQ(-1, syscall(-1));
+	EXPECT_EQ(errno, ENOSYS);
+	errno = 0;
+	EXPECT_EQ(-1, syscall(-101));
+	EXPECT_EQ(errno, ENOSYS);
+}
+
+TEST_F(TRACE_syscall, negative_ENOSYS)
+{
+	/*
+	 * There should be no difference between an "internal" skip
+	 * and userspace asking for syscall "-1".
+	 */
+	errno = 0;
+	EXPECT_EQ(-1, syscall(-1));
+	EXPECT_EQ(errno, ENOSYS);
+	/* And no difference for "still not valid but not -1". */
+	errno = 0;
+	EXPECT_EQ(-1, syscall(-101));
+	EXPECT_EQ(errno, ENOSYS);
+}
+
 TEST_F(TRACE_syscall, syscall_allowed)
 {
 	/* getppid works as expected (no changes). */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] selftests/seccomp: Check ENOSYS under tracing
  2020-07-05  6:12 ` [PATCH 3/3] selftests/seccomp: Check ENOSYS under tracing Kees Cook
@ 2020-07-05  7:01   ` Kees Cook
  0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-07-05  7:01 UTC (permalink / raw)
  To: Will Deacon
  Cc: Andy Lutomirski, Will Drewry, Keno Fischer, Oleg Nesterov,
	Shuah Khan, linux-kselftest, linux-kernel

On Sat, Jul 04, 2020 at 11:12:32PM -0700, Kees Cook wrote:
> There should be no difference between -1 and other negative syscalls
> while tracing.
> 
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Keno Fischer <keno@juliacomputing.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  tools/testing/selftests/seccomp/seccomp_bpf.c | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index 966dec340ea8..bf6aa06c435c 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -1973,6 +1973,32 @@ FIXTURE_TEARDOWN(TRACE_syscall)
>  	teardown_trace_fixture(_metadata, self->tracer);
>  }
>  
> +TEST(negative_ENOSYS)
> +{
> +	/* Untraced negative syscalls should return ENOSYS. */
> +	errno = 0;
> +	EXPECT_EQ(-1, syscall(-1));
> +	EXPECT_EQ(errno, ENOSYS);
> +	errno = 0;
> +	EXPECT_EQ(-1, syscall(-101));
> +	EXPECT_EQ(errno, ENOSYS);
> +}
> +
> +TEST_F(TRACE_syscall, negative_ENOSYS)
> +{
> +	/*
> +	 * There should be no difference between an "internal" skip
> +	 * and userspace asking for syscall "-1".
> +	 */
> +	errno = 0;
> +	EXPECT_EQ(-1, syscall(-1));
> +	EXPECT_EQ(errno, ENOSYS);
> +	/* And no difference for "still not valid but not -1". */
> +	errno = 0;
> +	EXPECT_EQ(-1, syscall(-101));
> +	EXPECT_EQ(errno, ENOSYS);
> +}
> +

I realized after sending this that the second function could just be:

+TEST_F(TRACE_syscall, negative_ENOSYS)
+{
+	negative_ENOSYS(_metadata);
+}

:)

>  TEST_F(TRACE_syscall, syscall_allowed)
>  {
>  	/* getppid works as expected (no changes). */
> -- 
> 2.25.1
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] selftests/harness: Clean up kern-doc for fixtures
  2020-07-05  6:12 ` [PATCH 1/3] selftests/harness: Clean up kern-doc for fixtures Kees Cook
@ 2020-07-05 16:53   ` Jakub Kicinski
  0 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2020-07-05 16:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Will Deacon, Shuah Khan, Keno Fischer, Andy Lutomirski,
	Will Drewry, Oleg Nesterov, linux-kselftest, linux-kernel

On Sat,  4 Jul 2020 23:12:30 -0700 Kees Cook wrote:
> The FIXTURE*() macro kern-doc examples had the wrong names for the C code
> examples associated with them. Fix those and clarify that FIXTURE_DATA()
> usage should be avoided.
> 
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Fixes: 74bc7c97fa88 ("kselftest: add fixture variants")
> Signed-off-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] selftests/seccomp: Refactor to use fixture variants
  2020-07-05  6:12 ` [PATCH 2/3] selftests/seccomp: Refactor to use fixture variants Kees Cook
@ 2020-07-05 16:53   ` Jakub Kicinski
  0 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2020-07-05 16:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Will Deacon, Andy Lutomirski, Will Drewry, Keno Fischer,
	Oleg Nesterov, Shuah Khan, linux-kselftest, linux-kernel

On Sat,  4 Jul 2020 23:12:31 -0700 Kees Cook wrote:
> Now that the selftest harness has variants, use them to eliminate a
> bunch of copy/paste duplication.
> 
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Acked-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing
  2020-07-05  6:12 [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing Kees Cook
                   ` (2 preceding siblings ...)
  2020-07-05  6:12 ` [PATCH 3/3] selftests/seccomp: Check ENOSYS under tracing Kees Cook
@ 2020-07-10 12:40 ` Will Deacon
  2020-07-10 16:11   ` Kees Cook
  3 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2020-07-10 12:40 UTC (permalink / raw)
  To: Kees Cook
  Cc: Keno Fischer, Andy Lutomirski, Will Drewry, Oleg Nesterov,
	Shuah Khan, linux-kselftest, linux-kernel

On Sat, Jul 04, 2020 at 11:12:29PM -0700, Kees Cook wrote:
> This expands the seccomp selftest to poke a architectural behavior corner
> that Keno Fischer noticed[1]. In the process, I took the opportunity
> to do the kselftest harness variant refactoring I'd been meaning to do,
> which made adding this test much nicer.
> 
> I'd prefer this went via the seccomp tree, as it builds on top of the
> other recent seccomp feature addition tests. Testing and reviews are
> welcome! :)

Thanks! I tested these on arm64 (qemu) and they helped me to find a bug
in some patches I was writing.

Tested-by: Will Deacon <will@kernel.org>

Will

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing
  2020-07-10 12:40 ` [PATCH for-next/seccomp 0/3] " Will Deacon
@ 2020-07-10 16:11   ` Kees Cook
  0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-07-10 16:11 UTC (permalink / raw)
  To: Will Deacon
  Cc: Keno Fischer, Andy Lutomirski, Will Drewry, Oleg Nesterov,
	Shuah Khan, linux-kselftest, linux-kernel

On Fri, Jul 10, 2020 at 01:40:33PM +0100, Will Deacon wrote:
> On Sat, Jul 04, 2020 at 11:12:29PM -0700, Kees Cook wrote:
> > This expands the seccomp selftest to poke a architectural behavior corner
> > that Keno Fischer noticed[1]. In the process, I took the opportunity
> > to do the kselftest harness variant refactoring I'd been meaning to do,
> > which made adding this test much nicer.
> > 
> > I'd prefer this went via the seccomp tree, as it builds on top of the
> > other recent seccomp feature addition tests. Testing and reviews are
> > welcome! :)
> 
> Thanks! I tested these on arm64 (qemu) and they helped me to find a bug
> in some patches I was writing.

Hurray for tests! :)

> Tested-by: Will Deacon <will@kernel.org>

Thanks!

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-07-10 16:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-05  6:12 [PATCH for-next/seccomp 0/3] Check ENOSYS under tracing Kees Cook
2020-07-05  6:12 ` [PATCH 1/3] selftests/harness: Clean up kern-doc for fixtures Kees Cook
2020-07-05 16:53   ` Jakub Kicinski
2020-07-05  6:12 ` [PATCH 2/3] selftests/seccomp: Refactor to use fixture variants Kees Cook
2020-07-05 16:53   ` Jakub Kicinski
2020-07-05  6:12 ` [PATCH 3/3] selftests/seccomp: Check ENOSYS under tracing Kees Cook
2020-07-05  7:01   ` Kees Cook
2020-07-10 12:40 ` [PATCH for-next/seccomp 0/3] " Will Deacon
2020-07-10 16:11   ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).