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: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20260919203514.2580997-1-benquike@gmail.com>
[not found] ` <202609192050.D1E0CD96@keescook>
2026-09-20 4:16 ` [PATCH v2] seccomp: restore knotif->state when SECCOMP_ADDFD_FLAG_SEND is interrupted 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-22 2:32 ` [PATCH v5] " Hui Peng
2026-09-22 4:31 ` Kees Cook
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.