Repository navigation
fs: close directories when handle allocation fails - #801
thehobbit85 wants to merge 1 commit into
Conversation
Check the folder-backed directory handle allocation before dereferencing it. On failure, close the opened DIR, preserve errno, and return the existing filesystem failure without publishing a handle. Add synthetic failure injection through the real handler plus normal enumeration and descriptor lifetime checks. Related to seedds#313 (comment).
e999599 to
83c0e31
Compare
Check the folder-backed directory handle allocation before dereferencing it. On failure, close the opened DIR, preserve errno and return through the existing filesystem error path without publishing a handle. The real-handler fixture reproduces the prior null write under ASan/UBSan. All 19 checks pass after the fix, including descriptor closure, unchanged output handles and subsequent successful enumeration. Co-authored-by: Eliran Zach <4012041+thehobbit85@users.noreply.github.com>
|
Landed on main as 4a1f683, keeping this as its own small fix. @thehobbit85 is credited as co-author because the implementation and failure-injection fixture were retained. Design and correctness review found no blockers: the failed ownership transition releases the opened DIR, preserves the allocation error, and leaves the output handle unpublished. This is a general root-cause fix with no game-specific rule or workaround. Independently reproduced the original null write under ASan/UBSan. All 19 real-handler checks pass with the fix, plain and under unsuppressed ASan/UBSan, including descriptor closure, untouched output handles, preserved errno and subsequent successful enumeration. The combined landing also passes the audio/save-sync fixtures and all 11 documentation checks. Published PR CI was green. Confidence is high. Regression risk is low because successful directory handling is unchanged; no gameplay regression run was warranted. Closing as landed. |
What and why
If folder-backed
OpenDirectoryopens a directory but cannot allocate its handle, it currently dereferences NULL and leaks the directory descriptor. Close the directory, preserve the allocation error, return the existing filesystem failure result, and leave the caller's output handle untouched.Addresses the allocation finding in issue #313, specifically this analysis comment. This focused change does not resolve the whole roadmap issue.
Current maintenance (2026-10-31, 22:00 UTC pass)
Rebased onto main
5474c6f34789a41770868564fbf4e9e32fc57584, producing83c0e31522e471e04e71abdd595bf8140a81a341without conflicts. The patch is identical by range-diff. Main still dereferences a failed directory-handle allocation without closing the opened directory, so this fix remains needed. All discussion comments, submitted reviews and inline threads were read; none are present.Fresh validation: the native real-handler directory fixture passed 19 checks, and all 17 documentation/benchmark tests passed;
git diff --checkpassed. Upstream audio-statistics production/test inputs match the maintained #730 checkout, whose freshly built fixture passed 22 checks. Earlier full host, sanitizer and Mac build results below remain historical because those unchanged suites were not repeated.Fresh harness discovery finds a Legends: Z-A build but no smoke route or baseline. No game run or device validation was performed; no gameplay result is claimed.
CI is pending for exact published head
83c0e31522e471e04e71abdd595bf8140a81a341. Earlier-head results are historical.Historical author validation on e999599
git diff --checkpasses.clang-tidyis unavailable locally; no lint pass is claimed and no tool was installed.regress.py impactmaps the directory/file calls; the intended five-game coverage is Hades, Breath of the Wild, Tears of the Kingdom, Pokémon Scarlet, and Pokémon Legends: Z-A, covering the known filesystem consumers and a newer SDK. No local game builds or configured harness paths exist, so these runs cannot be performed. Graphics code is unchanged; this fixture validates the host allocation/cleanup path directly.Docs
docs/reference/services.md; symptom index regenerated.