On Sat, Dec 28, 2019 at 10:24:51PM -0800, Sargun Dhillon wrote: > Add a self-test to make sure that the kernel returns EINVAL, if any > of the fields in seccomp_notif are set to non-null. > > Signed-off-by: Sargun Dhillon <sargun@xxxxxxxxx> > Suggested-by: Christian Brauner <christian.brauner@xxxxxxxxxx> > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > --- > tools/testing/selftests/seccomp/seccomp_bpf.c | 23 +++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index f53f14971bff..379391a7fa41 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -3601,6 +3601,29 @@ TEST(user_notification_continue) > } > } > > +TEST(user_notification_garbage) > +{ > + /* > + * intentionally set pid to a garbage value to make sure the kernel > + * catches it > + */ > + struct seccomp_notif req = { > + .pid = 1, > + }; > + int ret, 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_trap_syscall(__NR_dup, SECCOMP_FILTER_FLAG_NEW_LISTENER); > + ASSERT_GE(listener, 0); > + > + EXPECT_EQ(-1, ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req)); > + EXPECT_EQ(EINVAL, errno); Does that even work if no dup() syscall has been made and trapped? This looks like it would give you ENOENT... If you want a simple solution just do: diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 6944b898bb53..4c73ae8679ea 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3095,7 +3095,7 @@ TEST(user_notification_basic) pid_t pid; long ret; int status, listener; - struct seccomp_notif req = {}; + struct seccomp_notif req; struct seccomp_notif_resp resp = {}; struct pollfd pollfd; @@ -3158,6 +3158,13 @@ TEST(user_notification_basic) EXPECT_GT(poll(&pollfd, 1, -1), 0); EXPECT_EQ(pollfd.revents, POLLIN); + /* Test that we can't pass garbage to the kernel. */ + memset(&req, 0, sizeof(req)); + req.pid = -1; + EXPECT_EQ(-1, ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req)); + EXPECT_EQ(EINVAL, errno); + + req.pid = 0; EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); pollfd.fd = listener If you want a complete separate test then you can do: TEST(user_notification_garbage_recv) { pid_t pid; long ret; int status, listener; struct seccomp_notif req; struct seccomp_notif_resp resp = {}; struct pollfd pollfd; 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_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0); pid = fork(); ASSERT_GE(pid, 0); if (pid == 0) { ret = syscall(__NR_getppid); exit(ret != USER_NOTIF_MAGIC); } pollfd.fd = listener; pollfd.events = POLLIN | POLLOUT; EXPECT_GT(poll(&pollfd, 1, -1), 0); EXPECT_EQ(pollfd.revents, POLLIN); memset(&req, 0, sizeof(req)); req.pid = -1; EXPECT_EQ(-1, ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req)); EXPECT_EQ(EINVAL, errno); req.pid = 0; EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); pollfd.fd = listener; pollfd.events = POLLIN | POLLOUT; EXPECT_GT(poll(&pollfd, 1, -1), 0); EXPECT_EQ(pollfd.revents, POLLOUT); EXPECT_EQ(req.data.nr, __NR_getppid); memset(&resp, 0, sizeof(resp)); resp.id = req.id; resp.error = 0; resp.val = USER_NOTIF_MAGIC; EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0); EXPECT_EQ(waitpid(pid, &status, 0), pid); EXPECT_EQ(true, WIFEXITED(status)); EXPECT_EQ(0, WEXITSTATUS(status)); } Christian