From 7c22d3d7fe5d533eebfe08c36dbdbd2386200f7f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:53:20 +0000 Subject: [PATCH 1/4] resolver: close the store_fd symlink-target fd when kind() fails RealFS::kind() opens a symlink's target, then fstat()s it and reads the fd's realpath. The scopeguard that decided close-vs-store wrote the fd into the stack-local EntryCache whenever store_fd was set and the fd budget allowed caching. On the success path that EntryCache is returned and the fd reaches the resolver's Entry cache; on the fstat/get_fd_path /append_slice error paths the EntryCache is dropped and the fd leaks. Track whether the fd has been published to the returned value and close on every exit before that point. The cache.fd write moves onto the success path so the raw-pointer write through the guard is no longer needed. --- src/resolver/lib.rs | 39 ++--- .../resolver-kind-symlink-fd-leak.test.ts | 138 ++++++++++++++++++ 2 files changed, 159 insertions(+), 18 deletions(-) create mode 100644 test/js/bun/resolve/resolver-kind-symlink-fd-leak.test.ts diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index edc0325bef15..21107d9d260c 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -1446,8 +1446,6 @@ pub mod fs { kind_from_mode(stat_.st_mode as bun_sys::Mode) == FileKind::SymLink; let mut file_kind = kind_from_mode(stat_.st_mode as bun_sys::Mode); - let mut symlink: &[u8] = b""; - if is_symlink { let file: Fd = if let Some(valid) = existing_fd.unwrap_valid() { valid @@ -1468,23 +1466,32 @@ pub mod fs { }; FileSystem::set_max_fd(file.native()); - // The close-or-store cleanup runs on - // BOTH success and error paths — use scopeguard so close-or-store happens even if - // fstat()/get_fd_path() return early with `?`. - let need_to_close_files = self.need_to_close_files(); - let cache_ptr: *mut EntryCache = &raw mut cache; - let _guard = scopeguard::guard(file, move |file| { - if (!store_fd || need_to_close_files) && !existing_fd.is_valid() { + // The fd must be closed on every `?` early-return below; it is + // only kept open once it has been written into the returned + // `cache` (and only when we opened it for caching). + let we_opened_it = !existing_fd.is_valid(); + let close_even_on_success = !store_fd || self.need_to_close_files(); + let fd_published = core::cell::Cell::new(false); + let _guard = scopeguard::guard((), |()| { + if we_opened_it && (close_even_on_success || !fd_published.get()) { let _ = bun_sys::close(file); - } else if bun_core::feature_flags::STORE_FILE_DESCRIPTORS { - // SAFETY: `cache_ptr` points into a stack local that outlives this guard. - unsafe { (*cache_ptr).fd = file }; } }); - let file_stat = bun_sys::fstat(*_guard)?; - symlink = bun_sys::get_fd_path(*_guard, &mut outpath)?; + let file_stat = bun_sys::fstat(file)?; + let symlink = bun_sys::get_fd_path(file, &mut outpath)?; file_kind = kind_from_mode(file_stat.st_mode as bun_sys::Mode); + if !symlink.is_empty() { + cache.symlink = Interned::from_static( + FilenameStore::instance().append_slice(symlink)?, + ); + } + if bun_core::feature_flags::STORE_FILE_DESCRIPTORS + && !(we_opened_it && close_even_on_success) + { + cache.fd = file; + } + fd_published.set(true); } debug_assert!(file_kind != FileKind::SymLink); @@ -1494,10 +1501,6 @@ pub mod fs { } else { EntryKind::File }; - if !symlink.is_empty() { - cache.symlink = - Interned::from_static(FilenameStore::instance().append_slice(symlink)?); - } Ok(cache) } diff --git a/test/js/bun/resolve/resolver-kind-symlink-fd-leak.test.ts b/test/js/bun/resolve/resolver-kind-symlink-fd-leak.test.ts new file mode 100644 index 000000000000..0866c3af0e05 --- /dev/null +++ b/test/js/bun/resolve/resolver-kind-symlink-fd-leak.test.ts @@ -0,0 +1,138 @@ +// RealFS::kind() opens a symlink's target, then fstat()s and reads the fd's +// realpath. When the resolver was asked to cache the fd (store_fd, set by +// --hot / --watch), the cleanup guard wrote the fd into a stack-local +// EntryCache instead of closing it. On the success path that EntryCache is +// returned and the fd reaches the resolver's Entry cache; on the error path +// the EntryCache is dropped and the fd leaks. +// +// On Linux the realpath step is readlink("/proc/self/fd/N"), called through +// libc, so an LD_PRELOAD shim can make it fail with EIO for fds whose target +// contains a marker string. The inner process runs under --hot (store_fd=true), +// resolves one symlink per target, then counts open fds that still point at a +// marker target. +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { bunEnv, bunExe, isLinux, isMusl, tempDir } from "harness"; +import { symlinkSync } from "node:fs"; +import { join } from "node:path"; + +const cc = Bun.which("cc") || Bun.which("gcc") || Bun.which("clang"); +const LINK_COUNT = 16; +const MARKER = "__KIND_FAIL__"; + +const SHIM_C = /* c */ ` +#define _GNU_SOURCE +#include +#include +#include +#include +#include + +static ssize_t (*real_readlink)(const char *, char *, size_t); +static const char *off_path; + +__attribute__((constructor)) static void init(void) { + real_readlink = (ssize_t (*)(const char *, char *, size_t))dlsym(RTLD_NEXT, "readlink"); + off_path = getenv("SHIM_OFF_PATH"); +} + +ssize_t readlink(const char *path, char *buf, size_t bufsiz) { + if (!real_readlink) + real_readlink = (ssize_t (*)(const char *, char *, size_t))dlsym(RTLD_NEXT, "readlink"); + ssize_t n = real_readlink(path, buf, bufsiz); + if (n <= 0) return n; + if (off_path && access(off_path, F_OK) == 0) return n; + if (strncmp(path, "/proc/self/fd/", 14) != 0) return n; + size_t m = (size_t)n < bufsiz ? (size_t)n : bufsiz - 1; + char saved = buf[m]; + buf[m] = 0; + int hit = strstr(buf, "${MARKER}") != NULL; + buf[m] = saved; + if (hit) { errno = EIO; return -1; } + return n; +} +`; + +const INNER = /* ts */ ` +import { readdirSync, readlinkSync, writeFileSync } from "node:fs"; +const N = ${LINK_COUNT}; +let failed = 0; +for (let i = 0; i < N; i++) { + try { + require.resolve("./link" + i + ".js"); + } catch { + failed++; + } +} +// Disarm the shim so the leak count can readlink the fds it is counting. +writeFileSync(process.env.SHIM_OFF_PATH!, ""); +let leaked = 0; +for (const name of readdirSync("/proc/self/fd")) { + try { + if (readlinkSync("/proc/self/fd/" + name).includes("${MARKER}")) leaked++; + } catch {} +} +console.log("FAILED=" + failed + " LEAKED=" + leaked); +process.exit(0); +`; + +let dir: ReturnType; +let shimPath: string; +let offPath: string; + +beforeAll(async () => { + if (!isLinux || isMusl || !cc) return; + const files: Record = { + "shim.c": SHIM_C, + "inner.ts": INNER, + "package.json": "{}", + }; + for (let i = 0; i < LINK_COUNT; i++) files[`targets${MARKER}/t${i}.js`] = `export {};\n`; + dir = tempDir("resolver-kind-fail-leak", files); + for (let i = 0; i < LINK_COUNT; i++) { + symlinkSync(join(String(dir), `targets${MARKER}`, `t${i}.js`), join(String(dir), `link${i}.js`)); + } + shimPath = join(String(dir), "shim.so"); + offPath = join(String(dir), "SHIM_OFF"); + await using ccProc = Bun.spawn({ + cmd: [cc, "-shared", "-fPIC", "-o", shimPath, join(String(dir), "shim.c"), "-ldl"], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [ccOut, ccErr, ccExit] = await Promise.all([ccProc.stdout.text(), ccProc.stderr.text(), ccProc.exited]); + if (ccExit !== 0) throw new Error(`shim compile failed: ${ccErr || ccOut}`); +}); + +afterAll(() => { + dir?.[Symbol.dispose](); +}); + +// LD_PRELOAD interposition requires a dynamic libc; the musl build links libc +// statically, so the shim cannot intercept readlink() there. +test.skipIf(!isLinux || isMusl || !cc)( + "resolver closes the symlink-target fd when get_fd_path fails after a successful open (store_fd)", + async () => { + const existing = bunEnv.LD_PRELOAD; + await using proc = Bun.spawn({ + cmd: [bunExe(), "--hot", "inner.ts"], + cwd: String(dir), + env: { + ...bunEnv, + LD_PRELOAD: existing ? `${shimPath}:${existing}` : shimPath, + SHIM_OFF_PATH: offPath, + }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // FAILED=0: Entry::kind swallows the error and falls back to File, so the + // resolves still succeed. If bun stops routing get_fd_path through libc + // readlink() the shim goes inert and this test measures nothing; LEAKED=0 + // is the only load-bearing assertion. + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: `FAILED=0 LEAKED=0`, + stderr: "", + exitCode: 0, + }); + }, +); From 328c9557b4e9e07cbf161f304441d3c279c76647 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:56:09 +0000 Subject: [PATCH 2/4] [autofix.ci] apply automated fixes --- src/resolver/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 21107d9d260c..7004feded9fc 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -1482,9 +1482,8 @@ pub mod fs { let symlink = bun_sys::get_fd_path(file, &mut outpath)?; file_kind = kind_from_mode(file_stat.st_mode as bun_sys::Mode); if !symlink.is_empty() { - cache.symlink = Interned::from_static( - FilenameStore::instance().append_slice(symlink)?, - ); + cache.symlink = + Interned::from_static(FilenameStore::instance().append_slice(symlink)?); } if bun_core::feature_flags::STORE_FILE_DESCRIPTORS && !(we_opened_it && close_even_on_success) From 63c6cf6c35a2feac8f8b4cc99b69f3bd2fd2bdf1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:57:36 +0000 Subject: [PATCH 3/4] drop redundant guard comment --- src/resolver/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 7004feded9fc..dcbf86a6ffe7 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -1466,9 +1466,6 @@ pub mod fs { }; FileSystem::set_max_fd(file.native()); - // The fd must be closed on every `?` early-return below; it is - // only kept open once it has been written into the returned - // `cache` (and only when we opened it for caching). let we_opened_it = !existing_fd.is_valid(); let close_even_on_success = !store_fd || self.need_to_close_files(); let fd_published = core::cell::Cell::new(false); From 97fd60e5d63c294729446182197df79f4bce64d0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 4 Aug 2026 08:40:45 +0000 Subject: [PATCH 4/4] ci: retrigger