Translate guest paths through the sysroot sidecar - #242
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
8047c9c to
a24adb1
Compare
c159700 to
e446b7b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
de568de to
bb66e0a
Compare
bb66e0a to
ee35b0d
Compare
ee35b0d to
6a7cc5a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
63051f2 to
24c8a4a
Compare
Consolidate the sysroot path-translation handlers so guest-visible paths are reconstructed and reverse-mapped consistently across syscalls. Two guest-visible results change with that consolidation. A bind(2) whose address carries a '/' resolves the parent and reattaches the leaf verbatim, so the leaf never reached the reserved-name guard: binding a sidecar bookkeeping name collided with the real index file and reported EADDRINUSE for a name that stat(2) and open(O_CREAT) both report as ENOENT. It now agrees with them. The abstract-socket exit sweep unlinked every entry in the shared namespace directory, destroying backing files a forked child still had bound and listening, so it now retires only the untracked shortening symlinks; any participant may remove the directory once it empties, which also keeps a namespace created by a forked child from leaking. Path classification normalizes once per lookup and hands the same spelling to both the private-prefix and the guest-system-path rules, which previously ran two different normalizers over the same input and had to be kept in agreement by hand. Directory snapshots hold the sidecar index open across a readdir pass instead of reloading and reparsing it for every entry, which cost one file read per tokenized name. Opening the mapping reports an unreadable index separately from a directory that simply has none, because the two must not be treated alike: an index that cannot be read fails the snapshot so the watch keeps its previous baseline, while a directory without one maps every name to itself. Collapsing them would drop every mapped child from the listing, which the diff would then report as deletions of files that still exist. Both short-lived descriptors these paths open now carry O_CLOEXEC, so a concurrent execve on another vCPU thread cannot leak one into the new image while it is held.
Cover the sysroot translation model end to end: - Add the sysroot path-translation matrix test and the exec-cwd regression test. - Run the path matrix on a case-sensitive volume and validate it against a real kernel. - Cover guest-private prefix classification, execveat /dev/shm nofollow, the absock namespace lifecycle, and PT_INTERP tokenized-parent fallback. - Add the sidecar-state check helper. - Retire the listxattr indirect-coverage exemption, since the matrix now calls the syscall directly and the coverage audit rejects exemptions that no longer apply. The absock lifecycle is covered in both exit orders, since the two destroy different state: the existing case has a forked child exit first, and an owner-sweep mode has the namespace owner exit while a child still holds a bound abstract socket. The owner cannot report that result itself, so the surviving child prints a marker the recipe greps for. Reverse mapping is pinned at its bounds with a maximum-length component and at the sysroot root, the /dev/shm nofollow and nonblock fences are asserted through truncate and chdir, which reach a leaf by descriptor, and a tokenized directory holding many entries checks that one create still yields exactly one named event. Shared file helpers move to tests/test-util.h so the three copies that had drifted cannot diverge again; the promoted copy retries short writes, which two of them did not. Two lanes cover the index rules that a guest cannot set up for itself, so the host stages the state. test-sidecar-orphan places a token-shaped file with no index row beside an ordinary host-staged file and requires both to be listed and both to resolve, which separates passing an unmapped name through from hiding whatever the index does not name. test-inotify-index-fail makes a watched directory's index unreadable between the baseline and the next snapshot and requires that no child is announced as deleted; the guest prints a readiness marker and waits on a host-staged sentinel, because the failure has to begin after the baseline is taken and a guest-created sentinel would itself be stored under a token the recipe could not name.
Describe how guest paths are translated through the sysroot and sidecar, including AF_UNIX socket addresses, and link it from the internals and usage docs. Record the parts of the model a reader would otherwise have to trace the code to recover: which leg of the pipeline path_host_to_guest inverts and which legs it leaves untouched, why a bind reattaching its leaf must apply the reserved-name guard itself, and how the abstract-socket namespace directory is retired across a forked guest tree. The /dev/shm section gains the open-flag half of the never-follow invariant, covering the helper that forces those flags and the reason every open of a shm leaf is also nonblocking. State the two rules that follow from the index being a mapping rather than a gate: a name the index does not describe keeps its on-disk spelling, because lookup falls back to that spelling and a listing must agree with what can be opened; and an index that exists but cannot be read fails the operation instead of reading as empty, so a watch keeps its baseline rather than reporting every mapped child as deleted. Record the two lanes that hold those rules.
24c8a4a to
68886eb
Compare
jserv
left a comment
jserv
left a comment
There was a problem hiding this comment.
Review of the sysroot path-translation consolidation. Findings below are inline on the relevant lines. Focus was correctness and the never-follow / no-host-path-leak invariants the PR sets out to enforce.
Out of scope but worth noting: in sys_fchownat the post-chown verification stat uses fstatat(dir_ref.fd, ...) while the before-stat and the chown use host_dirfd (path_translation_dirfd). These only diverge on a /dev/shm redirect, where the path is absolute and dirfd is ignored, so it is benign today. This code predates this PR; flagging it because the fakeroot race check would inspect the wrong descriptor if the dirfd helper ever changes.
| * concurrent execve on another vCPU thread inside that window would | ||
| * otherwise leak it into the new image. | ||
| */ | ||
| int tmp_fd = openat(path_translation_dirfd(&tx, &dir_ref), tx.host_path, |
There was a problem hiding this comment.
execveat resolves the target with F_GETPATH here and passes only the host backing path into sys_execve (exec.c:433), which rebuilds the guest identity via path_host_to_guest(). That function cannot invert the /dev/shm per-UID backing dir or a FUSE-materialized copy, so execveat(dirfd, "/dev/shm/foo", ...) (or a FUSE-backed binary) makes /proc/self/exe report the private host path -- the exact leak this PR exists to prevent. Suggest carrying the guest spelling forward (join tx.guest_path against the dirfd/cwd to an absolute guest path) and passing that as the exec identity, so non-invertible legs keep the guest name.
| * access to the file contents. | ||
| */ | ||
| int host_fd = open(path, O_EVTONLY); | ||
| int host_fd = open(tx.host_path, O_EVTONLY); |
There was a problem hiding this comment.
This opens the translated leaf with open(tx.host_path, O_EVTONLY) and never applies path_translation_oflags(&tx, ...), so a guest symlink under /dev/shm is followed out of the namespace and an unrelated host file gets watched -- breaking the never-follow invariant the rest of the PR enforces. Suggest open(tx.host_path, path_translation_oflags(&tx, O_EVTONLY)).
| const char *guest, | ||
| bool temp) | ||
| { | ||
| return temp || (strcmp(host, guest) && access(host, F_OK) == 0); |
There was a problem hiding this comment.
exec_translated_usable() probes the interpreter with access(host, F_OK), which follows the final symlink. For a PT_INTERP redirected out of /dev/shm this dereferences the symlink and then accepts a leaf that the later O_NOFOLLOW open rejects with ELOOP, so the /lib/<basename> fallback is skipped and a resolvable interpreter fails to launch. The already-computed shm_nofollow is not threaded in. Suggest passing the shm flag through and probing shm paths with lstat() or open(..., O_NOFOLLOW).
| * a shortening link only degrades to the raw host link path, never to a | ||
| * missing socket. See docs/sysroot.md for the full lifecycle. | ||
| */ | ||
| if ((uint64_t) getpid() == absock_get_namespace_id()) { |
There was a problem hiding this comment.
Cleanup ownership gap: when only a forked child uses AF_UNIX pathname sockets, the child inherits the parent's pid as namespace_id, creates the dir plus any over-length shortening symlinks, but its cleanup runs with getpid() != absock_get_namespace_id(), so this symlink sweep is skipped. The parent registered no atexit because it never created the dir. The symlinks leak in /tmp, and the trailing rmdir then fails because the dir is not empty. Suggest letting the process that actually created the directory own the sweep (record the creator pid), or tracking shortening links per-process and unlinking them in every absock_cleanup.
| str_copy_trunc(host_path, mac_path, sizeof(host_path)); | ||
| pthread_mutex_lock(&absock_lock); | ||
| bool in_absock_dir = absock_dir_created && | ||
| !strncmp(host_path, absock_dir, strlen(absock_dir)); |
There was a problem hiding this comment.
The in_absock_dir test does a raw strncmp(host_path, absock_dir, strlen(absock_dir)) with no component boundary, so a sibling path like /tmp/elfuse-absock-1234extra/... is treated as inside the namespace and gets readlink()-dereferenced during reverse mapping. Suggest requiring the byte after the prefix to be '\0' or '/', and lstat() before readlink() so only real shortening symlinks are followed.
|
Ongoing rewrite of side car in progress. Closing this PR in favor of the rewrite attempt. |
2 participants
Consolidate the sysroot path-translation handlers so guest-visible paths are reconstructed and reverse-mapped consistently across syscalls.
Closes #233
Summary by cubic
Unifies sysroot path translation through the sidecar so guest paths resolve and reverse‑map consistently across exec, filesystem, inotify, and networking. Normalizes classification once per lookup, tightens
/dev/shmand interpreter handling, and updates AF_UNIX bind and cleanup behavior.New Features
path.cwith a one‑pass normalizer, a dirfd‑relative fast path, anAT_EMPTY_PATHhelper, and/dev/shmopens forced viapath_translation_oflags(O_NOFOLLOW|O_NONBLOCK); short‑lived descriptors useO_CLOEXEC.execveattranslates dirfd‑relative names and opens/dev/shmleaves withO_NOFOLLOW;getcwdand/proc/self/cwdreverse‑map sidecar tokens after fork/exec.EADDRINUSEon sidecar names); cleanup retires only shortening symlinks so a forked child's sockets keep working; addresses round‑trip across bind/connect/sendmsg/accept/recvmsg.Docs and Tests
docs/sysroot.mdand a full suite: path‑translation matrix (casefold and case‑sensitive lanes), exec‑cwd regression,/dev/shmexecveatno‑follow, absock lifecycle and cleanup, orphan token visibility, unreadable‑index snapshot behavior, plus a sidecar state checker wired intomake check; removed thelistxattrindirect‑coverage exemption.Written for commit 68886eb. Summary will update on new commits.