* [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted
@ 2026-09-19 20:35 Hui Peng
2026-09-19 22:22 ` Bradley Morgan
2026-09-20 3:50 ` Kees Cook
0 siblings, 2 replies; 8+ messages in thread
From: Hui Peng @ 2026-09-19 20:35 UTC (permalink / raw)
To: Kees Cook; +Cc: Andy Lutomirski, Will Drewry, linux-kernel
In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set,
knotif->state is transitioned from SECCOMP_NOTIFY_SENT to
SECCOMP_NOTIFY_REPLIED before dropping filter->notify_lock and waiting
in wait_for_completion_interruptible(&kaddfd.completion).
If wait_for_completion_interruptible() is interrupted by a signal before
the target task processes the kaddfd entry, seccomp_notify_addfd()
removes kaddfd.list from knotif->addfd via list_del(&kaddfd.list), but
leaves knotif->state set to SECCOMP_NOTIFY_REPLIED.
As a consequence:
1. When the target task wakes in seccomp_do_user_notification(), it
observes knotif->state == SECCOMP_NOTIFY_REPLIED with an empty addfd
list and zeroed knotif->val / knotif->error / knotif->flags, causing
the supervised syscall to prematurely complete with return value 0
without installing the file descriptor.
2. Any subsequent retry of SECCOMP_IOCTL_NOTIF_SEND or
SECCOMP_IOCTL_NOTIF_ADDFD by the supervisor fails with -EINPROGRESS
because knotif->state is no longer SECCOMP_NOTIFY_SENT.
Restore knotif->state = SECCOMP_NOTIFY_SENT when removing an unhandled
SECCOMP_ADDFD_FLAG_SEND request from knotif->addfd.
Fixes: 7cf97b125455 ("seccomp: Introduce addfd ioctl to seccomp user notifier")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1808,10 +1808,13 @@ static long seccomp_notify_addfd(struct seccomp_filter *filter,
* We need to check again if the addfd request has been handled,
* and if not, we will remove it from the queue.
*/
- if (list_empty(&kaddfd.list))
+ if (list_empty(&kaddfd.list)) {
ret = kaddfd.ret;
- else
+ } else {
list_del(&kaddfd.list);
+ if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND)
+ knotif->state = SECCOMP_NOTIFY_SENT;
+ }
out_unlock:
mutex_unlock(&filter->notify_lock);
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 2026-09-19 20:35 [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted Hui Peng @ 2026-09-19 22:22 ` Bradley Morgan 2026-09-20 3:50 ` Kees Cook 1 sibling, 0 replies; 8+ messages in thread From: Bradley Morgan @ 2026-09-19 22:22 UTC (permalink / raw) To: benquike; +Cc: kees, linux-kernel, luto, wad, akpm On 19 September 2026 21:35:14 BST, Hui Peng <benquike@gmail.com> wrote: >In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set, >knotif->state is transitioned from SECCOMP_NOTIFY_SENT to >SECCOMP_NOTIFY_REPLIED before dropping filter->notify_lock and waiting >in wait_for_completion_interruptible(&kaddfd.completion). > >If wait_for_completion_interruptible() is interrupted by a signal before >the target task processes the kaddfd entry, seccomp_notify_addfd() >removes kaddfd.list from knotif->addfd via list_del(&kaddfd.list), but >leaves knotif->state set to SECCOMP_NOTIFY_REPLIED. > >As a consequence: >1. When the target task wakes in seccomp_do_user_notification(), it > observes knotif->state == SECCOMP_NOTIFY_REPLIED with an empty addfd > list and zeroed knotif->val / knotif->error / knotif->flags, causing > the supervised syscall to prematurely complete with return value 0 > without installing the file descriptor. >2. Any subsequent retry of SECCOMP_IOCTL_NOTIF_SEND or > SECCOMP_IOCTL_NOTIF_ADDFD by the supervisor fails with -EINPROGRESS > because knotif->state is no longer SECCOMP_NOTIFY_SENT. > >Restore knotif->state = SECCOMP_NOTIFY_SENT when removing an unhandled >SECCOMP_ADDFD_FLAG_SEND request from knotif->addfd. > >Fixes: 7cf97b125455 ("seccomp: Introduce addfd ioctl to seccomp user notifier") >Assisted-by: LLM >Signed-off-by: Hui Peng <benquike@gmail.com> >--- >diff --git a/kernel/seccomp.c b/kernel/seccomp.c >--- a/kernel/seccomp.c >+++ b/kernel/seccomp.c >@@ -1808,10 +1808,13 @@ static long seccomp_notify_addfd(struct seccomp_filter *filter, > * We need to check again if the addfd request has been handled, > * and if not, we will remove it from the queue. > */ >- if (list_empty(&kaddfd.list)) >+ if (list_empty(&kaddfd.list)) { > ret = kaddfd.ret; >- else >+ } else { > list_del(&kaddfd.list); >+ if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND) >+ knotif->state = SECCOMP_NOTIFY_SENT; >+ } > > out_unlock: > mutex_unlock(&filter->notify_lock); > Akpm? Also, Reviewed-by: Bradley Morgan <brads@mainlining.org> --- Thanks! "I'm not a very positive person" - Linus torvalds ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 2026-09-19 20:35 [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted Hui Peng 2026-09-19 22:22 ` Bradley Morgan @ 2026-09-20 3:50 ` Kees Cook 2026-09-20 4:16 ` [PATCH v2] " Hui Peng ` (2 more replies) 1 sibling, 3 replies; 8+ messages in thread From: Kees Cook @ 2026-09-20 3:50 UTC (permalink / raw) To: Hui Peng; +Cc: Andy Lutomirski, Will Drewry, linux-kernel On Sat, Sep 19, 2026 at 08:35:14PM +0000, Hui Peng wrote: > In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set, > knotif->state is transitioned from SECCOMP_NOTIFY_SENT to > SECCOMP_NOTIFY_REPLIED before dropping filter->notify_lock and waiting > in wait_for_completion_interruptible(&kaddfd.completion). > > If wait_for_completion_interruptible() is interrupted by a signal before > the target task processes the kaddfd entry, seccomp_notify_addfd() > removes kaddfd.list from knotif->addfd via list_del(&kaddfd.list), but > leaves knotif->state set to SECCOMP_NOTIFY_REPLIED. Are you able to construct a regression test for this so we can catch this race if it ever gets exposed again? -- Kees Cook ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 2026-09-20 3:50 ` Kees Cook @ 2026-09-20 4:16 ` Hui Peng 2026-09-20 7:20 ` [PATCH v3] " Hui Peng 2026-09-20 10:10 ` [PATCH] " Lorenzo Stoakes (ARM) 2 siblings, 0 replies; 8+ messages in thread From: Hui Peng @ 2026-09-20 4:16 UTC (permalink / raw) To: Kees Cook, Andy Lutomirski, Will Drewry Cc: Bradley Morgan, linux-kernel, linux-kselftest, Hui Peng In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set, knotif->state is transitioned from SECCOMP_NOTIFY_SENT to SECCOMP_NOTIFY_REPLIED before dropping filter->notify_lock and waiting in wait_for_completion_interruptible(&kaddfd.completion). If wait_for_completion_interruptible() is interrupted by a signal before the target task processes the kaddfd entry, seccomp_notify_addfd() removes kaddfd.list from knotif->addfd via list_del(&kaddfd.list), but leaves knotif->state set to SECCOMP_NOTIFY_REPLIED. As a consequence: 1. When the target task wakes in seccomp_do_user_notification(), it observes knotif->state == SECCOMP_NOTIFY_REPLIED with an empty addfd list and zeroed knotif->val / knotif->error / knotif->flags, causing the supervised syscall to prematurely complete with return value 0 without installing the file descriptor. 2. Any subsequent retry of SECCOMP_IOCTL_NOTIF_SEND or SECCOMP_IOCTL_NOTIF_ADDFD by the supervisor fails with -EINPROGRESS (or -ENOENT after the target exits) because knotif->state is no longer SECCOMP_NOTIFY_SENT. Restore knotif->state = SECCOMP_NOTIFY_SENT when removing an unhandled SECCOMP_ADDFD_FLAG_SEND request from knotif->addfd, and add a kselftest (user_notification_addfd_send_interrupted) to cover this race. Fixes: 7cf97b125455 ("seccomp: Introduce addfd ioctl to seccomp user notifier") Reviewed-by: Bradley Morgan <brads@mainlining.org> Assisted-by: LLM Signed-off-by: Hui Peng <benquike@gmail.com> --- Changes in v2: - Added kselftest regression test (user_notification_addfd_send_interrupted) in tools/testing/selftests/seccomp/seccomp_bpf.c per Kees Cook. - Collected Reviewed-by tag from Bradley Morgan. kernel/seccomp.c | 7 +- tools/testing/selftests/seccomp/seccomp_bpf.c | 98 +++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/kernel/seccomp.c b/kernel/seccomp.c index 86cf4460d69e..94c7ba80a8f7 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -1808,10 +1808,13 @@ static long seccomp_notify_addfd(struct seccomp_filter *filter, * We need to check again if the addfd request has been handled, * and if not, we will remove it from the queue. */ - if (list_empty(&kaddfd.list)) + if (list_empty(&kaddfd.list)) { ret = kaddfd.ret; - else + } else { list_del(&kaddfd.list); + if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND) + knotif->state = SECCOMP_NOTIFY_SENT; + } out_unlock: mutex_unlock(&filter->notify_lock); -- 2.47.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 2026-09-20 3:50 ` Kees Cook 2026-09-20 4:16 ` [PATCH v2] " Hui Peng @ 2026-09-20 7:20 ` Hui Peng 2026-09-20 11:56 ` Bradley Morgan 2026-09-20 10:10 ` [PATCH] " Lorenzo Stoakes (ARM) 2 siblings, 1 reply; 8+ messages in thread From: Hui Peng @ 2026-09-20 7:20 UTC (permalink / raw) To: Kees Cook, Andy Lutomirski, Will Drewry, Shuah Khan Cc: linux-kernel, linux-kselftest, Hui Peng In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set in addfd.flags, knotif->state is optimistically transitioned from SECCOMP_NOTIFY_SENT to SECCOMP_NOTIFY_REPLIED before waking the tracee and waiting on kaddfd.completion: if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND) { knotif->state = SECCOMP_NOTIFY_REPLIED; ... } If wait_for_completion_interruptible(&kaddfd.completion) is interrupted by a signal before the tracee dequeues the kaddfd request (!list_empty(&kaddfd.list)), seccomp_notify_addfd() removes kaddfd from knotif->addfd and returns -EINTR to the supervisor without having installed the file descriptor. However, knotif->state is left as SECCOMP_NOTIFY_REPLIED (with knotif->error == 0 and knotif->val == 0). This causes two problems: 1. When the tracee runs in do_user_notif(), it checks if (knotif.state != SECCOMP_NOTIFY_REPLIED) after calling seccomp_handle_addfd(). Because knotif->state is already SECCOMP_NOTIFY_REPLIED, the tracee exits the notification wait loop prematurely and returns 0 from the trapped syscall without the file descriptor ever having been installed. 2. If the supervisor retries SECCOMP_IOCTL_NOTIF_ADDFD or SECCOMP_IOCTL_NOTIF_SEND before the tracee runs, it fails with -EINPROGRESS because knotif->state is no longer SECCOMP_NOTIFY_SENT. Fix this by restoring knotif->state back to SECCOMP_NOTIFY_SENT if SECCOMP_ADDFD_FLAG_SEND was set and kaddfd was not consumed before the interrupted wait. Also add a seccomp_bpf selftest (user_notification_addfd_send_interrupted) covering this race. Fixes: 0ae71c1990f3 ("seccomp: Add a flag to atomically addfd and perform send") Signed-off-by: Hui Peng <benquike@gmail.com> --- Changes in v3: - Actually include the tools/testing/selftests/seccomp/seccomp_bpf.c regression test in the patch diff (v2 accidentally omitted the selftest hunk), with detailed comments explaining the race and test setup. Changes in v2: - Add user_notification_addfd_send_interrupted regression test to tools/testing/selftests/seccomp/seccomp_bpf.c as requested by Kees Cook. kernel/seccomp.c | 2 + tools/testing/selftests/seccomp/seccomp_bpf.c | 161 ++++++++++++++++++ 2 files changed, 163 insertions(+) diff --git a/kernel/seccomp.c b/kernel/seccomp.c index 86cf4460d69e..94c7ba80a8f7 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -1808,10 +1808,13 @@ static long seccomp_notify_addfd(struct seccomp_filter *filter, * We need to check again if the addfd request has been handled, * and if not, we will remove it from the queue. */ - if (list_empty(&kaddfd.list)) + if (list_empty(&kaddfd.list)) { ret = kaddfd.ret; - else + } else { list_del(&kaddfd.list); + if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND) + knotif->state = SECCOMP_NOTIFY_SENT; + } out_unlock: mutex_unlock(&filter->notify_lock); diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 0622bc2acad4..e565728f9eb2 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -4368,6 +4368,173 @@ TEST(user_notification_addfd_rlimit) close(memfd); } +static void sigusr1_handler(int signo) +{ +} + +/* + * Verify that when SECCOMP_IOCTL_NOTIF_ADDFD with SECCOMP_ADDFD_FLAG_SEND is + * interrupted by a signal before the tracee dequeues the addfd request, + * knotif->state is restored from SECCOMP_NOTIFY_REPLIED back to + * SECCOMP_NOTIFY_SENT so that: + * 1. The woken tracee sees knotif->state == SECCOMP_NOTIFY_SENT in + * do_user_notif() and goes back to sleep instead of prematurely + * returning 0 from the trapped syscall without the FD installed. + * 2. The supervisor can retry SECCOMP_IOCTL_NOTIF_ADDFD (or + * SECCOMP_IOCTL_NOTIF_SEND) instead of failing with -EINPROGRESS. + * + * To deterministically hit the race window where the supervisor sleeps in + * wait_for_completion_interruptible(&kaddfd.completion) after waking the + * tracee (complete(&knotif->ready)) but before the tracee runs + * seccomp_handle_addfd(), pin all processes to a single CPU and enforce a + * strict 3-tier scheduling priority hierarchy on that CPU: + * - Supervisor: SCHED_FIFO priority 99 (highest) + * - Signal helper (sig_pid): SCHED_FIFO priority 50 (middle) + * - Tracee (pid): SCHED_IDLE (lowest) + */ +TEST(user_notification_addfd_send_interrupted) +{ + /* + * Save parent_pid before user_notif_syscall(__NR_getppid, ...) installs + * the seccomp filter on the calling process; children inherit that + * filter, so sig_pid must not call getppid(). + */ + pid_t pid, sig_pid, parent_pid = getpid(); + long ret; + int status, listener, memfd; + struct seccomp_notif_addfd addfd = {}; + struct seccomp_notif req = {}; + struct sigaction sa = {}; + struct sched_param sp_tracee_idle = { .sched_priority = 0 }; + struct sched_param sp_supervisor_fifo = { .sched_priority = 99 }; + struct sched_param sp_sig_helper_fifo = { .sched_priority = 50 }; + struct timespec delay = { .tv_nsec = 15000000 }; + cpu_set_t cpuset; + int cpu; + + /* Pin the supervisor (and its future child processes) to one CPU. */ + cpu = sched_getcpu(); + if (cpu >= 0) { + CPU_ZERO(&cpuset); + CPU_SET(cpu, &cpuset); + sched_setaffinity(0, sizeof(cpuset), &cpuset); + } + + sa.sa_handler = sigusr1_handler; + ASSERT_EQ(sigaction(SIGUSR1, &sa, NULL), 0); + + memfd = memfd_create("test", 0); + ASSERT_GE(memfd, 0); + + 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!"); + } + + /* + * Follow the convention of other user_notification_* tests in this + * file by trapping __NR_getppid: because the filter is installed on + * the supervisor before fork(), the trapped syscall must be a + * side-effect-free syscall that the supervisor itself never invokes. + * Even though getppid() does not normally return an FD, + * SECCOMP_ADDFD_FLAG_SEND replaces the trapped syscall's return value + * with the newly installed FD number (42). + */ + listener = user_notif_syscall(__NR_getppid, + SECCOMP_FILTER_FLAG_NEW_LISTENER); + ASSERT_GE(listener, 0); + + pid = fork(); + ASSERT_GE(pid, 0); + + if (pid == 0) { + /* + * Tracee: invoke __NR_getppid as a dummy trigger syscall to + * trap into do_user_notif(). Verify that the syscall returns + * the injected FD number (42) and that FD 42 is open. + */ + ret = syscall(__NR_getppid); + exit(ret != 42 || fcntl(42, F_GETFD) < 0); + } + + /* Wait for the tracee to trap in do_user_notif(). */ + ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); + + /* + * Demote the tracee to SCHED_IDLE and promote the supervisor to + * SCHED_FIFO(99) on the same CPU. + */ + sched_setscheduler(pid, SCHED_IDLE, &sp_tracee_idle); + if (sched_setscheduler(0, SCHED_FIFO, &sp_supervisor_fifo) != 0) { + close(listener); + close(memfd); + kill(pid, SIGKILL); + waitpid(pid, NULL, 0); + SKIP(return, "SCHED_FIFO requires CAP_SYS_NICE"); + } + + addfd.srcfd = memfd; + addfd.newfd_flags = O_CLOEXEC; + addfd.newfd = 42; + addfd.id = req.id; + addfd.flags = SECCOMP_ADDFD_FLAG_SETFD | SECCOMP_ADDFD_FLAG_SEND; + + /* + * Fork a signal helper on the same CPU and set it to SCHED_FIFO(50). + * Because the supervisor is currently running at SCHED_FIFO(99) on + * this CPU, sig_pid is queued on the runqueue but cannot run until the + * supervisor blocks inside the kernel. + * + * When the supervisor invokes ioctl(SECCOMP_IOCTL_NOTIF_ADDFD) below: + * 1. seccomp_notify_addfd() sets knotif->state = SECCOMP_NOTIFY_REPLIED, + * wakes the tracee (SCHED_IDLE), and blocks in + * wait_for_completion_interruptible(&kaddfd.completion). + * 2. The CPU scheduler immediately runs sig_pid (SCHED_FIFO 50) + * ahead of the woken tracee (SCHED_IDLE). + * 3. sig_pid sends SIGUSR1 to parent_pid, waking the supervisor + * (SCHED_FIFO 99), which immediately preempts sig_pid, aborts the + * wait with -ERESTARTSYS (-EINTR), removes kaddfd from + * knotif->addfd, and restores knotif->state = SECCOMP_NOTIFY_SENT + * before the tracee has executed a single instruction. + */ + sig_pid = fork(); + ASSERT_GE(sig_pid, 0); + if (sig_pid == 0) { + sched_setscheduler(0, SCHED_FIFO, &sp_sig_helper_fifo); + kill(parent_pid, SIGUSR1); + _exit(0); + } + + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1); + EXPECT_EQ(errno, EINTR); + EXPECT_EQ(waitpid(sig_pid, &status, 0), sig_pid); + + /* + * Restore normal scheduling and sleep briefly so the woken tracee + * runs in do_user_notif(). With knotif->state restored to + * SECCOMP_NOTIFY_SENT, the tracee must loop back to sleep waiting for + * the notification reply rather than returning 0 from __NR_getppid. + */ + sched_setscheduler(0, SCHED_OTHER, &sp_tracee_idle); + sched_setscheduler(pid, SCHED_OTHER, &sp_tracee_idle); + nanosleep(&delay, NULL); + + /* + * Retry SECCOMP_IOCTL_NOTIF_ADDFD. Because knotif->state is + * SECCOMP_NOTIFY_SENT, the retry succeeds (returns 42) instead of + * failing with -EINPROGRESS, installs FD 42 into the tracee, and wakes + * the tracee to complete the syscall with return value 42. + */ + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), 42); + + EXPECT_EQ(waitpid(pid, &status, 0), pid); + EXPECT_EQ(true, WIFEXITED(status)); + EXPECT_EQ(0, WEXITSTATUS(status)); + + close(listener); + close(memfd); +} + #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) -- 2.49.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 2026-09-20 7:20 ` [PATCH v3] " Hui Peng @ 2026-09-20 11:56 ` Bradley Morgan 0 siblings, 0 replies; 8+ messages in thread From: Bradley Morgan @ 2026-09-20 11:56 UTC (permalink / raw) To: benquike; +Cc: kees, linux-kernel, linux-kselftest, luto, shuah, wad On 20 September 2026 08:20:36 BST, Hui Peng <benquike@gmail.com> wrote: >In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set in >addfd.flags, knotif->state is optimistically transitioned from >SECCOMP_NOTIFY_SENT to SECCOMP_NOTIFY_REPLIED before waking the tracee >and waiting on kaddfd.completion: > > if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND) { > knotif->state = SECCOMP_NOTIFY_REPLIED; > ... > } > >If wait_for_completion_interruptible(&kaddfd.completion) is interrupted >by a signal before the tracee dequeues the kaddfd request >(!list_empty(&kaddfd.list)), seccomp_notify_addfd() removes kaddfd from >knotif->addfd and returns -EINTR to the supervisor without having >installed the file descriptor. > >However, knotif->state is left as SECCOMP_NOTIFY_REPLIED (with >knotif->error == 0 and knotif->val == 0). This causes two problems: > >1. When the tracee runs in do_user_notif(), it checks > if (knotif.state != SECCOMP_NOTIFY_REPLIED) after calling > seccomp_handle_addfd(). Because knotif->state is already > SECCOMP_NOTIFY_REPLIED, the tracee exits the notification wait loop > prematurely and returns 0 from the trapped syscall without the file > descriptor ever having been installed. > >2. If the supervisor retries SECCOMP_IOCTL_NOTIF_ADDFD or > SECCOMP_IOCTL_NOTIF_SEND before the tracee runs, it fails with > -EINPROGRESS because knotif->state is no longer SECCOMP_NOTIFY_SENT. > >Fix this by restoring knotif->state back to SECCOMP_NOTIFY_SENT if >SECCOMP_ADDFD_FLAG_SEND was set and kaddfd was not consumed before the >interrupted wait. Also add a seccomp_bpf selftest >(user_notification_addfd_send_interrupted) covering this race. > >Fixes: 0ae71c1990f3 ("seccomp: Add a flag to atomically addfd and perform send") Oh my godd you 1: removed me from CC 2: removed my R-B tag! What is your AI doing! NAK. I can't review something like this! Add a assisted by tag and CC me and I'll look at it >Signed-off-by: Hui Peng <benquike@gmail.com> >--- >Changes in v3: >- Actually include the tools/testing/selftests/seccomp/seccomp_bpf.c > regression test in the patch diff (v2 accidentally omitted the selftest > hunk), with detailed comments explaining the race and test setup. > >Changes in v2: >- Add user_notification_addfd_send_interrupted regression test to > tools/testing/selftests/seccomp/seccomp_bpf.c as requested by Kees Cook. > > kernel/seccomp.c | 2 + > tools/testing/selftests/seccomp/seccomp_bpf.c | 161 ++++++++++++++++++ > 2 files changed, 163 insertions(+) > >diff --git a/kernel/seccomp.c b/kernel/seccomp.c >index 86cf4460d69e..94c7ba80a8f7 100644 >--- a/kernel/seccomp.c >+++ b/kernel/seccomp.c >@@ -1808,10 +1808,13 @@ static long seccomp_notify_addfd(struct seccomp_filter *filter, > * We need to check again if the addfd request has been handled, > * and if not, we will remove it from the queue. > */ >- if (list_empty(&kaddfd.list)) >+ if (list_empty(&kaddfd.list)) { > ret = kaddfd.ret; >- else >+ } else { > list_del(&kaddfd.list); >+ if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND) >+ knotif->state = SECCOMP_NOTIFY_SENT; >+ } > > out_unlock: > mutex_unlock(&filter->notify_lock); >diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c >index 0622bc2acad4..e565728f9eb2 100644 >--- a/tools/testing/selftests/seccomp/seccomp_bpf.c >+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c >@@ -4368,6 +4368,173 @@ TEST(user_notification_addfd_rlimit) > close(memfd); > } > >+static void sigusr1_handler(int signo) >+{ >+} >+ >+/* >+ * Verify that when SECCOMP_IOCTL_NOTIF_ADDFD with SECCOMP_ADDFD_FLAG_SEND is >+ * interrupted by a signal before the tracee dequeues the addfd request, >+ * knotif->state is restored from SECCOMP_NOTIFY_REPLIED back to >+ * SECCOMP_NOTIFY_SENT so that: >+ * 1. The woken tracee sees knotif->state == SECCOMP_NOTIFY_SENT in >+ * do_user_notif() and goes back to sleep instead of prematurely >+ * returning 0 from the trapped syscall without the FD installed. >+ * 2. The supervisor can retry SECCOMP_IOCTL_NOTIF_ADDFD (or >+ * SECCOMP_IOCTL_NOTIF_SEND) instead of failing with -EINPROGRESS. >+ * >+ * To deterministically hit the race window where the supervisor sleeps in >+ * wait_for_completion_interruptible(&kaddfd.completion) after waking the >+ * tracee (complete(&knotif->ready)) but before the tracee runs >+ * seccomp_handle_addfd(), pin all processes to a single CPU and enforce a >+ * strict 3-tier scheduling priority hierarchy on that CPU: >+ * - Supervisor: SCHED_FIFO priority 99 (highest) >+ * - Signal helper (sig_pid): SCHED_FIFO priority 50 (middle) >+ * - Tracee (pid): SCHED_IDLE (lowest) >+ */ >+TEST(user_notification_addfd_send_interrupted) >+{ >+ /* >+ * Save parent_pid before user_notif_syscall(__NR_getppid, ...) installs >+ * the seccomp filter on the calling process; children inherit that >+ * filter, so sig_pid must not call getppid(). >+ */ >+ pid_t pid, sig_pid, parent_pid = getpid(); >+ long ret; >+ int status, listener, memfd; >+ struct seccomp_notif_addfd addfd = {}; >+ struct seccomp_notif req = {}; >+ struct sigaction sa = {}; >+ struct sched_param sp_tracee_idle = { .sched_priority = 0 }; >+ struct sched_param sp_supervisor_fifo = { .sched_priority = 99 }; >+ struct sched_param sp_sig_helper_fifo = { .sched_priority = 50 }; >+ struct timespec delay = { .tv_nsec = 15000000 }; >+ cpu_set_t cpuset; >+ int cpu; >+ >+ /* Pin the supervisor (and its future child processes) to one CPU. */ >+ cpu = sched_getcpu(); >+ if (cpu >= 0) { >+ CPU_ZERO(&cpuset); >+ CPU_SET(cpu, &cpuset); >+ sched_setaffinity(0, sizeof(cpuset), &cpuset); >+ } >+ >+ sa.sa_handler = sigusr1_handler; >+ ASSERT_EQ(sigaction(SIGUSR1, &sa, NULL), 0); >+ >+ memfd = memfd_create("test", 0); >+ ASSERT_GE(memfd, 0); >+ >+ 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!"); >+ } >+ >+ /* >+ * Follow the convention of other user_notification_* tests in this >+ * file by trapping __NR_getppid: because the filter is installed on >+ * the supervisor before fork(), the trapped syscall must be a >+ * side-effect-free syscall that the supervisor itself never invokes. >+ * Even though getppid() does not normally return an FD, >+ * SECCOMP_ADDFD_FLAG_SEND replaces the trapped syscall's return value >+ * with the newly installed FD number (42). >+ */ >+ listener = user_notif_syscall(__NR_getppid, >+ SECCOMP_FILTER_FLAG_NEW_LISTENER); >+ ASSERT_GE(listener, 0); >+ >+ pid = fork(); >+ ASSERT_GE(pid, 0); >+ >+ if (pid == 0) { >+ /* >+ * Tracee: invoke __NR_getppid as a dummy trigger syscall to >+ * trap into do_user_notif(). Verify that the syscall returns >+ * the injected FD number (42) and that FD 42 is open. >+ */ >+ ret = syscall(__NR_getppid); >+ exit(ret != 42 || fcntl(42, F_GETFD) < 0); >+ } >+ >+ /* Wait for the tracee to trap in do_user_notif(). */ >+ ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); >+ >+ /* >+ * Demote the tracee to SCHED_IDLE and promote the supervisor to >+ * SCHED_FIFO(99) on the same CPU. >+ */ >+ sched_setscheduler(pid, SCHED_IDLE, &sp_tracee_idle); >+ if (sched_setscheduler(0, SCHED_FIFO, &sp_supervisor_fifo) != 0) { >+ close(listener); >+ close(memfd); >+ kill(pid, SIGKILL); >+ waitpid(pid, NULL, 0); >+ SKIP(return, "SCHED_FIFO requires CAP_SYS_NICE"); >+ } >+ >+ addfd.srcfd = memfd; >+ addfd.newfd_flags = O_CLOEXEC; >+ addfd.newfd = 42; >+ addfd.id = req.id; >+ addfd.flags = SECCOMP_ADDFD_FLAG_SETFD | SECCOMP_ADDFD_FLAG_SEND; >+ >+ /* >+ * Fork a signal helper on the same CPU and set it to SCHED_FIFO(50). >+ * Because the supervisor is currently running at SCHED_FIFO(99) on >+ * this CPU, sig_pid is queued on the runqueue but cannot run until the >+ * supervisor blocks inside the kernel. >+ * >+ * When the supervisor invokes ioctl(SECCOMP_IOCTL_NOTIF_ADDFD) below: >+ * 1. seccomp_notify_addfd() sets knotif->state = SECCOMP_NOTIFY_REPLIED, >+ * wakes the tracee (SCHED_IDLE), and blocks in >+ * wait_for_completion_interruptible(&kaddfd.completion). >+ * 2. The CPU scheduler immediately runs sig_pid (SCHED_FIFO 50) >+ * ahead of the woken tracee (SCHED_IDLE). >+ * 3. sig_pid sends SIGUSR1 to parent_pid, waking the supervisor >+ * (SCHED_FIFO 99), which immediately preempts sig_pid, aborts the >+ * wait with -ERESTARTSYS (-EINTR), removes kaddfd from >+ * knotif->addfd, and restores knotif->state = SECCOMP_NOTIFY_SENT >+ * before the tracee has executed a single instruction. >+ */ >+ sig_pid = fork(); >+ ASSERT_GE(sig_pid, 0); >+ if (sig_pid == 0) { >+ sched_setscheduler(0, SCHED_FIFO, &sp_sig_helper_fifo); >+ kill(parent_pid, SIGUSR1); >+ _exit(0); >+ } >+ >+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1); >+ EXPECT_EQ(errno, EINTR); >+ EXPECT_EQ(waitpid(sig_pid, &status, 0), sig_pid); >+ >+ /* >+ * Restore normal scheduling and sleep briefly so the woken tracee >+ * runs in do_user_notif(). With knotif->state restored to >+ * SECCOMP_NOTIFY_SENT, the tracee must loop back to sleep waiting for >+ * the notification reply rather than returning 0 from __NR_getppid. >+ */ >+ sched_setscheduler(0, SCHED_OTHER, &sp_tracee_idle); >+ sched_setscheduler(pid, SCHED_OTHER, &sp_tracee_idle); >+ nanosleep(&delay, NULL); >+ >+ /* >+ * Retry SECCOMP_IOCTL_NOTIF_ADDFD. Because knotif->state is >+ * SECCOMP_NOTIFY_SENT, the retry succeeds (returns 42) instead of >+ * failing with -EINPROGRESS, installs FD 42 into the tracee, and wakes >+ * the tracee to complete the syscall with return value 42. >+ */ >+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), 42); >+ >+ EXPECT_EQ(waitpid(pid, &status, 0), pid); >+ EXPECT_EQ(true, WIFEXITED(status)); >+ EXPECT_EQ(0, WEXITSTATUS(status)); >+ >+ close(listener); >+ close(memfd); >+} >+ > #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) > --- Thanks! "I'm not a very positive person" - Linus torvalds ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 2026-09-20 3:50 ` Kees Cook 2026-09-20 4:16 ` [PATCH v2] " Hui Peng 2026-09-20 7:20 ` [PATCH v3] " Hui Peng @ 2026-09-20 10:10 ` Lorenzo Stoakes (ARM) 2026-09-20 11:55 ` Bradley Morgan 2 siblings, 1 reply; 8+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-09-20 10:10 UTC (permalink / raw) To: Kees Cook; +Cc: Hui Peng, Andy Lutomirski, Will Drewry, linux-kernel On Sat, Sep 19, 2026 at 08:50:57PM -0700, Kees Cook wrote: > On Sat, Sep 19, 2026 at 08:35:14PM +0000, Hui Peng wrote: > > In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set, > > knotif->state is transitioned from SECCOMP_NOTIFY_SENT to > > SECCOMP_NOTIFY_REPLIED before dropping filter->notify_lock and waiting > > in wait_for_completion_interruptible(&kaddfd.completion). > > > > If wait_for_completion_interruptible() is interrupted by a signal before > > the target task processes the kaddfd entry, seccomp_notify_addfd() > > removes kaddfd.list from knotif->addfd via list_del(&kaddfd.list), but > > leaves knotif->state set to SECCOMP_NOTIFY_REPLIED. > > Are you able to construct a regression test for this so we can catch > this race if it ever gets exposed again? I'd disregard this guy and treat this as, at best, an AI-generated bug report. He sent 115 patches across 40 subsystems in 2 hours. https://lore.kernel.org/all/?q=f%3Ahui+peng Unfortunately I feel we are only at the beginning of this kind of thing :( > > -- > Kees Cook > -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 2026-09-20 10:10 ` [PATCH] " Lorenzo Stoakes (ARM) @ 2026-09-20 11:55 ` Bradley Morgan 0 siblings, 0 replies; 8+ messages in thread From: Bradley Morgan @ 2026-09-20 11:55 UTC (permalink / raw) To: ljs; +Cc: benquike, kees, linux-kernel, luto, wad On 20 September 2026 11:10:45 BST, "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote: >On Sat, Sep 19, 2026 at 08:50:57PM -0700, Kees Cook wrote: >> On Sat, Sep 19, 2026 at 08:35:14PM +0000, Hui Peng wrote: >> > In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set, >> > knotif->state is transitioned from SECCOMP_NOTIFY_SENT to >> > SECCOMP_NOTIFY_REPLIED before dropping filter->notify_lock and waiting >> > in wait_for_completion_interruptible(&kaddfd.completion). >> > >> > If wait_for_completion_interruptible() is interrupted by a signal >before >> > the target task processes the kaddfd entry, seccomp_notify_addfd() >> > removes kaddfd.list from knotif->addfd via list_del(&kaddfd.list), but >> > leaves knotif->state set to SECCOMP_NOTIFY_REPLIED. >> >> Are you able to construct a regression test for this so we can catch >> this race if it ever gets exposed again? > >I'd disregard this guy and treat this as, at best, an AI-generated bug >report. > >He sent 115 patches across 40 subsystems in 2 hours. > >https://lore.kernel.org/all/?q=f%3Ahui+peng > >Unfortunately I feel we are only at the beginning of this kind of thing :( Bruh, wasted my time on review. Mad.. I do believe it LGTM though, thanks Lorenzo for the tip. > >> >> -- >> Kees Cook >> > >-- >Cheers, Lorenzo > > --- Thanks! "I'm not a very positive person" - Linus torvalds ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-20 11:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-19 20:35 [PATCH] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted Hui Peng 2026-09-19 22:22 ` Bradley Morgan 2026-09-20 3:50 ` Kees Cook 2026-09-20 4:16 ` [PATCH v2] " Hui Peng 2026-09-20 7:20 ` [PATCH v3] " Hui Peng 2026-09-20 11:56 ` Bradley Morgan 2026-09-20 10:10 ` [PATCH] " Lorenzo Stoakes (ARM) 2026-09-20 11:55 ` Bradley Morgan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®