Skip to content

Commit ae25da3

Browse files
committed
frontend/drivers/platform_emscripten: NULL-check allocs in OPFS and FetchFS boot init
Four unchecked allocations in the emscripten platform driver's boot-time filesystem mount paths: === OPFS mount: strdup === char *parent = strdup(opfs_mount); path_parent_dir(parent, strlen(parent)); strdup unchecked; strlen(NULL) NULL-derefs on OOM before path_parent_dir even runs. === FetchFS manifest: calloc + strdup === char *line = calloc(sizeof(char), max_line_len); ... char *base_url = strdup(line); Pre-patch the calloc was unchecked. If it returned NULL: - getline() glibc/musl tolerate NULL line pointer (it allocates its own buffer internally), so the getline call itself would not immediately crash. - But the subsequent strdup(line) at line ~869 would run against whatever getline stored into line - if the manifest was empty getline would have returned -1 and we skip the strdup. If non-empty, getline would have allocated a buffer and the strdup works. - However, inside the while-loop at line ~885, getline is called again with &line still pointing at getline's internal allocation - still fine. - The real risk: if the initial calloc returned NULL and the manifest had exactly one line, we'd skip the 'base URL' parse but continue into wasmfs_create_fetch_backend which needs base_url. This path is robustly handled by the existing !base_url check inside the loop. So the calloc NULL-check is defensive rather than strictly required. But the consistency with the other allocations in this block argues for adding it. === FetchFS per-entry: two strdups === char *parent = strdup(realfs_path); path_parent_dir(parent, strlen(parent)); ... char *parent = strdup(fetchfs_path); path_parent_dir(parent, strlen(parent)); Both unchecked; strlen(NULL) NULL-derefs on OOM. === Fix: abort() on OOM matching the init policy === All the other failure paths in these init blocks call abort() (fopen failure, wasmfs_create_fetch_backend failure, wasmfs_create_file failure, mkdir failure). These are boot- time filesystem mount operations on the web build; if any of them fail there is no sensible recovery path. Match that policy: print a diagnostic and abort on each OOM. No functional change when memory is available. On a memory-starved browser tab (rare, but can happen on mobile or with many other tabs open) we get a clear abort with a log message pointing at the OOM site rather than a SIGSEGV from strlen/memcpy of NULL. === Swept-clean in the same pass === Other frontend/drivers/ files verified clean: - platform_unix.c: 7 sites, all NULL-checked with prior audit comments in place (android_app, savedState, newargv exec fork, inotify_data, path_change_data, voice_out / speed_out accessibility). - platform_darwin.m: 4 sites all guarded - watch_data / watches callocs with proper cleanup chain, change_data calloc with the prior-audit explicit-free-on-failure handling, the watches[i].path strdup at line ~1030 is intentionally unchecked because the field is only read by the teardown path (for free()) - strdup failure is benign, just means no debug-log path, teardown sees NULL and skips the free. - platform_ps2.c: 3 sites all intentionally designed around OOM - they're memory-probing loops that halve the request size until malloc succeeds or size reaches 0. Idiomatic for the platform. - platform_xdk.c: 2 sites both NULL-checked with prior audit comments. - platform_switch.c: 2 sites both NULL-checked (malloc with early-return, realloc with tmp-pattern + goto error). - platform_wii.c: 1 site NULL-checked. - platform_dos.c: 1 site NULL-checked with prior audit comment. - platform_ctr.c: 1 site NULL-checked via 'if (code_buffer)' wrapping the subsequent writes. - platform_emscripten.c main() platform data calloc: NULL-checked with prior audit comment - return 1 from main() on failure. Thread-safety: all four sites run on the emscripten main thread at process entry / mount setup time. No concurrency. Reachability: emscripten memory is browser-tab-bounded; OOM at process entry is rare but possible (especially on mobile devices with other tabs consuming the shared memory pool). The fix changes the failure mode from 'opaque SIGSEGV in strlen' to 'explicit abort with OOM log message' - strictly better for debugging.
1 parent dcaec9b commit ae25da3

1 file changed

Lines changed: 38 additions & 0 deletions

File tree

frontend/drivers/platform_emscripten.c

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,14 @@ static void platform_emscripten_mount_filesystems(void)
811811
backend_t opfs = wasmfs_create_opfs_backend();
812812
{
813813
char *parent = strdup(opfs_mount);
814+
/* NULL-check strdup: path_parent_dir/strlen on NULL parent
815+
* NULL-derefs. Match the abort() policy used for all
816+
* other failures in this boot-time init path. */
817+
if (!parent)
818+
{
819+
printf("[OPFS] out of memory duplicating mount path\n");
820+
abort();
821+
}
814822
path_parent_dir(parent, strlen(parent));
815823
if (!path_mkdir(parent))
816824
{
@@ -860,6 +868,22 @@ static void platform_emscripten_mount_filesystems(void)
860868
abort();
861869
}
862870
char *line = calloc(sizeof(char), max_line_len);
871+
/* NULL-check the calloc: getline below receives &line and
872+
* will realloc it if needed, but the getline interface
873+
* requires the initial pointer to be either NULL (fine)
874+
* or a valid malloc'd buffer. On OOM 'line' stays NULL
875+
* which the getline glibc implementation tolerates (it
876+
* will allocate itself), but e.g. musl's getline also
877+
* tolerates NULL so behaviour is consistent. However,
878+
* if the calloc succeeded and then later in this function
879+
* we strdup(line) at line ~869 / line ~899, those would
880+
* NULL-deref if line were NULL. Simplest correct path:
881+
* abort on OOM like the other boot-init failures. */
882+
if (!line)
883+
{
884+
printf("[FetchFS] out of memory allocating manifest line buffer\n");
885+
abort();
886+
}
863887
__len = max_line_len;
864888
if (getline(&line, &__len, file) == -1 || __len == 0)
865889
printf("[FetchFS] missing base URL suggest empty manifest, skipping fetch initialization\n");
@@ -909,6 +933,14 @@ static void platform_emscripten_mount_filesystems(void)
909933
/* Make the directories for link path */
910934
{
911935
char *parent = strdup(realfs_path);
936+
/* NULL-check: same pattern as opfs_mount strdup
937+
* above - path_parent_dir/strlen on NULL would
938+
* crash, abort to match boot-init policy. */
939+
if (!parent)
940+
{
941+
printf("[FetchFS] out of memory duplicating realfs path\n");
942+
abort();
943+
}
912944
path_parent_dir(parent, strlen(parent));
913945
if (!path_mkdir(parent))
914946
{
@@ -920,6 +952,12 @@ static void platform_emscripten_mount_filesystems(void)
920952
/* Make the directories for URL path */
921953
{
922954
char *parent = strdup(fetchfs_path);
955+
/* NULL-check: same pattern, abort on OOM. */
956+
if (!parent)
957+
{
958+
printf("[FetchFS] out of memory duplicating fetchfs path\n");
959+
abort();
960+
}
923961
path_parent_dir(parent, strlen(parent));
924962
if (!path_mkdir(parent))
925963
{

0 commit comments

Comments
 (0)