Keep a save file open until the last call through its handle is out, so a CloseFile beside a read or write cannot redirect it.

fs_host.c file_of copied the slot's host file under the shared lock and let go of the lock, and read_file,
write_file, file_size, set_file_size and flush_file then used it with no lock, while close_file on another thread
cleared the slot and closed the file at once. Windows hands a closed HANDLE's value to the next file opened (the
audit's probe got it back in 100 of 100 rounds and read the other file through the stale value), so a call under
way went on in another file: a save write could land in the wrong one. Only the game's own misuse of a handle
reaches it; this is the open file half of AUDIT's F-1-09.

Each call now counts itself in its slot from file_of to a new file_release. close_file closes at once when no
call is under way and otherwise marks the slot closing; the last call out closes the file (file_closed, with the
save bookkeeping close_file had). Nothing waits for another call, the slot and so the guest's handle value is not
handed out again before the close, and a call after the CloseFile gets FS_R_INVALID_ARGUMENT, as before for a
closed handle. Holding the lock across the I/O was not taken: opens and closes on every guest thread would then
wait for save reads and writes, and write_file's save_touched can wait for a Commit.

Reproduced before the change with stand-in host files that reuse a closed handle's value: the new
tests/fs_file_use_check.py built against the old functions closed during a read, gave the next open the same
handle, and the read returned the other file's bytes. With the change it passes (a close during a read happens
after it, once; a call after the close is refused; 20000 close-and-open rounds beside a reading thread read no
foreign bytes), as do save_share_check, save_retry_check, save_zip_check and the compile checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

runtime/fs_host.c

@@ -38,7 +38,9 @@
 enum { M_FREE, M_SAVE, M_TEMP, M_CACHE, M_SAVE_RO };   // M_SAVE: the working copy; M_SAVE_RO: the committed save
 
 typedef struct { int kind; char name[32]; char root[1024]; } Mount;
-typedef struct { PlatFile *h; int mode, save; } File;   // h NULL = free; save: on a save-data mount
+// h NULL = free; save: on a save-data mount; users: calls using h now (file_of .. file_release); closing: CloseFile
+// came while users > 0, and the last of them closes h
+typedef struct { PlatFile *h; int mode, save; int32_t users; int closing; } File;
 typedef struct { char *name; int kind; uint64_t size; } Listed;
 typedef struct { Listed *e; int n, next, mode, used; } Dir;   // the directory's entries at open; used 0 = free
 
@@ -1145,15 +1147,45 @@ static uint32_t commit(const char *name) {
 }
 
 // ---- files
+// A guest handle's host file is used outside `lock` (a read or write may take long, and a save write waits for a
+// Commit first, save_touched), so a CloseFile on another thread could close it in the middle of a call. Windows hands
+// a closed HANDLE's value to the next file opened, so the call went on in another file: a save write could land in the
+// wrong one. Each call now counts itself in the slot from file_of to file_release. CloseFile closes at once when no
+// call is under way, and otherwise marks the slot; the last call out closes it. Nothing waits for another call, the
+// slot (and with it the guest's handle value) is not handed out again before the close, and a call that comes after
+// the CloseFile gets FS_R_INVALID_ARGUMENT, as a closed handle does.
 static File *file_of(uint64_t h, PlatFile **fh, int *mode) {
     if (h < FILE_BASE || h >= FILE_BASE + NFILES) return NULL;
     plat_mutex_lock_shared(&lock);
     File *f = &files[h - FILE_BASE];
-    PlatFile *v = f->h;
-    if (fh) *fh = f->h;
-    if (mode) *mode = f->mode;
+    int ok = f->h && !f->closing;
+    if (ok) {
+        __atomic_add_fetch(&f->users, 1, __ATOMIC_ACQ_REL);   // shared holders count at once; close_file reads it exclusive
+        if (fh) *fh = f->h;
+        if (mode) *mode = f->mode;
+    }
     plat_mutex_unlock_shared(&lock);
-    return v ? f : NULL;
+    return ok ? f : NULL;
+}
+
+// After the host file's close: the save's bookkeeping, as a closed save file needs.
+static void file_closed(PlatFile *h, int save) {
+    plat_file_close(h);
+    if (save) {
+        save_touched();
+        plat_dec32(&save_writers);
+    }
+}
+
+// The end of a call that file_of let in. The last call out of a slot CloseFile closed meanwhile closes the file.
+static void file_release(File *f) {
+    if (__atomic_sub_fetch(&f->users, 1, __ATOMIC_ACQ_REL)) return;
+    PlatFile *h = NULL;
+    int save = 0;
+    plat_mutex_lock(&lock);
+    if (f->h && f->closing && !__atomic_load_n(&f->users, __ATOMIC_ACQUIRE)) h = f->h, save = f->save, *f = (File){0};
+    plat_mutex_unlock(&lock);
+    if (h) file_closed(h, save);
 }
 
 // Positional reads: bytes read at off, 0 at the end, -1 on an error.
