Skip to content

Resolve /proc/self/fd/<n> for path-based syscalls - #289

Open
doanbaotrung wants to merge 1 commit into
sysprog21:mainfrom
open-sources-port:proc-self-fd
Open

Resolve /proc/self/fd/<n> for path-based syscalls#289
doanbaotrung wants to merge 1 commit into
sysprog21:mainfrom
open-sources-port:proc-self-fd

Conversation

@doanbaotrung

@doanbaotrung doanbaotrung commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

resolve_proc_at_path() returns early for absolute paths, so path_translate_at never resolved /proc/self/fd/ and handed the sysroot spelling to the host. open(), stat() and readlink() were unaffected -- each has its own /proc intercept -- but chmod, chown and utimensat got ENOENT for a descriptor that was perfectly valid.

On Linux that path is a magic symlink, and it is the standard way to reach a file through an fd when no f*() variant applies. systemd's fchmod_opath() uses it because fchmod() rejects O_PATH descriptors, and reads ENOENT there as "this fd is not valid", reporting EBADF:

Failed to copy permissions from /etc/group to /etc/.#group...:
Bad file descriptor

which stopped systemd-sysusers from writing /etc/group and failed dpkg --configure systemd.

Map the path to the host path the descriptor holds. Only host_path is rewritten; guest_path and intercept_path keep the /proc spelling so the existing intercepts still match. A descriptor with no host path (pipe, socket, anonymous fd) and a path walking through the number are both left alone.


Summary by cubic

Resolve absolute fd magic links like /proc/self/fd/, /proc//fd/, /dev/fd/, and /dev/std{in,out,err} to the underlying host file for follow-style path-based syscalls, matching Linux. Previously we forwarded the sysroot spelling, so chmod/chown/utimensat/truncate/access on valid O_PATH fds returned ENOENT and surfaced as EBADF (e.g., systemd’s fchmod_opath); now those calls act on the descriptor’s file.

  • Apply in path_translate_at for absolute paths only when neither PATH_TR_NOFOLLOW nor PATH_TR_CREATE is set; set host_path from F_GETPATH on a dup of the host fd and return before sysroot resolution.
  • Accept only a bare descriptor with no trailing component; recognize /proc/self, this process’s pid, /dev/fd, and /dev/std{in,out,err}. Skip fds with no host path (pipes, sockets, anonymous).
  • Parse pid and fd components like Linux (digits only; no sign or leading zero except "0"); reject "+3", "03", and " 3".
  • Preserve open/stat/readlink behavior and existing /proc intercepts by keeping guest_path/intercept_path unchanged; no-follow or create-style operations remain unchanged.

Written for commit fa662bf. Summary will update on new commits.

Review in cubic

@doanbaotrung
doanbaotrung marked this pull request as draft August 12, 2026 09:05
cubic-dev-ai[bot]

This comment was marked as resolved.

@doanbaotrung
doanbaotrung marked this pull request as ready for review August 12, 2026 14:46

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/syscall/path.c">

<violation number="1" location="src/syscall/path.c:258">
P1: When the fd's file is renamed, unlinked, or replaced after `F_GETPATH` returns, the later path-based syscall uses a stale pathname and can fail with `ENOENT` or modify a different inode instead of the file held by the fd. Keep the duplicated descriptor alive through the operation or use an identity-preserving fd-backed operation.</violation>

<violation number="2" location="src/syscall/path.c:348">
P2: This early return rewrites host_path for every follow-style absolute /proc/self/fd/<n> translation, not just the chmod/chown/utimensat family the PR targets. When the /proc open intercept does not serve the path (sys_openat_path falls through to open(tx.host_path) at fs.c:573), open/stat behavior changes from ENOENT to acting on the resolved file, contradicting the stated invariant that open/stat/readlink stay unchanged. Verify the /proc intercept always shadows these paths before relying on host_path-only scoping.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/syscall/path.c

char resolved[MAXPATHLEN];
int rc = fcntl(host_fd, F_GETPATH, resolved);
close(host_fd);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When the fd's file is renamed, unlinked, or replaced after F_GETPATH returns, the later path-based syscall uses a stale pathname and can fail with ENOENT or modify a different inode instead of the file held by the fd. Keep the duplicated descriptor alive through the operation or use an identity-preserving fd-backed operation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/syscall/path.c, line 258:

