Messages in this thread | | | Date | Wed, 19 Oct 2022 22:04:44 -0700 | From | Kees Cook <> | Subject | Re: [PATCH 5/5] selftest/seccomp: add a new test for the sync mode of seccomp user notify |
| |
On October 19, 2022 6:10:48 PM PDT, Andrei Vagin <avagin@gmail.com> wrote: >Test output: >RUN global.user_notification_sync ... >seccomp_bpf.c:4279:user_notification_sync:basic: 8655 nsec/syscall >seccomp_bpf.c:4279:user_notification_sync:sync: 2919 nsec/syscall >OK global.user_notification_sync
This looks like a benchmark, not a functionality test. But maybe the test is "is sync faster than async?"
> >Signed-off-by: Andrei Vagin <avagin@gmail.com> >--- > tools/testing/selftests/seccomp/seccomp_bpf.c | 80 +++++++++++++++++++ > 1 file changed, 80 insertions(+) > >diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c >index 4ae6c8991307..01f872415c17 100644 >--- a/tools/testing/selftests/seccomp/seccomp_bpf.c >+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c >@@ -4241,6 +4241,86 @@ TEST(user_notification_addfd_rlimit) > close(memfd); > } > >+/* USER_NOTIF_BENCH_TIMEOUT is 100 miliseconds. */ >+#define USER_NOTIF_BENCH_TIMEOUT 100000000ULL >+#define NSECS_PER_SEC 1000000000ULL >+ >+#ifndef SECCOMP_USER_NOTIF_FD_SYNC_WAKE_UP >+#define SECCOMP_USER_NOTIF_FD_SYNC_WAKE_UP (1UL << 0) >+#define SECCOMP_IOCTL_NOTIF_SET_FLAGS SECCOMP_IOW(4, __u64) >+#endif >+ >+static void user_notification_sync_loop(struct __test_metadata *_metadata, >+ char *test_name, int listener) >+{ >+ struct timespec ts; >+ uint64_t start, end, nr; >+ struct seccomp_notif req = {}; >+ struct seccomp_notif_resp resp = {}; >+ >+ clock_gettime(CLOCK_MONOTONIC, &ts); >+ start = ts.tv_nsec + ts.tv_sec * NSECS_PER_SEC; >+ for (end = start, nr = 0; end - start < USER_NOTIF_BENCH_TIMEOUT; nr++) { >+ memset(&req, 0, sizeof(req)); >+ req.pid = 0; >+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); >+ >+ EXPECT_EQ(req.data.nr, __NR_getppid); >+ >+ resp.id = req.id; >+ resp.error = 0; >+ resp.val = USER_NOTIF_MAGIC; >+ resp.flags = 0; >+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0);
I think these EXPECTs should be ASSERTs...
>+ >+ clock_gettime(CLOCK_MONOTONIC, &ts); >+ end = ts.tv_nsec + ts.tv_sec * NSECS_PER_SEC; >+ } >+ TH_LOG("%s:\t%lld nsec/syscall", test_name, USER_NOTIF_BENCH_TIMEOUT / nr); >+} >+ >+TEST(user_notification_sync) >+{ >+ pid_t pid; >+ long ret; >+ int status, listener; >+ >+ ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); >+ ASSERT_EQ(0, ret) { >+ TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); >+ } >+ >+ listener = user_notif_syscall(__NR_getppid, >+ SECCOMP_FILTER_FLAG_NEW_LISTENER); >+ ASSERT_GE(listener, 0); >+ >+ pid = fork(); >+ ASSERT_GE(pid, 0); >+ >+ if (pid == 0) { >+ while (1) { >+ ret = syscall(__NR_getppid); >+ if (ret == USER_NOTIF_MAGIC) >+ continue; >+ break; >+ } >+ _exit(1); >+ } >+ >+ user_notification_sync_loop(_metadata, "basic", listener); >+ >+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SET_FLAGS, >+ SECCOMP_USER_NOTIF_FD_SYNC_WAKE_UP, 0), 0);
Same here.
Also can you test that invalid SET_FLAGS are correctly rejected here?
>+ >+ user_notification_sync_loop(_metadata, "sync", listener); >+
If the timings are collected, add a test that sync is <= async here?
>+ kill(pid, SIGKILL); >+ EXPECT_EQ(waitpid(pid, &status, 0), pid); >+ EXPECT_EQ(true, WIFSIGNALED(status)); >+ EXPECT_EQ(SIGKILL, WTERMSIG(status)); >+} >+ >+ > /* Make sure PTRACE_O_SUSPEND_SECCOMP requires CAP_SYS_ADMIN. */ > FIXTURE(O_SUSPEND_SECCOMP) { > pid_t pid;
Otherwise, yeah, looks good.
-- Kees Cook
| |