From 467cce86c63eecea20eb49bacfb46ba7d6bde798 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 10 Jun 2026 19:33:43 +1000 Subject: [PATCH] syscall: hold an ancestor-dirfd stack as the held-directory cache The held-directory cache kept only the single current leaf directory, so moving to a sibling re-resolved the whole path from the anchor and re-opened every ancestor. The generator and receiver don't recurse -- they iterate a path-sorted file list (iteration == DFS) -- so there was no call stack holding the ancestors. Keep the whole current ancestor chain open as pinned dirfds, keyed on path components, and on each resolution reuse the longest common prefix, popping only the divergent tail. Each directory is then opened once while we are inside its subtree. get_dir_fd() (receiver/generator) and held_dir_path_fd() (sender) share the stack; ds_descend() still resolves each component, so in-tree dir-symlink following and confinement are unchanged. change_dir() drops the cwd-relative stack on any real chdir -- the one invalidation needed now that the cache is content-keyed, not pointer-keyed. The per-chunk reset_dir_fd_cache() calls in receiver.c/generator.c are gone: they guarded the old pointer-keyed cache's aliasing bug, which a content-keyed stack of pinned fds cannot have, and holding the fds across chunks is strictly more race-safe (a swapped ancestor resolves to the pinned original). (cherry picked from commit 182bed6cc6b7c33e546132a069b2d5f4203cb91c) --- receiver.c | 2 - syscall.c | 154 +++++++++++++++++++++++++++++++++++++++++++++-------- util1.c | 3 ++ 3 files changed, 135 insertions(+), 24 deletions(-) diff --git a/receiver.c b/receiver.c index 8eb99e0d..60be959b 100644 --- a/receiver.c +++ b/receiver.c @@ -670,8 +670,6 @@ int recv_files(int f_in, int f_out, char *local_name) ndx = read_ndx_and_attrs(f_in, f_out, &iflags, &fnamecmp_type, xname, &xlen); if (ndx == NDX_DONE) { - /* Chunk/phase boundary: drop any held dir fd. */ - reset_dir_fd_cache(); if (!am_server && cur_flist) { set_current_file_index(NULL, 0); if (INFO_GTE(PROGRESS, 2)) diff --git a/syscall.c b/syscall.c index a8df9560..6718b1ec 100644 --- a/syscall.c +++ b/syscall.c @@ -2394,33 +2394,143 @@ int open_dir_secure(const char *dirname) #endif } -/* Cache of the current directory's held fd, keyed on the interned - * file->dirname pointer (flist.c reallocates lastdir only on a directory - * change, so distinct live dirs have distinct pointers). Reopened on a - * pointer change; reset_dir_fd_cache() MUST be called at flist-chunk - * boundaries so a reused lastdir allocation can't alias a stale fd. */ -static const char *cur_dkey = (const char *)-1; -static int cur_dfd = -1; - -int get_dir_fd(const char *dirname) -{ - if (dirname != cur_dkey) { - if (cur_dfd >= 0) - close(cur_dfd); - cur_dfd = open_dir_secure(dirname); - cur_dkey = dirname; - } - return cur_dfd; -} +/* Persistent ancestor-dirfd stack for held-directory traversal. + * + * The transfer's file list is path-sorted, so iterating it walks the tree in + * DFS order and consecutive directory resolutions share a long leading prefix. + * Rather than re-resolve a full path from the anchor each time (re-opening + * every ancestor dir per file), we keep the whole current ancestor chain open + * as pinned, race-safe dirfds and, on the next resolution, reuse the longest + * common component prefix -- popping only the divergent tail and descending the + * new tail. Each directory is then opened once while we are inside its subtree. + * + * The chain is relative to the process cwd (for a NULL anchor), so change_dir() + * drops it on any real chdir; it otherwise persists across flist chunks (the + * pinned fds stay valid, and a raced/replaced ancestor resolves to the original + * inode the fd holds -- the held-dirfd race-safety property, not a hazard). + * Each component is resolved with ds_descend(), which follows in-tree directory + * symlinks exactly as secure_relative_open() does; only the resolved dir fd is + * kept (intermediate symlink-target fds are closed -- sound, since an open + * dirfd needs no live parent). */ +#if defined AT_FDCWD && defined O_NOFOLLOW && defined O_DIRECTORY +#define DPC_MAXDEPTH 64 +static const char *dpc_anchor = (const char *)-2; +static int dpc_base = -1; /* opened anchor dir (owned), or -1 */ +static int dpc_fd[DPC_MAXDEPTH]; /* dpc_fd[i] = dir after components 0..i */ +static char dpc_name[DPC_MAXDEPTH][256]; /* textual component names */ +static int dpc_depth = 0; void reset_dir_fd_cache(void) { - if (cur_dfd >= 0) - close(cur_dfd); - cur_dfd = -1; - cur_dkey = (const char *)-1; + while (dpc_depth > 0) + close(dpc_fd[--dpc_depth]); + if (dpc_base >= 0) + close(dpc_base); + dpc_base = -1; + dpc_anchor = (const char *)-2; } +/* Resolve directory `dirpath` beneath `anchor` (NULL = cwd, else an absolute + * trusted root), reusing the held ancestor stack. Returns a BORROWED dirfd + * owned by the cache (do NOT close), or -1 (errno preserved for a real open + * error, errno==0 for an uncacheable path -- "..", too deep/long, or a relative + * non-cwd anchor) so the caller can fall back to secure_relative_open(). */ +static int dpc_dir_fd(const char *anchor, const char *dirpath) +{ + char copy[MAXPATHLEN]; + char *comps[DPC_MAXDEPTH]; + char *sv = NULL; + int nc = 0, p, i; + + if (anchor && anchor[0] != '/') { errno = 0; return -1; } + if (!dirpath) + dirpath = ""; + if (dirpath[0] == '/') { errno = 0; return -1; } + + if (anchor != dpc_anchor || dpc_base < 0) { + int fl; + reset_dir_fd_cache(); + dpc_base = openat(AT_FDCWD, anchor ? anchor : ".", O_RDONLY | O_DIRECTORY); + if (dpc_base < 0) + return -1; + if ((fl = fcntl(dpc_base, F_GETFD)) >= 0) + fcntl(dpc_base, F_SETFD, fl | FD_CLOEXEC); + dpc_anchor = anchor; + } + + if (strlcpy(copy, dirpath, sizeof copy) >= sizeof copy) { errno = ENAMETOOLONG; return -1; } + for (char *c = strtok_r(copy, "/", &sv); c; c = strtok_r(NULL, "/", &sv)) { + if (c[0] == '.' && c[1] == '\0') + continue; /* "." */ + if (c[0] == '.' && c[1] == '.' && c[2] == '\0') { errno = 0; return -1; } + if (nc >= DPC_MAXDEPTH || strlen(c) >= sizeof dpc_name[0]) { errno = 0; return -1; } + comps[nc++] = c; + } + + /* Reuse the longest common prefix; drop the divergent tail. */ + for (p = 0; p < dpc_depth && p < nc && strcmp(dpc_name[p], comps[p]) == 0; p++) + ; + while (dpc_depth > p) + close(dpc_fd[--dpc_depth]); + + /* Descend the new tail, holding each resolved component. */ + for (i = p; i < nc; i++) { + int afd = dpc_depth > 0 ? dpc_fd[dpc_depth-1] : dpc_base; + struct dirstack ds; + int hops = SECURE_OPEN_MAXSYMLINKS; + int fd, fl; + if (ds_init(&ds, afd) < 0) + return -1; + if (ds_descend(&ds, comps[i], &hops) < 0) { + int e = errno; + ds_free(&ds); + errno = e; + return -1; + } + fd = ds_take(&ds); + ds_free(&ds); /* closes intermediate symlink fds, not afd */ + if (fd < 0) + return -1; + if ((fl = fcntl(fd, F_GETFD)) >= 0) + fcntl(fd, F_SETFD, fl | FD_CLOEXEC); + strlcpy(dpc_name[dpc_depth], comps[i], sizeof dpc_name[0]); + dpc_fd[dpc_depth++] = fd; + } + + return nc > 0 ? dpc_fd[dpc_depth-1] : dpc_base; +} + +/* Public entry for the sender (no secure_relpath_active gate: its send paths + * confine unconditionally). Borrowed fd; -1 => caller uses the full walk. */ +int held_dir_path_fd(const char *anchor, const char *dirpath) +{ + return dpc_dir_fd(anchor, dirpath); +} + +int get_dir_fd(const char *dirname) +{ + if (!secure_relpath_active()) { errno = 0; return -1; } + return dpc_dir_fd(NULL, dirname); +} +#else +void reset_dir_fd_cache(void) +{ +} +int held_dir_path_fd(const char *anchor, const char *dirpath) +{ + (void)anchor; + (void)dirpath; + errno = 0; + return -1; +} +int get_dir_fd(const char *dirname) +{ + (void)dirname; + errno = 0; + return -1; +} +#endif + /* Return the cached current-directory fd iff `path` lives directly in the * entry's own directory (file->dirname) -- the common case for held-dirfd * traversal. Returns -1 (caller falls back to the do_*_at() wrappers) for diff --git a/util1.c b/util1.c index ce62332c..c0fdbadd 100644 --- a/util1.c +++ b/util1.c @@ -1251,6 +1251,9 @@ int change_dir(const char *dir, int set_path_only) curr_dir_depth = count_dir_elements(curr_dir + module_dirlen); } + if (!set_path_only) /* a real chdir invalidates the cwd-relative dir-fd stack */ + reset_dir_fd_cache(); + if (DEBUG_GTE(CHDIR, 1) && !set_path_only) rprintf(FINFO, "[%s] change_dir(%s)\n", who_am_i(), curr_dir);