@@ -1184,7 +1216,7 @@ static uint32_t open_file(uint64_t *out, const char *path, int mode) {
     plat_mutex_lock(&lock);
     for (int i = 0; i < NFILES; i++)
         if (!files[i].h) {
-            files[i] = (File){f, mode, save_writer};
+            files[i] = (File){.h = f, .mode = mode, .save = save_writer};
             *out = FILE_BASE + (uint64_t)i;
             plat_mutex_unlock(&lock);
             if (save_writer) {
@@ -1201,8 +1233,12 @@ static uint32_t read_file(uint64_t *out, uint64_t h, int64_t offset, void *buffe
     if (romfs_handle(h)) return romfs_read(out, h, offset, buffer, size);
     PlatFile *f;
     int mode;
-    if (!out || offset < 0 || (!buffer && size) || !file_of(h, &f, &mode)) return FS_R_INVALID_ARGUMENT;
-    if (!(mode & FS_OPEN_READ)) return FS_R_OPEN_MODE;
+    File *file = !out || offset < 0 || (!buffer && size) ? NULL : file_of(h, &f, &mode);
+    if (!file) return FS_R_INVALID_ARGUMENT;
+    if (!(mode & FS_OPEN_READ)) {
+        file_release(file);
+        return FS_R_OPEN_MODE;
+    }
     uint64_t done = 0;
     while (done < size) {
         uint32_t chunk = size - done > 0x40000000 ? 0x40000000 : (uint32_t)(size - done);
@@ -1211,6 +1247,7 @@ static uint32_t read_file(uint64_t *out, uint64_t h, int64_t offset, void *buffe
         if (!n) break;
         done += (uint64_t)n;
     }
+    file_release(file);
     *out = done;
     return 0;
 }
@@ -1226,11 +1263,17 @@ static uint32_t write_file(uint64_t h, int64_t offset, const void *buffer, uint6
     int mode;
     File *file = offset < 0 || (!buffer && size) ? NULL : file_of(h, &f, &mode);
     if (!file) return FS_R_INVALID_ARGUMENT;
-    if (!(mode & FS_OPEN_WRITE)) return FS_R_OPEN_MODE;
+    if (!(mode & FS_OPEN_WRITE)) {
+        file_release(file);
+        return FS_R_OPEN_MODE;
+    }
     PlatFileId id;
     if (!plat_file_id(f, &id)) rt_fatal("fs: host save size");
     // offset + size without the sum: a size near 2^64 would wrap it below the file's size
-    if ((size > id.size || (uint64_t)offset > id.size - size) && !(mode & FS_OPEN_APPEND)) return FS_R_APPEND_REQUIRED;
+    if ((size > id.size || (uint64_t)offset > id.size - size) && !(mode & FS_OPEN_APPEND)) {
+        file_release(file);
+        return FS_R_APPEND_REQUIRED;
+    }
     if (file->save) save_touched();
     for (uint64_t done = 0; done < size;) {
         uint32_t chunk = size - done > 0x40000000 ? 0x40000000 : (uint32_t)(size - done);
@@ -1243,6 +1286,7 @@ static uint32_t write_file(uint64_t h, int64_t offset, const void *buffer, uint6
         done += (uint64_t)put;
     }
     if (option && (*option & 1)) plat_file_fsync(f);   // WriteOptionFlag_Flush
+    file_release(file);
     return 0;
 }
 
@@ -1250,7 +1294,11 @@ static uint32_t file_size(int64_t *out, uint64_t h) {
     if (romfs_handle(h)) return romfs_size(out, h);
     PlatFile *f;
     PlatFileId id;
-    if (!out || !file_of(h, &f, NULL) || !plat_file_id(f, &id)) return FS_R_INVALID_ARGUMENT;
+    File *file = out ? file_of(h, &f, NULL) : NULL;
+    if (!file) return FS_R_INVALID_ARGUMENT;
+    int ok = plat_file_id(f, &id);
+    file_release(file);
+    if (!ok) return FS_R_INVALID_ARGUMENT;
     *out = (int64_t)id.size;
     return 0;
 }
@@ -1260,16 +1308,23 @@ static uint32_t set_file_size(uint64_t h, int64_t size) {
     int mode;
     File *file = size < 0 ? NULL : file_of(h, &f, &mode);
     if (!file) return FS_R_INVALID_ARGUMENT;
-    if (!(mode & FS_OPEN_WRITE)) return FS_R_OPEN_MODE;
+    if (!(mode & FS_OPEN_WRITE)) {
+        file_release(file);
+        return FS_R_OPEN_MODE;
+    }
     if (file->save) save_touched();
-    return fs_result(plat_file_set_size(f, (uint64_t)size));
+    uint32_t r = fs_result(plat_file_set_size(f, (uint64_t)size));
+    file_release(file);
+    return r;
 }
 
 static uint32_t flush_file(uint64_t h) {
     PlatFile *f;
     int mode;
-    if (!file_of(h, &f, &mode)) return FS_R_INVALID_ARGUMENT;
+    File *file = file_of(h, &f, &mode);
+    if (!file) return FS_R_INVALID_ARGUMENT;
     if (mode & FS_OPEN_WRITE) plat_file_fsync(f);
+    file_release(file);
     return 0;
 }
 
@@ -1278,17 +1333,18 @@ static void close_file(uint64_t h) {
         romfs_close(h);
         return;
     }
-    if (!file_of(h, NULL, NULL)) rt_fatal("fs: close of invalid file handle %#llx", (unsigned long long)h);
+    File *f = h >= FILE_BASE && h < FILE_BASE + NFILES ? &files[h - FILE_BASE] : NULL;
+    PlatFile *close_now = NULL;
+    int open = 0, save = 0;
     plat_mutex_lock(&lock);
-    PlatFile *f = files[h - FILE_BASE].h;
-    int save = files[h - FILE_BASE].save;
-    files[h - FILE_BASE] = (File){0};
-    plat_mutex_unlock(&lock);
-    plat_file_close(f);
-    if (save) {
-        save_touched();
-        plat_dec32(&save_writers);
+    if (f && f->h && !f->closing) {
+        open = 1;
+        if (__atomic_load_n(&f->users, __ATOMIC_ACQUIRE)) f->closing = 1;   // the last call under way closes it
+        else close_now = f->h, save = f->save, *f = (File){0};
     }
+    plat_mutex_unlock(&lock);
+    if (!open) rt_fatal("fs: close of invalid file handle %#llx", (unsigned long long)h);
+    if (close_now) file_closed(close_now, save);
 }
 
 static uint32_t create_file(const char *path, int64_t size) {

tests/fs_file_use_check.py

@@ -0,0 +1,226 @@
+"""A save file's handle and a CloseFile on another thread (runtime/fs_host.c file_of, file_release, close_file): a
+call under way keeps its host file open, the close happens once when the last call is out, the guest's handle value
+is not handed out again before that, and a call after the close gets FS_R_INVALID_ARGUMENT. The host files here are
+stand-ins that hand a closed file's value to the next one opened, as Windows does with a HANDLE: a read that went on
+through a closed handle would read the other file. Builds fs_host.c's file functions into a test program. No game
+files.
+
+Usage: python tests/fs_file_use_check.py [path to clang]"""
+import os
+import re
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+MSYS_BIN = Path(r'C:\msys64\ucrt64\bin')
+source = (ROOT / 'runtime' / 'fs_host.c').read_text(encoding='utf-8')
+
+
+def compiler():
+    if len(sys.argv) > 1:
+        return sys.argv[1]
+    if os.environ.get('CC'):
+        return os.environ['CC']
+    if os.name == 'nt' and (MSYS_BIN / 'clang.exe').exists():
+        return str(MSYS_BIN / 'clang.exe')
+    return shutil.which('clang') or 'clang'
+
+
+def function(signature):
+    start = source.index(signature)
+    brace = source.index('{', start)
+    depth, end = 1, brace + 1
+    while depth:
+        depth += (source[end] == '{') - (source[end] == '}')
+        end += 1
+    return source[start:end]
+
+
+HARNESS = r'''
+#include <windows.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+typedef SRWLOCK PlatMutex;
+#define PLAT_MUTEX_INIT SRWLOCK_INIT
+static void plat_mutex_lock(PlatMutex *m) { AcquireSRWLockExclusive(m); }
+static void plat_mutex_unlock(PlatMutex *m) { ReleaseSRWLockExclusive(m); }
+static void plat_mutex_lock_shared(PlatMutex *m) { AcquireSRWLockShared(m); }
+static void plat_mutex_unlock_shared(PlatMutex *m) { ReleaseSRWLockShared(m); }
+static void plat_inc32(volatile int32_t *v) { InterlockedIncrement((volatile LONG *)v); }
+static void plat_dec32(volatile int32_t *v) { InterlockedDecrement((volatile LONG *)v); }
+static void rt_log(const char *fmt, ...) { (void)fmt; }
+__attribute__((noreturn)) static void rt_fatal(const char *fmt, ...) { printf("FATAL: %s\n", fmt); fflush(stdout); ExitProcess(99); }
+enum { FS_OPEN_READ = 1, FS_OPEN_WRITE = 2, FS_OPEN_APPEND = 4 };
+enum { FS_R_INVALID_ARGUMENT = 6001, FS_R_OPEN_MODE = 6002, FS_R_PATH_NOT_FOUND = 6003, FS_R_UNSUPPORTED_OPEN = 6004 };
+enum { M_FREE, M_SAVE, M_TEMP, M_CACHE, M_SAVE_RO };
+enum { PLAT_FS_FILE = 1 };
+#define FILE_BASE 0x1000
+#define NFILES 8
+#define PATH_CAP 64
+static int romfs_owns(const char *p) { (void)p; return 0; }
+static uint32_t romfs_open(uint64_t *o, const char *p, int m) { (void)o, (void)p, (void)m; return 1; }
+static int romfs_handle(uint64_t h) { (void)h; return 0; }
+static uint32_t romfs_read(uint64_t *o, uint64_t h, int64_t off, void *b, uint64_t n) { (void)o, (void)h, (void)off, (void)b, (void)n; return 1; }
+static void romfs_close(uint64_t h) { (void)h; }
+static void fs_trace(const char *w, const char *p) { (void)w, (void)p; }
+static uint32_t fs_result(int e) { return e ? 6099u : 0; }
+static void mem_thaw(uintptr_t a, uint64_t n) { (void)a, (void)n; }
+static uint32_t host_path_kind(const char *path, char *p, size_t n, int *kind) {
+    snprintf(p, n, "%s", path);
+    *kind = M_SAVE;
+    return 0;
+}
+static int plat_fs_kind(const char *p) { (void)p; return PLAT_FS_FILE; }
+static volatile int32_t save_writers;
+static void save_touched(void) {}
+
+// The host's files: four kernel objects; a close frees one, and the next open takes the lowest free one, so a closed
+// handle's value comes back for another file at once.
+typedef struct PlatFile { volatile LONG open, closes; char content; } PlatFile;
+static PlatFile kfiles[4];
+static SRWLOCK klock = SRWLOCK_INIT;
+static HANDLE hold_read, in_read;   // hold_read set: a read waits for it (and says so through in_read)
+static PlatFile *plat_fs_open(const char *p, int write, int *err) {
+    (void)write;
+    AcquireSRWLockExclusive(&klock);
+    for (int i = 0; i < 4; i++)
+        if (!kfiles[i].open) {
+            kfiles[i].open = 1, kfiles[i].content = p[0];
+            ReleaseSRWLockExclusive(&klock);
+            return &kfiles[i];
+        }
+    ReleaseSRWLockExclusive(&klock);
+    *err = 1;
+    return NULL;
+}
+static void plat_file_close(PlatFile *f) {
+    AcquireSRWLockExclusive(&klock);
+    if (!f->open) printf("FAIL: a host file closed twice\n");
+    f->open = 0, f->closes++;
+    ReleaseSRWLockExclusive(&klock);
+}
+static int64_t plat_file_pread(PlatFile *f, void *buf, uint32_t n, uint64_t off) {
+    (void)off;
+    if (hold_read) { SetEvent(in_read); WaitForSingleObject(hold_read, INFINITE); }
+    for (volatile int spin = 0; spin < 200; spin++) {}   // the window in which another thread may close and open
+    if (!f->open) return -1;
+    memset(buf, f->content, n);
+    return n;
+}
+'''
+
+MAIN = r'''
+static uint64_t h_read;
+static char got;
+static uint32_t got_r;
+static DWORD WINAPI reader(void *arg) {
+    (void)arg;
+    uint64_t n = 0;
+    char b[4] = {0};
+    got_r = read_file(&n, h_read, 0, b, 4);
+    got = b[0];
+    return 0;
+}
+
+static volatile LONG stop, foreign, invalid, reads;
+static volatile uint64_t current;   // the handle the looping reader reads: a file named "a"
+static DWORD WINAPI loop_reader(void *arg) {
+    (void)arg;
+    while (!stop) {
+        uint64_t n = 0;
+        char b[4] = {0};
+        uint32_t r = read_file(&n, current, 0, b, 4);
+        if (!r && b[0] != 'a') InterlockedIncrement(&foreign);
+        if (r == FS_R_INVALID_ARGUMENT) InterlockedIncrement(&invalid);
+        InterlockedIncrement(&reads);
+    }
+    return 0;
+}
+
+int main(void) {
+    int fails = 0;
+    uint64_t ha, hb, hc;
+    // 1. A read under way when CloseFile comes: it finishes in its own file; the close happens when it is out, once.
+    if (open_file(&ha, "a", FS_OPEN_READ | FS_OPEN_WRITE)) return printf("FAIL: open\n"), 1;
+    PlatFile *ka = files[ha - FILE_BASE].h;
+    hold_read = CreateEventW(NULL, TRUE, FALSE, NULL), in_read = CreateEventW(NULL, FALSE, FALSE, NULL);
+    h_read = ha;
+    HANDLE t = CreateThread(NULL, 0, reader, NULL, 0, NULL);
+    WaitForSingleObject(in_read, INFINITE);
+    close_file(ha);
+    int closed_early = ka->closes;
+    uint64_t n = 0;
+    char b[4];
+    HANDLE hr = hold_read;
+    hold_read = NULL;   // the calls below do not wait
+    uint32_t after = read_file(&n, ha, 0, b, 4);
+    open_file(&hb, "b", FS_OPEN_READ);
+    int reused = hb == ha || files[hb - FILE_BASE].h == ka;
+    SetEvent(hr);
+    WaitForSingleObject(t, INFINITE);
+    printf("close during a read: closed %s, a read after the close %s, the next open %s, the read got '%c' (%s), the "
+           "file closed %ld time(s)\n", closed_early ? "AT ONCE (FAIL)" : "after the read",
+           after == FS_R_INVALID_ARGUMENT ? "refused" : "ALLOWED (FAIL)", reused ? "TOOK ITS HANDLE (FAIL)" : "got another",
+           got, got_r || got != 'a' ? "FAIL" : "ok", (long)ka->closes);
+    if (closed_early || after != FS_R_INVALID_ARGUMENT || reused || got != 'a' || got_r || ka->closes != 1) fails++;
+    open_file(&hc, "c", FS_OPEN_READ);
+    if (hc != ha) printf("FAIL: the closed slot was not free again (%#llx)\n", (unsigned long long)hc), fails++;
+    close_file(hb), close_file(hc);
+
+    // 2. A reader loops over a file "a" while another thread closes it, opens "b" (which may take a closed handle's
+    //    value) and opens "a" again: no read returns b's bytes.
+    open_file(&ha, "a", FS_OPEN_READ);
+    current = ha;
+    HANDLE r = CreateThread(NULL, 0, loop_reader, NULL, 0, NULL);
+    for (int i = 0; i < 20000; i++) {
+        close_file(current);
+        open_file(&hb, "b", FS_OPEN_READ);
+        open_file(&ha, "a", FS_OPEN_READ);
+        current = ha;
+        close_file(hb);
+    }
+    stop = 1;
+    WaitForSingleObject(r, INFINITE);
+    close_file(current);
+    printf("a reader beside 20000 closes and opens: %ld reads, %ld refused after a close, %ld read another file's "
+           "bytes\n", (long)reads, (long)invalid, (long)foreign);
+    if (foreign) fails++;
+    int open_left = 0;
+    for (int i = 0; i < 4; i++) open_left += kfiles[i].open;
+    for (int i = 0; i < NFILES; i++) open_left += files[i].h != NULL;
+    if (open_left) printf("FAIL: %d files left open\n", open_left), fails++;
+    printf("fs_file_use_check: %s\n", fails ? "FAILED" : "ok");
+    return fails ? 1 : 0;
+}
+'''
+
+
+def main():
+    if os.name != 'nt':
+        print('fs_file_use_check: Windows only')
+        return 0
+    file_t = re.search(r'typedef struct \{ PlatFile \*h;[^\n]*\} File;', source).group(0)
+    parts = [HARNESS, file_t, 'static File files[NFILES];', 'static PlatMutex lock = PLAT_MUTEX_INIT;']
+    for sig in ('static File *file_of(', 'static void file_closed(', 'static void file_release(',
+                'static int64_t host_read_at(', 'static uint32_t open_file(', 'static uint32_t read_file(',
+                'static void close_file('):
+        parts.append(function(sig))
+    parts.append(MAIN)
+    work = ROOT / 'build' / 'test' / 'fs_file_use'
+    work.mkdir(parents=True, exist_ok=True)
+    (work / 'fs_file_use_check.c').write_text('\n'.join(parts), encoding='utf-8')
+    exe = work / 'fs_file_use_check.exe'
+    env = dict(os.environ)
+    if MSYS_BIN.exists():
+        env['PATH'] = str(MSYS_BIN) + os.pathsep + env.get('PATH', '')
+    subprocess.check_call([compiler(), '-O1', '-g', '-Wall', '-Wno-unused-function', str(work / 'fs_file_use_check.c'),
+                           '-o', str(exe)], env=env)
+    return subprocess.run([str(exe)], env=env, timeout=300).returncode
+
+
+if __name__ == '__main__':
+    sys.exit(main())