seccomp: keep close and fork out of the supervisor so signals cannot fail them - #241
Open
congwang-mk wants to merge 4 commits into
Open
congwang-mk wants to merge 4 commits into
congwang-mk wants to merge 4 commits into
Conversation
close() was on the notification list only so the netlink cookie set could drop a (pid, fd) entry when the child closed the socket. A trapped syscall that a signal interrupts before the supervisor receives it fails with EINTR without having run, and close() is the one call nobody retries, because on Linux it always releases the fd. Here it had not: a shell running `a | b | c` leaked a pipe's write end into the last stage, which then never saw EOF (issue #235). Nothing in userspace closes that window, so close() has to stay out of the supervisor's hands. The tracking was incomplete anyway. dup2() over the slot, close_range() and O_CLOEXEC across an exec all empty or refill it without a close(), leaving an entry that made bind() and getsockname() on whatever socket landed there next answer as if it were ours. Record the injected socket's SO_COOKIE with the entry and check on lookup, through pidfd_getfd(), that the slot still holds that socket, dropping the entry when it does not. The cookie rather than the inode number: socket inodes come from a 32-bit counter shared with pipes that wraps without checking for live collisions, so a sandboxed process could steer a new socket onto a recorded number, while a socket cookie is 64 bits and never reused. The check runs only for fds that are in the set. Entries of a process are dropped when it exits. Signed-off-by: Cong Wang <cwang@multikernel.io>
clone(CLONE_NEWUSER) was refused and clone3(CLONE_NEWUSER) was not (issue #240). The BPF arg filter checks clone's flags in its first argument, but clone3 keeps them in a struct in user memory, which BPF cannot read, and the fork handler only ever looked at clone's. A sandboxed process could create a user namespace, and from inside it the others. Having the supervisor read the struct would not fix it: the handler answers Continue, the kernel reads the struct again, and a CLONE_VM peer can rewrite the flags in between. Answer clone3 with ENOSYS from the filter instead, which is what glibc and Rust's std expect from a kernel without it: both fall back to clone, whose flags the filter can check. musl and Go do not use clone3. The arg filter block runs ahead of the notification checks, so nothing can route around it. clone3 leaves the notification list with this, and the racy read of its flags that thread accounting relied on goes away with it. Signed-off-by: Cong Wang <cwang@multikernel.io>
freeze() set a flag that made the fork handler park every fork notification unanswered until thaw(), so that no process could appear while the group was being stopped. The kernel already guarantees that: copy_process() collects a signal sent to several processes during a fork and delivers it as if it came after the fork, restarting the fork with the signal pending, so a SIGSTOP to the group reaches the child too. The hold was also never released. thaw() cleared the list of parked notification ids without answering them, so a process that forked during a freeze stayed blocked in that fork for good. It was one of the reasons fork had to be intercepted in every sandbox. With it gone, freeze() and thaw() are the group signals and nothing else. Signed-off-by: Cong Wang <cwang@multikernel.io>
fork, vfork and clone were on the notification list of every sandbox. A trapped syscall that a signal interrupts before the supervisor has received it fails with EINTR without having run, and these three never fail that way natively (the kernel restarts them unconditionally), so nothing retries them: dash reports "Cannot fork" and gives up, pthread_create fails (issue #235). Nothing in userspace closes that window, so the syscalls have to stay out of the supervisor's hands wherever they can. What interception was for, and what each needs now: - the process limit: only when max_processes is set, already opt-in; - registering a child before it runs, for argv decisions: only with a policy_fn or an exec handler; - refusing namespace flags: the BPF arg filter does that, ahead of the notification checks, and clone3 is refused outright; - holding forks during a checkpoint freeze: gone, the kernel covers it; - the running count in the virtual /proc/loadavg: taken from the process index, where every process registers on its first notified syscall, which the dynamic loader's opens make immediate. exit and exit_group were notified only so an exit could release a process slot, so they follow the same rule. Thread creation no longer takes a round trip through the supervisor either. With the default policy, 1000 three-stage pipelines in dash complete with no failure, where main completes none of ten runs of 100. With a process limit set the race is still there, about one "Cannot fork" per 1000 pipelines. resource_peaks() reports a process peak of 1 when fork is not supervised; learn sets a policy_fn, so its profiles are unaffected. Signed-off-by: Cong Wang <cwang@multikernel.io>
congwang-mk
force-pushed
the
notif-eager-recv
branch
from
September 20, 2026 04:00
811928c to
a08a094
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #240. Fixes #235, except for a rare race that remains when fork is supervised and that only a kernel change can close (see "Known remaining race").
Problem
A syscall routed to
SECCOMP_RET_USER_NOTIFsleeps interruptibly until the supervisor has received the notification;SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECVonly protects it after that. If a signal whose handler lacksSA_RESTARTlands first, the syscall fails with EINTR without having run, including syscalls that never return EINTR natively. dash's SIGCHLD handler is one such handler:close()leaked a pipe's write end into the last stage ofa | b | c, which never saw EOF: the pipeline hung;fork()made dash print "Cannot fork".Nothing in userspace closes that window, so the fix is to keep the syscalls nobody retries out of the supervisor's hands.
Release builds, runs of 100 three-stage dash pipelines:
--max-processes 64(20 runs)Commits
(pid, fd)entries of the netlink cookie set. The set now records the injected socket'sSO_COOKIEand checks the slot throughpidfd_getfd()on lookup. Not the inode number: those come from a 32-bit counter shared with pipes that wraps without checking live collisions. This also fixes stale entries left bydup2,close_rangeandO_CLOEXECon exec.clone3carriedCLONE_NEWUSERpast the ban that stopsclone, because its flags are in a struct BPF cannot read. A supervisor-side check would race aCLONE_VMpeer rewriting the struct. glibc and Rust's std fall back tocloneon ENOSYS; the arg filter block runs ahead of the notification checks, so nothing routes around it.copy_process()makes a signal sent to a group appear after any fork it overlaps. Also broken:thaw()cleared the held ids without answering them, so a process that forked during a freeze hung forever.clone/fork/vfork/exit/exit_groupare notified only withmax_processes, apolicy_fn, or an exec handler. The namespace-flag check is the BPF filter's, and the running count of the virtual/proc/loadavgcomes from the process index.An earlier version of this PR also received notifications on a dedicated thread, to shorten the exposed window. It is dropped: with
closeandforkout of the way it made no measurable difference to the numbers above, while it made every queued task unstoppable until answered (which broke checkpoint and the execve argv freeze), cost a thread per sandbox and about 10 us per notified syscall.Behavior changes
clone3returns ENOSYS in every sandbox.resource_peaks()reports a process peak of 1 when fork is not supervised.learnsets apolicy_fn, so its profiles are unaffected.fork()andclose()cost native time again, since neither round-trips through the supervisor.Known remaining race
Accepted as is; it cannot be eliminated from userspace.
max_processes, apolicy_fn, or an exec handler. The default policy is not affected.SA_RESTART(dash's SIGCHLD) lands in the few microseconds betweenfork()entering the kernel and the supervisor receiving its notification.fork()then fails with EINTR, which it never returns natively, and dash prints "Cannot fork".close(), which is no longer notified in any mode.SECCOMP_IOCTL_NOTIF_RECV, andSECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECVonly takes over after that. Receiving sooner does not help measurably: a build of this branch that received notifications on a dedicated thread scored the same (17 of 20 runs against 17 of 20), so that approach was dropped, together with its costs. Enforcing a process limit needs the supervisor to see each fork.-ERESTARTNOINTRinstead of-ERESTARTSYS, or make the wait killable from the start, most likely behind a new filter flag.Other notified syscalls (
openat, ...) have the same window in principle. I could not provoke a failure (0 of 2000 shell redirections), but that test may not be sensitive to it.Tests
fork()completes while the supervisor is busy with a slow handler; fails once a process limit is added.clone3with and withoutCLONE_NEWUSERgets ENOSYS,clone(CLONE_NEWUSER)EPERM,subprocessstill works.closenever notified.Commandall run withclone3refused;sandlock learnwith a pipeline works.sandlock-core777 lib and 447 integration tests pass,learn_test40 pass. Python and Go suites were not run locally.test_controltests fail intermittently here; main fails them at the same rate (5 of 12 loops of its integration binary), so they are not from this branch.Touches
netlink/handlers.rsnear the change in #234; whichever lands second needs a small rebase.🤖 Generated with Claude Code