From: Kees Cook <kees@kernel.org>
To: Hui Peng <benquike@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
Will Drewry <wad@chromium.org>, Shuah Khan <shuah@kernel.org>,
Bradley Morgan <brads@mainlining.org>,
Lorenzo Stoakes <ljs@kernel.org>,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH v4] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted
Date: Mon, 21 Sep 2026 02:19:38 -0700 [thread overview]
Message-ID: <202609210158.F3D96F157@keescook> (raw)
In-Reply-To: <20260920185910.3307638-1-benquike@gmail.com>
On Sun, Sep 20, 2026 at 06:59:10PM +0000, Hui Peng wrote:
> 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.
Yeah, this fix is the same restoration logic as seccomp_notify_recv()
uses (although that may actually need "if (state == SECCOMP_NOTIFY_SENT) ..."
added). It does make it clear there is a missing state in the state
machine. SECCOMP_NOTIFY_REPLIES means both "A reply is reserved" and
"A reply has been delivered". But the hidden state is maintained:
"REPLIED + queued ADDFD+SEND" == in flight, and "REPLIED + no queued
addfd" == delivered.
Thank you for the selftest addition, the sched trick is nice!
> Tested in QEMU against Linux 7.3.0-rc3 by exercising kernel/seccomp.c
> and verifying the fix with KASAN enabled.
I've verified this now too. It's kind of a ugly problem because unlucky
timing makes it look like the returned fd is fd 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);
Unchecked return value?
> + 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");
> + }
Instead of this you may want to look at FIXTURE_TEARDOWN to clean up
(though it's not strictly needed since the harness is run in a
subprocess so all these go away on test exit). The one thing that might
be worth doing is making sure sched_setscheduler(0, SCHED_OTHER, ...)
happens ASAP or the test could block everything on a single CPU
machine/VM/CI.
> [...]
> + sig_pid = fork();
> + ASSERT_GE(sig_pid, 0);
> + if (sig_pid == 0) {
> + sched_setscheduler(0, SCHED_FIFO, &sp_sig_helper_fifo);
Missed return value check here too.
> + 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);
Need to check these too...
> + 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
Thank you for the test, it really helps see the shape of the issue.
-Kees
--
Kees Cook
next prev parent reply other threads:[~2026-09-21 9:19 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-19 20:35 [PATCH] " 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 18:59 ` [PATCH v4] " Hui Peng
2026-09-21 9:19 ` Kees Cook [this message]
2026-09-20 10:10 ` [PATCH] " Lorenzo Stoakes (ARM)
2026-09-20 11:55 ` Bradley Morgan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202609210158.F3D96F157@keescook \
--to=kees@kernel.org \
--cc=benquike@gmail.com \
--cc=brads@mainlining.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=ljs@kernel.org \
--cc=luto@amacapital.net \
--cc=shuah@kernel.org \
--cc=stable@vger.kernel.org \
--cc=wad@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®