<comment>When the fd's file is renamed, unlinked, or replaced after `F_GETPATH` returns, the later path-based syscall uses a stale pathname and can fail with `ENOENT` or modify a different inode instead of the file held by the fd. Keep the duplicated descriptor alive through the operation or use an identity-preserving fd-backed operation.</comment>

<file context>
@@ -200,6 +201,71 @@ static int path_check_relative_sysroot_containment(guest_fd_t dirfd,
+
+    char resolved[MAXPATHLEN];
+    int rc = fcntl(host_fd, F_GETPATH, resolved);
+    close(host_fd);
+    if (rc < 0)
+        return 0;
</file context>

@maxliu04002 maxliu04002 Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address this issue.

Comment thread src/syscall/path.c
* file, or unlinkat("/proc/self/fd/<n>") would delete it instead of failing
* on the /proc entry.
*/
if (tx->guest_path[0] == '/' &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This early return rewrites host_path for every follow-style absolute /proc/self/fd/ translation, not just the chmod/chown/utimensat family the PR targets. When the /proc open intercept does not serve the path (sys_openat_path falls through to open(tx.host_path) at fs.c:573), open/stat behavior changes from ENOENT to acting on the resolved file, contradicting the stated invariant that open/stat/readlink stay unchanged. Verify the /proc intercept always shadows these paths before relying on host_path-only scoping.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/syscall/path.c, line 348:

<comment>This early return rewrites host_path for every follow-style absolute /proc/self/fd/<n> translation, not just the chmod/chown/utimensat family the PR targets. When the /proc open intercept does not serve the path (sys_openat_path falls through to open(tx.host_path) at fs.c:573), open/stat behavior changes from ENOENT to acting on the resolved file, contradicting the stated invariant that open/stat/readlink stay unchanged. Verify the /proc intercept always shadows these paths before relying on host_path-only scoping.</comment>

<file context>
@@ -265,6 +331,28 @@ int path_translate_at(guest_fd_t dirfd,
+     * file, or unlinkat("/proc/self/fd/<n>") would delete it instead of failing
+     * on the /proc entry.
+     */
+    if (tx->guest_path[0] == '/' &&
+        !(flags & (PATH_TR_NOFOLLOW | PATH_TR_CREATE)) &&
+        resolve_proc_fd_host_path(tx->guest_path, tx->host_buf,
</file context>

@doanbaotrung
doanbaotrung force-pushed the proc-self-fd branch 3 times, most recently from 1f96445 to 2349355 Compare August 13, 2026 11:31
jserv

This comment was marked as outdated.

Comment thread src/syscall/path.c Outdated
Comment thread src/syscall/path.c Outdated

@jserv jserv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase latest main branch and resolve conflicts.

@doanbaotrung
doanbaotrung marked this pull request as draft August 14, 2026 08:24
@doanbaotrung
doanbaotrung marked this pull request as ready for review August 14, 2026 08:27
@doanbaotrung

Copy link
Copy Markdown
Collaborator Author

You're right and the sentence was simply false. F_GETPATH reports where the descriptor's file actually lives, and for a good fraction of descriptors that's outside the sysroot — an emulated character device, a /dev/shm backing file (which proc_dev_shm_resolve deliberately puts in a per-UID host directory), or inherited stdio pointing at a tty or whatever the user redirected.

I'd also been using it as an implied containment argument, which it can't carry. The real reasons are different, and I've split them out:

     * Returning before sysroot resolution is not a containment claim about the
     * path: F_GETPATH reports where the descriptor's file actually lives, which
     * is regularly outside the sysroot -- an emulated character device, a
     * /dev/shm backing file, inherited stdio. Re-resolving one of those as a
     * guest path would be wrong, since it is already a host path. Nothing is
     * widened by it either: the guest holds the descriptor, so this reaches
     * only what it could already reach through it.

The early return is right for a mechanical reason (the string is already a host path, so guest→host mapping doesn't apply to it), and containment rests on the descriptor, not the path.

strtol vs name_to_int — fixed in both places.

Confirmed against the kernel: name_to_int() rejects a leading zero on any name longer than one character, and bails on the first non-digit, so sign and whitespace never get in. strtol accepts all three.

Added path_parse_fd_name() implementing exactly those rules, and routed both call sites through it — resolve_fd_magiclink_host_path() and proc_parse_fd_index(). Fixing only mine would have been worse than leaving both: open("/proc/self/fd/+3") would keep succeeding through the intercept while chmod on the same string failed, so the two would disagree about which names exist.

Verified with fd 9 held open (only the bare 9 resolves):

name open stat chmod /dev/fd
9 reads file ok ok ok
+9 rejected rejected rejected
09 rejected rejected rejected rejected
-9, " 9", 9x rejected

One divergence I did not fix

Rejected names now report EBADF where Linux reports ENOENT — on Linux these are dcache lookups, so a malformed or closed fd name is always ENOENT, never EBADF. That comes from the errno_on_invalid argument at the four proc_parse_fd_index() call sites, two of which already pass ENOENT and two EBADF:

procemu.c:924   EBADF     procemu.c:2768  ENOENT
procemu.c:3212  ENOENT    procemu.c:3265  EBADF

Unifying those on ENOENT would match Linux for both the malformed-name and closed-slot cases, but it changes behaviour for callers beyond this PR's subject, so I left it. Say the word and I'll do it here or file it separately.

@jserv
jserv requested review from henrybear327 and maxliu04002 and removed request for maxliu04002 August 14, 2026 08:33

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 existing issue remains and 1 new issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/syscall/path.c">

<violation number="1" location="src/syscall/path.c:246">
P3: The /proc magic-link shape is matched with exact strncmp against fixed prefixes, so non-canonical spellings that Linux normalizes — /proc/self//fd/3, /proc//self/fd/3, /proc/self/fd//3 — are not recognized and fall through to generic resolution, which fails on the host with ENOENT. This only affects redundant-separator spellings (glibc usually normalizes before syscalls, but raw syscalls can pass them), so it is low impact, but the fix leaves those spellings broken while the canonical form works.</violation>
</file>

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Re-trigger cubic

Comment thread src/syscall/path.c Outdated
Comment thread src/syscall/path.c
{
const char *rest = NULL;

if (strncmp(path, "/proc/", 6) == 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The /proc magic-link shape is matched with exact strncmp against fixed prefixes, so non-canonical spellings that Linux normalizes — /proc/self//fd/3, /proc//self/fd/3, /proc/self/fd//3 — are not recognized and fall through to generic resolution, which fails on the host with ENOENT. This only affects redundant-separator spellings (glibc usually normalizes before syscalls, but raw syscalls can pass them), so it is low impact, but the fix leaves those spellings broken while the canonical form works.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/syscall/path.c, line 246:

<comment>The /proc magic-link shape is matched with exact strncmp against fixed prefixes, so non-canonical spellings that Linux normalizes — /proc/self//fd/3, /proc//self/fd/3, /proc/self/fd//3 — are not recognized and fall through to generic resolution, which fails on the host with ENOENT. This only affects redundant-separator spellings (glibc usually normalizes before syscalls, but raw syscalls can pass them), so it is low impact, but the fix leaves those spellings broken while the canonical form works.</comment>

<file context>
@@ -200,6 +201,106 @@ static int path_check_relative_sysroot_containment(guest_fd_t dirfd,
+{
+    const char *rest = NULL;
+
+    if (strncmp(path, "/proc/", 6) == 0) {
+        rest = path + 6;
+        if (!strncmp(rest, "self/", 5)) {
</file context>

resolve_proc_at_path() returns early for absolute paths, so
path_translate_at never resolved /proc/self/fd/<n> and handed the sysroot
spelling to the host. open(), stat() and readlink() were unaffected --
each has its own /proc intercept -- but chmod, chown and utimensat got
ENOENT for a descriptor that was perfectly valid.

On Linux that path is a magic symlink, and it is the standard way to
reach a file through an fd when no f*() variant applies. systemd's
fchmod_opath() uses it because fchmod() rejects O_PATH descriptors, and
reads ENOENT there as "this fd is not valid", reporting EBADF:

  Failed to copy permissions from /etc/group to /etc/.#group...:
  Bad file descriptor

which stopped systemd-sysusers from writing /etc/group and failed
`dpkg --configure systemd`.

Map the path to the host path the descriptor holds. Only host_path is
rewritten; guest_path and intercept_path keep the /proc spelling so the
existing intercepts still match. A descriptor with no host path (pipe,
socket, anonymous fd) and a path walking through the number are both left
alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants