chroot: fix chdir to short paths, and the resolver bugs behind it - #193
Merged
Conversation
host_to_virtual mapped a host path back to its virtual path by joining
the remainder of the mount source onto the mount's virtual base, but a
child sitting exactly on the mount point leaves an empty remainder, and
PathBuf::join("") appends a separator. getcwd then copied "/proc/" into
the child instead of "/proc". Path comparison ignores a trailing
separator, so nothing caught this internally; only the bytes handed to
the child differ.
Signed-off-by: Cong Wang <cwang@multikernel.io>
The supervisor cannot chdir a child on its behalf: only the kernel can update the calling task's fs_struct. The chroot handler works around that by rewriting the child's path argument to /proc/self/fd/N, which is what makes issue #178's short paths fail. Replacing that rewrite means the supervisor has to know where each task believes it is, so add the storage first. The cell hangs off ProcessIndex rather than PerProcessState because path resolution reads it from synchronous helpers, and it is shared behind an Arc the way the kernel shares fs_struct: a thread joins its leader's cell so a sibling's chdir is visible, while anything else copies its parent's value at registration the way fork(2) does. Seeding a child needs the parent pid, so notif.rs's private read_ppid moves in beside the other /proc identity readers rather than being duplicated. Signed-off-by: Cong Wang <cwang@multikernel.io>
A chdir to a short absolute path failed with ENAMETOOLONG under chroot: "/proc/self/fd/N" is 16 bytes and the handler could only redirect the child through it by overwriting the path buffer the child had already passed, so anything shorter than 15 characters was refused. That hit every common mount point (cd /root, cd /tmp, cd /workspace) and the virtual root itself, while ls and open on the same paths worked, and it broke mkdir -p, which chdirs through each component. Reported as #178. The rewrite cannot be made to fit: seccomp-notify cannot change the syscall arguments, so the replacement has to live in the child's own buffer. Resolve the target on-behalf and record the result instead, and let every AT_FDCWD resolution, getcwd, and the /proc virtualization read that notion rather than /proc/<pid>/cwd, which now stays where exec left it. fchdir moves the real cwd without passing a path, so it joins the notified set to keep the tracked value in step. Dropping the rewrite also drops what it cost: the TOCTOU window where the kernel re-read a path we had just written (issue #27), and a force-write through /proc/<pid>/mem that punched past page protections and left a .rodata path literal permanently corrupted in the child. Signed-off-by: Cong Wang <cwang@multikernel.io>
The module's Continue-safety notes still described a chdir that rewrote the child's path argument, which no longer exists, and the surviving categories were justified only against the TOCTOU race. They now rest on a second fact worth writing down: because the supervisor services chdir without moving the child's own cwd, that cwd is frozen at exec and diverges from the sandbox's view as soon as anything chdirs, so handing a healthy path syscall back to the kernel resolves it against the wrong root or the wrong directory. Every existing Continue was checked against that rule and none breaks it: each is either fd-based, where no path is resolved (AT_EMPTY_PATH stat, getdents, fchdir), or reached only after a fault that makes the kernel fail identically. This is the note that keeps the next handler from adding a third kind. Drop the claim that the execve rewrite's race would be closed by an opt-in CLONE_THREAD deny. No such plan exists, and it could not work: threads are ordinary and a sandbox that refuses them is unusable, so Landlock is the bound and the note should not promise otherwise. Signed-off-by: Cong Wang <cwang@multikernel.io>
openat2 was trapped so it could not bypass the fs deny check, but it was
never registered with the chroot handlers, so under a virtual root it ran
as written: an absolute path resolved against the host root rather than
the rootfs, which is why openat2("/etc/passwd") read the host's file or,
more often, failed with ENOENT for a file that plainly exists in the
image. Relative paths were resolved against the child's real cwd, which
no longer moves now that the supervisor services chdir itself.
The handler could not simply be registered for it: openat2 keeps flags,
mode and resolve in a struct open_how in child memory, so args[2] is a
pointer where openat has flags. Decode through decode_open_args, which
already knows all three spellings for the deny path, and drop the
synthesized notification the legacy open handler used to build, since
that shape now conflicts with decoding by syscall number.
A caller's RESOLVE_* request has to survive being serviced on its behalf:
NO_SYMLINKS and NO_MAGICLINKS constrain the path itself, so they are
re-checked against the path as written (resolution for the policy check
follows symlinks and would otherwise satisfy them silently) and passed to
the on-behalf open. BENEATH, IN_ROOT and NO_XDEV are relative to the
child's own dirfd, which the supervisor cannot replay from the sandbox
root, so they are dropped rather than misapplied; RESOLVE_IN_ROOT still
bounds the walk.
Signed-off-by: Cong Wang <cwang@multikernel.io>
/proc/self/cwd is the kernel's own view of a cwd the supervisor stopped moving, so it reported wherever exec left the child: readlink returned the launch directory, a host path outside the virtual root, and opening through the link failed with EACCES because that path resolves under no mount the sandbox knows. The readlink handler was worse off than the rest. It never canonicalized "self" to the caller's pid, and since sandlock services /proc through an on-behalf openat2, /proc/self there is the *supervisor's* own directory, not the child's. That predates the cwd work and would have leaked the supervisor's cwd whenever the two diverged. Canonicalize first, like build_virtual_path already did for every other handler. Reads of the link now come from the tracked cwd, and paths that resolve through it are rewritten before resolution so open, stat and getdents land in the same directory the link reports. Where nothing is tracked and the real cwd maps to nothing the sandbox can name, the answer is "/" rather than the host path behind it, matching what getcwd already does. Signed-off-by: Cong Wang <cwang@multikernel.io>
/proc/<pid>/fd/N is a magic link, so what readlink hands back is a real host path the kernel synthesized rather than link text. The handler ran that through host_to_virtual and, when the file lay outside the virtual root and every mount, fell back to printing it verbatim. A child could read the host location of anything it had inherited: redirect sandlock's output to a file and the sandbox learns exactly where that file lives. Ordinary symlinks must keep that same fallback, since read_link returns their stored text and a link to /etc/foo genuinely reads "/etc/foo", so the fix is scoped to the magic link rather than applied to the tail. Targets the sandbox can name still resolve to their virtual path, the kernel's own pipe:[…] and socket:[…] spellings pass through untouched, and a file with no name in the sandbox is reported as file:[inode], in the same shape, so callers telling one stream from another still work. Signed-off-by: Cong Wang <cwang@multikernel.io>
The chroot resolver walks a path with the kernel following symlinks, which is what open wants and the opposite of what several other syscalls need. Every one of them was handed the already-resolved target and acted on it: rm of a symlink deleted the file it pointed at and left the dangling link behind, mv moved the target, lstat described the target's type and size, lchown owned the target, and the l-prefixed xattr calls read the target's attributes. Only readlink was right, and only because it open-coded a parent walk of its own. Resolve the parent and append the final component instead, which is that open-coded walk lifted into resolve_in_root_nofollow and shared. It was already sitting inside resolve_in_root as the fallback for an O_CREAT target that does not exist yet: a name that cannot be walked to and a name that must not be walked through want the same answer. unlinkat and renameat2 take it unconditionally, linkat unless the caller passes AT_SYMLINK_FOLLOW, and the stat, statx, utimensat, fchownat and xattr paths when the caller asked not to follow. fchownat also has to call lchown rather than chown once it gets there, or the resolution stops at the link and the call walks past it anyway. Signed-off-by: Cong Wang <cwang@multikernel.io>
The /proc open path refuses a pid outside the sandbox, so a child cannot open /proc/<host pid>/cwd, but readlink of the same path went through the chroot handler, which had no such check. The supervisor answers on the child's behalf and sees the whole host process table, so the magic links reported real cwd, exe and fd targets for processes the sandbox is not supposed to know exist. Apply the same extract_proc_pid + membership test the open path uses, rather than a second notion of which pids are visible. Signed-off-by: Cong Wang <cwang@multikernel.io>
congwang-mk
force-pushed
the
chdir-virtual-cwd
branch
from
August 7, 2026 18:42
99ef440 to
1003321
Compare
Two failures that only show up off x86_64, both older than this branch and both uncovered by tests added here. stat_and_write hand-packed the reply in x86_64 field order: st_nlink as a 64-bit field ahead of st_mode. aarch64 and riscv64 put st_mode and st_nlink first as 32-bit fields, so a child there read st_nlink's low half as its mode and got 1, with the file type bits missing entirely. st_size lands at the same offset on all three, which is why the existing stat tests, which assert on size, stayed green. Let libc lay the struct out and copy it whole, the way the statfs handler already does. rename went unmediated on aarch64 for a simpler reason: the chroot notif list carries renameat2 and the legacy rename, but not renameat, and libc's rename() compiles to renameat wherever the ABI dropped the plain call. An absolute path therefore reached the kernel as written and resolved against the host root, so mv inside a rootfs failed with ENOENT. renameat carries renameat2's argument slots minus the flags this handler never reads, so it routes to the same handler. Signed-off-by: Cong Wang <cwang@multikernel.io>
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
1 participant
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 #178, plus a set of chroot correctness bugs the fix surfaced along
the way. Every commit stands alone and carries its own tests.
The reported bug
Under
chroot=+fs_mount=,chdirto a short absolute path failed withENAMETOOLONGwhilelsandopenon the same path worked://tmp/workspace/root/bbbbbbbbbbbbbbbbbbbbpwdshowed/proc/self/fd/5chdircannot be run on behalf of a child: only the kernel updates thecalling task's
fs_struct. The old handler worked around that by injectinga dirfd and overwriting the child's own path buffer with
/proc/self/fd/N,then letting the kernel run that. Sixteen bytes do not fit the buffer behind
/tmp, and nothing fits behind/. The reporter'spwdartifact was thesame write seen from the other side: the shell read back its own corrupted
string.
The rewrite cannot be made to fit, since seccomp-notify cannot change syscall
arguments, so the supervisor now tracks the cwd instead. Dropping the rewrite
also drops what it cost: the TOCTOU window where the kernel re-read a path we
had just written (#27), and a force-write through
/proc/<pid>/memthatpunched past page protections and left
.rodatapath literals permanentlycorrupted.
The tracked cwd is shared the way the kernel shares
fs_struct: threads jointheir leader's cell, forks copy it.
fchdirjoins the notified set so itcannot desync, and
getcwd, everyAT_FDCWDresolution, the/procvirtualization and
/proc/<pid>/cwdall read the tracked value.Bugs found while verifying that
Each of these predates this branch and is independent of
chdir.rmof a symlink deleted the target. The resolver follows the finalcomponent, which is right for
openand wrong for the no-follow family, sounlinkatunlinked the target and left the dangling link.renameat2movedthe target,
lstatdescribed it,lchownowned it, and thel*xattrcallsread its attributes. Only
readlinkwas correct, and only because itopen-coded its own parent walk. That walk is now shared as
resolve_in_root_nofollow.openat2was never mediated under chroot. It was trapped for the denycheck but not registered with the chroot handlers, so an absolute path
resolved against the host root:
openat2("/etc/passwd")read the host's fileor failed with ENOENT for a file plainly in the image. It could not simply be
registered, because
openat2keeps flags in astruct open_howin childmemory where
openathas them in a register.RESOLVE_NO_SYMLINKSandNO_MAGICLINKSare re-checked against the path as written and passed through;BENEATH,IN_ROOTandNO_XDEVare relative to the child's own dirfd,which the supervisor cannot replay from the sandbox root, so they are dropped
rather than misapplied.
/proc/self/cwdand/proc/<pid>/fd/Nleaked host paths.readlinknever canonicalized
selfto the caller's pid, and since sandlock services/procthrough an on-behalfopenat2,/proc/selfthere was thesupervisor's own process. Unmappable magic-link targets were also printed
verbatim, so a child could read the host location of anything it inherited.
Those now report the tracked cwd, the virtual path, or
file:[inode]in thekernel's own
pipe:[…]style.readlinkof a foreign pid was ungated. The/procopen path refuses apid outside the sandbox;
readlinkof the same path did not, and thesupervisor sees the whole host process table.
getcwdreported/proc/for a child sitting exactly on a mount point,because
PathBuf::join("")appends a separator.Testing
710 lib, 413 integration, 398 Python, all passing. Seventeen new tests cover
the short paths, the virtual root, relative resolution after
chdirandfchdir, thread and fork inheritance of the cwd, theopenat2paths andRESOLVE_NO_SYMLINKS, the magic links, the host-path leaks, and the symlinkno-follow family including the destructive
rmcase.Known gaps, deliberately left
virtual_cwdand its own copy of the16-byte limit, narrowed to directories that exist only in the upper layer.
RESOLVE_BENEATHfidelity foropenat2, as above.It is metadata only; nothing on the restore path chdirs to it.
lstaton a/procmagic link reports the target's type, since pathsthrough the link are rewritten before resolution.