errno: map a kernel errno outside the table to EUNKNOWN instead of transmuting - #40720
Conversation
…ansmuting `SystemErrno::from_raw` and the Linux raw-syscall `GetErrno for usize` built the `#[repr(u16)]` enum with `transmute`, guarded only by a `debug_assert!` (or an `n < 4096` range check). Linux declares 134 variants, but the kernel can hand userspace any code below 4096: FUSE filesystems pass through arbitrary values and driver-internal codes such as ENOTSUPP (524) leak out. Every libc `get_errno` path decoded that into an invalid enum value. Add an `EUNKNOWN` variant to the Linux, Darwin and FreeBSD enums (the Windows enum already has one at 134) and derive `strum::FromRepr`. `from_raw` is now a total function: a declared discriminant maps to its variant and anything else to `EUNKNOWN`. `init` decodes through `from_repr` as well, so the crate no longer contains a transmute. The Windows `E::from_raw` gets the same shape with `UNKNOWN` as fallback. `Error::get_errno` and `e_from_negated` used to fall back to `SUCCESS` for a code they could not decode. A failure that reads as success is wrong (the shell builtins turn it into exit code 0), so they fall back to `EUNKNOWN` too. The test installs a seccomp filter that makes fsync(2) fail with 524 and checks that fs.fsyncSync reports `code: "EUNKNOWN"`. A debug build used to die on the `from_raw` assertion.
|
Warning Review limit reached
On-demand reviews are free for the next 24 days. After that, they cost $0.25 per reviewed file. Or wait 11 seconds for your next included review. View limit detailsLimit details: You’ve used all 5 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
Comment |
|
Status: ready for review (head 748a37a). Reproduced with the seccomp helper from the test: a filter makes
CI (build 107387): 180 of 181 jobs pass. The new seccomp test ran on all Linux lanes (5 pass, 11 expect calls on each). The one red job is darwin x64 Related: #38914 fixes the same transmute as half of a larger change, with |
|
Updated 11:35 PM PT - Aug 27th, 2026
❌ @robobun, your commit 748a37a has 1 failures in
🧪 To try this PR locally: bunx bun-pr 40720That installs a local version of the PR into your bun-40720 --bun |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes the fallback semantics of Error::get_errno/e_from_negated (SUCCESS → EUNKNOWN) across all four platforms and picks a Node-divergent error shape (errno: -134, code: "EUNKNOWN" vs Node's errno: -524, code: "Unknown system error -524"), a human sign-off on the design choice would still be worthwhile.
Checked: from_raw stays const fn (strum's from_repr is const); E::EUNKNOWN resolves on Windows via the existing E::UNKNOWN alias; LIBUV_ERROR_MAP/COREUTILS_ERROR_MAP default-fill the new slot with "unknown error" via EnumMap::LENGTH; resolve_system_errno's < MAX gate now admits the new variant so code: "EUNKNOWN" surfaces; SystemErrno::init's unsigned_abs() rewrite preserves the old None-on-out-of-range contract including at i64::MIN.
Extended reasoning...
Overview
This PR eliminates the last transmute::<u16, SystemErrno> paths in bun_errno, replacing them with strum::FromRepr-generated checked lookups that fall back to a new EUNKNOWN variant appended to each POSIX enum (Linux 134, Darwin 107, FreeBSD 98). MAX is bumped by one on each. SystemErrno::from_raw, the Linux raw-syscall GetErrno for usize, the Windows E::from_raw, and impl_get_errno_libc! all become total. Separately, e_from_negated and bun_sys::Error::get_errno change their unmapped fallback from SUCCESS to EUNKNOWN. SystemErrno::init is rewritten as u16::try_from(code.unsigned_abs()).and_then(from_repr). Rust unit tests exercise 524/512/u16::MAX and round-tripping; the seccomp test file is generalized to parametrize the blocked syscall and errno, and adds an __NR_fsync-fails-with-524 case asserting code: "EUNKNOWN".
Security risks
None identified. The change removes release-build UB (invalid #[repr(u16)] enum discriminants from kernel-supplied errno values), which is strictly a hardening. No new unsafe blocks are introduced; two transmutes and their SAFETY comments are deleted. The seccomp test helper is compiled from a local C string into a temp dir and only ever runs bun -e under a syscall filter — no new attack surface.
Level of scrutiny
Medium-high. The errno crate is load-bearing for every syscall error surfaced to JS across four target OSes, and the #[cfg]-gated arms are only type-checked on their own target. I verified the Windows arm compiles by confirming E::EUNKNOWN is an existing associated-const alias for E::UNKNOWN in windows_errno.rs. The MAX bump ripples into system_errno_max_dense(), resolve_system_errno's < MAX gate, and the EnumMap-backed error-label tables — all handle the new dense slot correctly (default "unknown error" fill, IntoStaticStr gives the name). The errno_table_full_range test iterates 1..max and will cover the new slot. The init rewrite preserves semantics: unsigned_abs() on any i64 fits or overflows u16::try_from, and from_repr rejects undeclared codes exactly as the old range check did (plus now accepts the round-tripping EUNKNOWN discriminant).
Other factors
The SUCCESS → EUNKNOWN fallback change in get_errno/e_from_negated is semantically correct (an Error should never read as success — the PR notes shell builtins turn that into exit 0) but is a behavior change a maintainer should be aware of. The PR also explicitly diverges from Node's presentation for out-of-table codes (Node preserves the raw number in errno and the message; this reports the synthetic -134/EUNKNOWN on the get_errno path because the return type is the enum) and names two conflicting open PRs (#38914, #30924) taking a panic-based approach instead — a human should pick the direction. The prior automated run's inline comments led only to comment-trimming commits (no logic changes since f5c4849), so this is the first top-level verdict.
…0720 The transmute ban stays. It passes against main and keeps the unchecked shape out of src/errno.
Problem
SystemErrno::from_raw(src/errno/lib.rs) and the Linux raw-syscallimpl GetErrno for usize(src/errno/linux_errno.rs) built the#[repr(u16)]enum withtransmute::<u16, E>. The only guard was adebug_assert!(n < MAX)or ann < 4096range check. Linux declares 134 variants, so any larger code is an invalid enum value in a release build (amatchon it can jump anywhere). A debug build dies withpanic: assertion failed: (n as usize) < (Self::MAX as usize).ENOTSUPP(524) andERESTARTSYS(512) leak out. Every libcget_errno(rc)path (impl_get_errno_libc!) andE::from_rawcaller decoded them this way.Fix
EUNKNOWNvariant to the Linux (134), Darwin (107) and FreeBSD (98) enums, one past the last kernel errno. The Windows enum already hasEUNKNOWN = 134. Derivestrum::FromRepron the POSIX enums.from_rawis now a total function:from_repr(n)for a declared discriminant,EUNKNOWNfor anything else.initdecodes throughfrom_reprtoo. The errno crate has no transmute left. The WindowsE::from_rawgets the same shape, withUNKNOWNas the fallback.Error::get_errnoande_from_negatedfell back toSUCCESSfor a code they could not decode. AnErroris a failure, and the shell builtins turn thatSUCCESSinto exit code 0. They fall back toEUNKNOWNnow.from_errnoandto_zig_errkeep theirEIOfallback.test/js/node/fs/fs-stat-seccomp-linux.test.ts, new block "node:fs errno outside the SystemErrno table" (a seccomp filter makesfsync(2)fail with 524,fs.fsyncSyncmust reportcode: "EUNKNOWN"). Alsocargo test -p bun_errno,cargo check --workspacefor the linux, darwin, windows, freebsd and android targets,test/js/node/fs/fs.test.ts,test/js/node/test/parallel/test-uv-errno.js.Background
SystemErrno(aliasEon POSIX) is the per-OS errno enum. Rust requires a#[repr(u16)]enum to hold a declared discriminant at all times. A value that is not one is undefined behaviour at the point of construction, whether or not amatchever runs.strum::FromReprgeneratesconst fn from_repr(u16) -> Option<Self>, an exhaustive match over the declared variants. It is the checked constructor the Windows enums already used.get_errno(rc)turns a syscall return value into anE. On libc targets it reads the thread-local errno after a-1. On Linux raw syscalls the kernel returns-errnoin the result register, and any value in-4095..=-1is an error, a range wider than the table.bun_sys::Errorstores the errno as a plainu16and decodes it on read. A code outside the table already reported as nameUNKNOWNwith the real negative number on that path.get_errno(rc)has no room for the raw number becauseEis an exhaustive enum, so the new variant is what it reports. JS seescode: "EUNKNOWN"anderrno: -134on Linux.Notes
Reproduction, both on the release binary and on a debug build. The seccomp helper from the test makes
fsync(2)return 524, then runsfs.fsyncSync(fd):Design choice: the fallback is a variant, not
EIO. Windows already models unmapped errors this way, the result round-trips throughinit(init(134)isSome(EUNKNOWN), so a storedEUNKNOWNnever reads back asSUCCESS), and it keeps the user-visible name honest. Node on Linux reportserrno: -524, code: "Unknown system error -524"for the same call. The raw number is not preserved here becauseget_errno(rc)returns the enum. Thecheck!-style wrappers inbun_sys(Error::from_code_int) store the raw number and are unchanged.Placement:
EUNKNOWNsits at the oldMAX, andMAXgrows by one, so the enums stay dense andenum_map::Enum,LIBUV_ERROR_MAP,COREUTILS_ERROR_MAPandsystem_errno_max_dense()keep working without changes. Both maps fill the new slot with "unknown error".os.constants.errnoandutil.getSystemErrorMap()are built from other tables and do not change.impl_get_errno_libc!now converts thec_interrno withu16::try_frominstead ofas u16, so a value that does not fit cannot wrap onto another variant.Related open PRs. #38914 fixes the same transmute as half of a larger change (release-mode
assert!in theZStr/WStrconstructors). It makesfrom_rawpanic on an undeclared code and routes OS codes toEIO. This PR takes the variant approach and touches only the errno path. The two conflict insrc/errno. #30924 makesfrom_rawpanic on OS input. #39340 renames the WindowsEUNKNOWNstrum name toUNKNOWN. If it lands, the three POSIX variants added here want the same#[strum(serialize = "UNKNOWN")].Other suites run:
test/js/bun/shell/commands/rm.test.ts(its "replaced by a symlink during deletion" case runs 4.7 to 5.1 s against the 5 s default in this ASAN container and times out on some runs, with and without the change),test/js/bun/sys/error-name-from-libuv.test.ts,cargo clippy -p bun_errno -p bun_sys,cargo fmt --check.no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/fs/fs-stat-seccomp-linux.test.ts