diff --git a/dm/host/hle_fs.c b/dm/host/hle_fs.c index 92fc95619d..5057229f15 100644 --- a/dm/host/hle_fs.c +++ b/dm/host/hle_fs.c @@ -982,7 +982,9 @@ HLE_FORCE("_ZN2nn2fs13OpenDirectoryEPNS0_15DirectoryHandleEPKci", "fsp-srv → o } HP(path); DIR *d = opendir(hp); if (!d) return err_res("opendir", hp); - dirh_t *h = calloc(1, sizeof *h); h->d = d; strlcpy(h->path, hp, sizeof h->path); h->mode = mode; + dirh_t *h = calloc(1, sizeof *h); + if (!h) { int e = errno; closedir(d); errno = e; return err_res("directory allocation", hp); } + h->d = d; strlcpy(h->path, hp, sizeof h->path); h->mode = mode; const char *c = strchr(path, ':'); if (c) for (int i = 0; i < g_nmnt; i++) if (g_mnt[i].overlay[0] && strlen(g_mnt[i].name) == (size_t)(c - path) && !strncmp(g_mnt[i].name, path, (size_t)(c - path))) { dir_snapshot(h, &g_mnt[i], c + 1); break; diff --git a/dm/host/tests/Makefile b/dm/host/tests/Makefile index b639c75ac9..8766efb8d2 100644 --- a/dm/host/tests/Makefile +++ b/dm/host/tests/Makefile @@ -110,6 +110,13 @@ $(B)/t_thread_leak: t_thread_leak.c ../thread.c ../dm.h $(GAME_H) | $(B) $(CLANG) -O1 -Wno-unused-function -I.. $(GINC) -o $@ t_thread_leak.c $(B)/t_fs: t_fs.c t_fs_stub.c ../hle_fs.c ../save_sync.c ../save_sync.h ../hle_bigstack.c ../hle.h ../dm.h $(GAME_H) | $(B) $(CLANG) -O1 -Wno-unused-function -I.. $(GINC) -o $@ t_fs.c t_fs_stub.c ../hle_bigstack.c ../save_sync.c +# Inject only the folder handle allocation; directory I/O remains real. +$(B)/t_fs_directory: t_fs_directory.c t_fs_stub.c ../hle_fs.c ../romfs_image.h ../save_sync.c ../save_sync.h ../hle_bigstack.c ../hle.h ../dm.h $(GAME_H) | $(B) + $(CLANG) -O1 -g -Wall -Werror -Wno-unused-function -I.. $(GINC) -o $@ t_fs_directory.c t_fs_stub.c ../hle_bigstack.c ../save_sync.c +.PHONY: fs-directory +fs-directory: $(B)/t_fs_directory + TMPDIR="$(abspath $(B))" $(B)/t_fs_directory +run: fs-directory $(B)/t_fs_commit_list: t_fs_commit_list.c t_fs_stub.c ../hle_fs.c ../save_sync.c ../save_sync.h ../hle_bigstack.c ../hle.h ../dm.h $(GAME_H) | $(B) $(CLANG) -O1 -Wall -Wno-unused-function -I.. $(GINC) -o $@ t_fs_commit_list.c t_fs_stub.c ../hle_bigstack.c ../save_sync.c $(B)/t_fs_commit_list_tsan: t_fs_commit_list.c t_fs_stub.c ../hle_fs.c ../save_sync.c ../save_sync.h ../hle_bigstack.c ../hle.h ../dm.h $(GAME_H) | $(B) diff --git a/dm/host/tests/t_fs_directory.c b/dm/host/tests/t_fs_directory.c new file mode 100644 index 0000000000..6d177ffa8e --- /dev/null +++ b/dm/host/tests/t_fs_directory.c @@ -0,0 +1,95 @@ +// Real folder-backed OpenDirectory: allocation failure must return an error, +// leave the output untouched, and close the DIR already opened by the handler. +#include +#include +#include + +static void *directory_calloc(size_t count, size_t size); +static DIR *directory_opendir(const char *path); +static int directory_closedir(DIR *d); +#define calloc directory_calloc +#define opendir directory_opendir +#define closedir directory_closedir +#include "../hle_fs.c" +#undef closedir +#undef opendir +#undef calloc + +struct dm_flags g_flag; +uint64_t g_t0_ns; +uint64_t now_ns(void) { return 0; } +void dm_guest_write_prepare(void *a, size_t n) { (void)a; (void)n; } + +static int fail_handle, failed_allocations, opened_fd = -1; +static void *directory_calloc(size_t count, size_t size) { + if (fail_handle && count == 1 && size == sizeof(dirh_t)) { + failed_allocations++; + errno = ENOMEM; + return NULL; + } + return calloc(count, size); +} +static DIR *directory_opendir(const char *path) { + DIR *d = opendir(path); + if (d) opened_fd = dirfd(d); + return d; +} + +static int directory_closedir(DIR *d) { + int r = closedir(d); + // Successful cleanup may change errno; it must not hide the allocation error. + if (fail_handle) errno = ENOENT; + return r; +} + +static int checks, fails; +#define CHECK(c, text) do { checks++; int ok = !!(c); fails += !ok; printf("%s %s\n", ok ? "PASS" : "FAIL", text); } while (0) + +int main(void) { + char root[1024], file[1100]; + const char *tmp = getenv("TMPDIR"); + snprintf(root, sizeof root, "%s/t_fs_directory.XXXXXX", tmp ? tmp : "/private/tmp"); + if (!mkdtemp(root)) { perror("mkdtemp"); return 2; } + snprintf(file, sizeof file, "%s/entry", root); + FILE *f = fopen(file, "w"); + if (!f) { perror("fopen"); rmdir(root); return 2; } + fputs("hello", f); fclose(f); + CHECK(mount("test", root, 0) == RES_OK, "mount the synthetic folder"); + + const uint64_t initial[] = {0, 0x1234}; + for (size_t i = 0; i < sizeof initial / sizeof initial[0]; i++) { + uint64_t h = initial[i]; + opened_fd = -1; failed_allocations = 0; fail_handle = 1; + Result r = h_fs_OpenDirectory(&h, "test:/", DIRMODE_DIR | DIRMODE_FILE); + int error = errno; + fail_handle = 0; + CHECK(failed_allocations == 1, "inject failure into the directory-handle allocation"); + CHECK(r == RES_FAIL, "failed allocation returns the filesystem resource failure"); + CHECK(error == ENOMEM, "allocation error survives directory cleanup"); + CHECK(h == initial[i], "failed open leaves the caller's handle untouched"); + CHECK(opened_fd >= 0, "the handler opened a real directory before allocation failed"); + errno = 0; + CHECK(fcntl(opened_fd, F_GETFD) == -1 && errno == EBADF, "failed open closes the real directory descriptor"); + } + h_fs_CloseDirectory(0); + + uint64_t h = 0; + Result r = h_fs_OpenDirectory(&h, "test:/", DIRMODE_DIR | DIRMODE_FILE); + int fd = opened_fd; + CHECK(r == RES_OK && h != 0, "a later successful open publishes a handle"); + if (r == RES_OK && h) { + CHECK(fcntl(fd, F_GETFD) >= 0, "successful open keeps its directory descriptor alive"); + int64_t count = -1; + CHECK(h_fs_DirCount(&count, h) == RES_OK && count == 1, "the successful handle counts the one synthetic file"); + DirectoryEntry entry[2] = {0}; + CHECK(h_fs_ReadDirectory(&count, entry, h, 2) == RES_OK && count == 1 && + !strcmp(entry[0].name, "entry") && entry[0].type == ENT_FILE && entry[0].size == 5, + "the successful handle reads the file name, type and size"); + h_fs_CloseDirectory(h); + errno = 0; + CHECK(fcntl(fd, F_GETFD) == -1 && errno == EBADF, "CloseDirectory releases the successful handle's descriptor"); + } + CHECK(!unlink(file) && !rmdir(root), "remove the synthetic fixture"); + printf("== fs directory: %d checks, %d failed ==\n", checks, fails); + return fails != 0; +} diff --git a/docs/agents/symptoms.md b/docs/agents/symptoms.md index 84313c92a7..937924bc39 100644 --- a/docs/agents/symptoms.md +++ b/docs/agents/symptoms.md @@ -144,6 +144,7 @@ One line per symptom in the [subsystem references](../reference/): what you see - a call the host hooks under its newer name reaches the SDK's own code on an older game. → [Older SDK names for the same calls](../reference/services.md#older-sdk-names-for-the-same-calls) - abort in `sm::Initialize` under `nn::friends::GetBlockedUserList` after the opening movie. → [SDK 4's `friends::GetBlockedUserList` without a `Uid`](../reference/services.md#sdk-4s-friendsgetblockeduserlist-without-a-uid) - the first loading screen never ends; `DM_FS=1` shows `save:/system` "not found" and no `fs mount save`. → [SDK 6's `account::OpenPreselectedUser` returns a Result](../reference/services.md#sdk-6s-accountopenpreselecteduser-returns-a-result) +- the host crashes in `h_fs_OpenDirectory` when its folder-backed directory handle allocation fails. → [Directory handle allocation failure](../reference/services.md#directory-handle-allocation-failure) - IL2CPP "Could not open …/global-metadata.dat" (then a crash in its logger). → [The SDK's own `fs::IsMounted` reads an empty mount table](../reference/services.md#the-sdks-own-fsismounted-reads-an-empty-mount-table) - a 10-byte read of the wrong RomFS file, then a null pointer at boot; files named `rom:\StartupUI.pak` are not found. → [File handles with low bits set, and backslash paths](../reference/services.md#file-handles-with-low-bits-set-and-backslash-paths) - Addressables never start (no `StreamingAssets/aa/settings.json` in the `DM_FS=1` output at all), then a null asset crash (`main+0x31100ec`). → [C library path checks ask the file system's limits](../reference/services.md#c-library-path-checks-ask-the-file-systems-limits) diff --git a/docs/reference/services.md b/docs/reference/services.md index 0608442f22..707df2ad7e 100644 --- a/docs/reference/services.md +++ b/docs/reference/services.md @@ -80,6 +80,13 @@ Most service calls can be found before they crash, before the first boot or the ## File system and save data +### Directory handle allocation failure + +- **Symptom:** the host crashes in `h_fs_OpenDirectory` when its folder-backed directory handle allocation fails. +- **Cause:** `opendir` succeeded, but the following `calloc` of `dirh_t` was dereferenced without a null check. Returning early without cleanup would also leak the already-open `DIR`. +- **Fix:** close the directory on allocation failure, preserve `errno` across cleanup, and return the existing filesystem failure through `err_res` before publishing a handle. The caller's output stays untouched, as on the other failed-open paths. `t_fs_directory.c` injects allocation failure in the real handler, checks the descriptor is closed and the output unchanged, then exercises successful open, count, read and close. +- **Found on:** the [GCC audit in issue #313](https://github.com/seedds/nxnative/issues/313#issuecomment-6523656102), reproduced with a synthetic directory and allocation fault injection; no game-specific failure was reported. + ### The SDK's own `fs::IsMounted` reads an empty mount table - **Symptom:** IL2CPP "Could not open …/global-metadata.dat" (then a crash in its logger).