diff --git a/runtime/audren.c b/runtime/audren.c index 104c058d..013c0b9f 100644 --- a/runtime/audren.c +++ b/runtime/audren.c @@ -717,8 +717,10 @@ void audren_manager_handler(IpcSession *s, const IpcRequest *rq, IpcResponse *rs AudrenParameter p = IPC_ARG(rq, AudrenParameter, 0); uint64_t work = IPC_ARG(rq, uint64_t, 0x38); need(rq->ncopy >= 1, "OpenAudioRenderer without a transfer memory handle"); - IpcTransferMemory *tm = kern_handle_get(rq->copy[0], KERN_TMEM); - need(tm && tm->size >= work, "OpenAudioRenderer work buffer is not a transfer memory of the stated size"); + // A copy made under the kernel lock: a CloseHandle of the transfer memory on another thread frees the object. + IpcTransferMemory tm; + int have_tm = kern_handle_copy(rq->copy[0], KERN_TMEM, &tm, sizeof tm); + need(have_tm && tm.size >= work, "OpenAudioRenderer work buffer is not a transfer memory of the stated size"); AudioRenderer *r = renderer_new(&p); if (rt_knob("AUDIO_STATS", "0")) { r->stats = calloc(1, sizeof *r->stats); @@ -743,7 +745,7 @@ void audren_manager_handler(IpcSession *s, const IpcRequest *rq, IpcResponse *rs "splitters %d/%d, REV%d, work buffer %#llx+%#llx", p.sample_rate, p.sample_count, p.mix_buffer_count, p.voice_count, p.effect_count, p.submix_count, p.sink_count, p.splitter_count, p.splitter_destination_count, revision_number(p.revision), - (unsigned long long)tm->addr, (unsigned long long)work); + (unsigned long long)tm.addr, (unsigned long long)work); rs->move[rs->nmove++] = ipc_session_new("IAudioRenderer", renderer_handler, r, renderer_close); return; } diff --git a/runtime/host.h b/runtime/host.h index 7bed58b7..ef94b596 100644 --- a/runtime/host.h +++ b/runtime/host.h @@ -344,7 +344,11 @@ void kern_core_diag_frame(uint64_t frame, uint64_t interval_ns); // perf_prese void kern_sched_report(uint64_t frame); // NX2NAVI_TRACE=sched: per-thread time split, on present uint32_t kern_handle_new(int kind, void *obj); // kind: KERN_SESSION or KERN_TMEM (ipc.c) void *kern_handle_get(uint32_t h, int kind); -void kern_handle_close(uint32_t h); // CloseHandle from the host side, any thread; fatal on a +void *kern_handle_hold(uint32_t h, int kind, void (*hold)(void *obj)); // kern_handle_get, hold(obj) run under the + // kernel lock (a reference the caller drops) +int kern_handle_copy(uint32_t h, int kind, void *out, size_t n); // a copy of the object, made under the + // kernel lock; 0 for a bad handle +void kern_handle_close(uint32_t h); // CloseHandle from the host side, any thread; fatal on a // bad handle enum { KERN_SESSION = 4, KERN_TMEM = 5 }; uint32_t kern_event_new(void); // writable handle diff --git a/runtime/ipc.c b/runtime/ipc.c index 98c53693..c93b1d91 100644 --- a/runtime/ipc.c +++ b/runtime/ipc.c @@ -254,6 +254,9 @@ static uint32_t session_clone(IpcSession *s) { return kern_handle_new(KERN_SESSION, s); } +// kern_handle_hold's callback: a request's reference, dropped through ipc_handle_closed once the request is answered. +static void session_hold(void *o) { plat_inc32(&((IpcSession *)o)->refs); } + void ipc_handle_closed(int kind, void *obj) { if (kind == KERN_TMEM) { free(obj); @@ -529,7 +532,10 @@ int ipc_svc(Cpu *c, uint32_t imm) { } case 0x21: case 0x22: { // SendSyncRequest(w0 handle) / WithUserBuffer(x0 buffer, x1 size, w2 handle) uint32_t h = (uint32_t)(imm == 0x21 ? x[0] : x[2]); - IpcSession *s = kern_handle_get(h, KERN_SESSION); + // The request holds a reference on the session until it is answered: a CloseHandle of the session on another + // thread meanwhile leaves the object, and its close callback (renderer_close frees the audio renderer the + // handler uses), to whichever drops the last reference. + IpcSession *s = kern_handle_hold(h, KERN_SESSION, session_hold); if (!s) { rt_log("ipc: svc %#x on %#x, not a session handle", imm, h); x[0] = IPC_R_BAD_HANDLE; @@ -542,6 +548,7 @@ int ipc_svc(Cpu *c, uint32_t imm) { t_req = c; dispatch(s, m); t_req = outer; + ipc_handle_closed(KERN_SESSION, s); // the request's reference x[0] = 0; return 1; } diff --git a/runtime/kernel.c b/runtime/kernel.c index 41b946e0..8c17ea89 100644 --- a/runtime/kernel.c +++ b/runtime/kernel.c @@ -2090,6 +2090,26 @@ void *kern_handle_get(uint32_t h, int kind) { return o; } +// kern_handle_get that also runs hold(obj) holding G, so the caller can take a reference before a CloseHandle on +// another thread drops the handle's own: handle_close clears the slot holding G and only then calls ipc_handle_closed. +void *kern_handle_hold(uint32_t h, int kind, void (*hold)(void *obj)) { + lock(); + void *o = h_peek(h, kind); + if (o) hold(o); + unlock(); + return o; +} + +// Copies n bytes of a handle's object holding G, for objects without a reference count (KERN_TMEM, freed by its +// CloseHandle): 1 when h is a handle of that kind, else 0 and nothing copied. +int kern_handle_copy(uint32_t h, int kind, void *out, size_t n) { + lock(); + const void *o = h_peek(h, kind); + if (o) memcpy(out, o, n); + unlock(); + return o != NULL; +} + uint32_t kern_event_new(void) { Event *e = calloc(1, sizeof *e); if (!e) rt_fatal("kernel: event allocation"); diff --git a/tests/ipc_session_check.c b/tests/ipc_session_check.c new file mode 100644 index 00000000..79e0cf22 --- /dev/null +++ b/tests/ipc_session_check.c @@ -0,0 +1,151 @@ +// runtime/ipc.c with runtime/kernel.c linked: what a CloseHandle racing a request does to the session and the +// transfer memory the request uses, with the platform layer linked and the rest of the runtime stubbed. Built and run +// by tests/ipc_session_check.py; prints each failed check and exits 1 on any. +// session a request whose handler blocks while another thread closes the session's only handle: the session's +// close callback runs once, after the handler returned, never while it runs +// tmem kern_handle_copy hands out the transfer memory's fields while the handle is open, and 0 after its close +#include "ipc.c" + +#include + +static int fails; +#define T_CHECK(c, ...) do { if (!(c)) { fails++; printf("FAIL line %d: ", __LINE__); printf(__VA_ARGS__); printf("\n"); } } while (0) + +// ---- the runtime ipc.c and kernel.c reach +unsigned rt_trace; +uint64_t rt_guest_start_ns; +volatile uint32_t prof_replay_tid, prof_main_tid; +const int rt_game_version = 111; +int mem_readable(uint64_t a, uint64_t n) { (void)a, (void)n; return 0; } +int mem_host_readable(uint64_t a, uint64_t n) { (void)a, (void)n; return 0; } +const char *cfg_get(const char *name) { (void)name; return NULL; } +const char *cfg_value(const char *name) { (void)name; return "0"; } +int cfg_bool(const char *name) { (void)name; return 0; } +int rt_ab(void) { return 0; } +int rt_knob(const char *name, const char *def) { (void)name; return strcmp(def, "0") != 0; } +int64_t frame_step_world_us(void) { return -1; } +const char *ldr_describe(uint64_t a, char *buf, int n) { snprintf(buf, (size_t)n, "%#llx", (unsigned long long)a); return buf; } +uint64_t ldr_symbol(const char *name) { (void)name; return 0; } +void ldr_load(const char *exefs_dir) { (void)exefs_dir; } +const RtLoaded *ldr_module(int i) { (void)i; return NULL; } +uint64_t ldr_plt_stub(const char *module, const char *name) { (void)module, (void)name; return 0; } +void perf_main_thread(void) {} +void perf_size(PerfSizes *s, const char *name, uint64_t count, uint64_t limit, uint64_t bytes) { + (void)s, (void)name, (void)count, (void)limit, (void)bytes; +} +int rt_linked_modules(RtModule *out, int max) { (void)out, (void)max; return 0; } +const RtGuestLayout *mem_layout(void) { return NULL; } +const RtConfig *rt_config(void) { return NULL; } +RtStats rt_stats(void) { return (RtStats){0, 0, 0}; } +void *mem_kernel_alloc(uint64_t size, uint64_t align) { + void *p = plat_aligned_alloc(size, align < 64 ? 64 : align); + if (!p) rt_fatal("out of memory"); + memset(p, 0, size); + return p; +} +void mem_kernel_free(void *p, uint64_t size) { (void)size; plat_aligned_free(p); } +void rt_log(const char *fmt, ...) { (void)fmt; } +void rt_logf(const char *fmt, ...) { (void)fmt; } +void rt_log_raw(const char *p, size_t n) { (void)p, (void)n; } +void rt_fatal(const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + printf("FATAL: "); + vprintf(fmt, ap); + printf("\n"); + va_end(ap); + fflush(stdout); + _Exit(3); +} +void rt_dispatch(Cpu *c) { (void)c; rt_fatal("rt_dispatch in the test"); } +void rt_svc(Cpu *c, uint32_t imm, uint64_t pc) { + (void)pc; + if (!ipc_svc(c, imm)) kern_svc(c, imm); +} +uint64_t rt_call(Cpu *c, uint64_t fn, int n, ...) { (void)c, (void)fn, (void)n; rt_fatal("rt_call in the test"); } +int ov_handheld(void) { return 0; } +long audren_renderers_live(void) { return 0; } +void audren_manager_handler(IpcSession *s, const IpcRequest *rq, IpcResponse *rs) { (void)s, (void)rq, (void)rs; } +void audio_dmg_handler(IpcSession *s, const IpcRequest *rq, IpcResponse *rs) { (void)s, (void)rq, (void)rs; } +void audio_sysmix_handler(IpcSession *s, const IpcRequest *rq, IpcResponse *rs) { (void)s, (void)rq, (void)rs; } +void nfp_user_manager_handler(IpcSession *s, const IpcRequest *rq, IpcResponse *rs) { (void)s, (void)rq, (void)rs; } + +// ---- a service whose command 7 blocks until released, and whose state its close callback frees +typedef struct { volatile int alive, entered, release, done, closes, closed_while_running; } Svc; + +static void blocking_handler(IpcSession *s, const IpcRequest *rq, IpcResponse *rs) { + Svc *v = s->state; + if (rq->cmd == 7) { + __atomic_store_n(&v->entered, 1, __ATOMIC_SEQ_CST); + while (!__atomic_load_n(&v->release, __ATOMIC_SEQ_CST)) plat_wait_ns(200000); + T_CHECK(v->alive, "the service's state is still alive when the handler resumes"); + } + __atomic_store_n(&v->done, 1, __ATOMIC_SEQ_CST); + rs->result = 0; +} + +static void svc_close(void *state) { + Svc *v = state; + if (!__atomic_load_n(&v->done, __ATOMIC_SEQ_CST)) v->closed_while_running = 1; + v->alive = 0; + v->closes++; +} + +typedef struct { Cpu cpu; uint32_t msg[IPC_MSG_BYTES / 4]; uint32_t h, result, kernel; } Client; + +static void client_main(void *arg) { + Client *cl = arg; + cl->cpu.tpidrro = (uint64_t)(uintptr_t)cl->msg; + Reply r; + cl->kernel = cmd(&cl->cpu, cl->h, 7, NULL, 0, &r); + cl->result = r.result; +} + +static void session_check(void) { + long live0 = ipc_sessions_live(); + static Svc v; + v.alive = 1; + static Client cl; + cl.h = ipc_session_new("blocking", blocking_handler, &v, svc_close); + void *t = plat_thread_new(client_main, &cl, "client"); + while (!__atomic_load_n(&v.entered, __ATOMIC_SEQ_CST)) plat_wait_ns(200000); + kern_handle_close(cl.h); // the session's only handle, closed while its request runs + plat_wait_ns(50000000); + T_CHECK(v.closes == 0, "the close callback waits for the running request (ran %d times)", v.closes); + T_CHECK(v.alive, "the service's state outlives the CloseHandle while a request runs"); + __atomic_store_n(&v.release, 1, __ATOMIC_SEQ_CST); + plat_thread_join(t); + T_CHECK(cl.kernel == 0 && cl.result == 0, "the request is answered (kernel %#x, result %#x)", cl.kernel, cl.result); + T_CHECK(v.closes == 1, "the close callback runs once after the request (ran %d times)", v.closes); + T_CHECK(!v.closed_while_running, "the close callback never runs while the handler does"); + T_CHECK(ipc_sessions_live() == live0, "the session is freed (%ld live, %ld before)", ipc_sessions_live(), live0); + // A request on an open session leaves it open; the CloseHandle afterwards frees it. + static Svc w; + w.alive = 1, w.release = 1; + static Client c2; + c2.h = ipc_session_new("open", blocking_handler, &w, svc_close); + client_main(&c2); + T_CHECK(c2.kernel == 0 && w.closes == 0, "a request leaves an open session open (closes %d)", w.closes); + kern_handle_close(c2.h); + T_CHECK(w.closes == 1 && ipc_sessions_live() == live0, "its CloseHandle frees it (closes %d)", w.closes); +} + +static void tmem_check(void) { + IpcTransferMemory *t = malloc(sizeof *t); + *t = (IpcTransferMemory){0x80000000ull, 0x10000, 3}; + uint32_t h = kern_handle_new(KERN_TMEM, t); + IpcTransferMemory copy = {0}; + T_CHECK(kern_handle_copy(h, KERN_TMEM, ©, sizeof copy) && copy.addr == 0x80000000ull && copy.size == 0x10000, + "the transfer memory is copied while its handle is open"); + T_CHECK(!kern_handle_copy(h, KERN_SESSION, ©, sizeof copy), "a handle of another kind copies nothing"); + kern_handle_close(h); + T_CHECK(!kern_handle_copy(h, KERN_TMEM, ©, sizeof copy), "a closed transfer memory handle copies nothing"); +} + +int main(void) { + setvbuf(stdout, NULL, _IONBF, 0); + session_check(); + tmem_check(); + printf(fails ? "ipc_session_check: %d failed\n" : "ipc_session_check: ok\n", fails); + return fails ? 1 : 0; +} diff --git a/tests/ipc_session_check.py b/tests/ipc_session_check.py new file mode 100644 index 00000000..51f6bc16 --- /dev/null +++ b/tests/ipc_session_check.py @@ -0,0 +1,54 @@ +"""Build and run tests/ipc_session_check.c: runtime/ipc.c with runtime/kernel.c linked, checking that a CloseHandle racing +a request leaves the session (and its close callback) to the request's end and that a closed transfer memory handle is +not read; with the platform layer linked and the rest of the runtime stubbed. No game files: the module layout the +runtime includes is a stand-in written to the test's build folder. + +Usage: ipc_session_check.py +""" +import os +import shutil +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +MSYS_BIN = Path(r'C:\msys64\ucrt64\bin') + + +def compiler(): + 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' + +LAYOUT = '''// A stand-in for the generated layout.h (tests/ipc_session_check.py). +#pragma once +#define MOD_MAIN_BASE 0x7000000000ull +#define MOD_MAIN_SIZE 0x4ea0000ull +''' + + +def main(): + work = ROOT / 'build' / 'test' / 'ipc_session' + work.mkdir(parents=True, exist_ok=True) + (work / 'layout.h').write_text(LAYOUT) + win = os.name == 'nt' + exe = work / ('ipc_session_check.exe' if win else 'ipc_session_check') + env = dict(os.environ) + if win and MSYS_BIN.exists(): + env['PATH'] = str(MSYS_BIN) + os.pathsep + env.get('PATH', '') + rt = ROOT / 'runtime' + plat = [rt / 'platform' / ('win32.c' if win else 'linux.c'), rt / 'platform' / 'common.c'] + libs = ['-lbcrypt', '-lws2_32', '-lpsapi', '-lwinmm', '-ldbghelp', '-lshell32', '-lole32', '-luser32', '-ladvapi32'] if win \ + else ['-lpthread'] + subprocess.check_call([compiler(), '-O1', '-g', '-Wall', '-Wno-unused-function', '-fno-strict-aliasing', + '-march=x86-64-v3', '-I', str(rt), '-I', str(work), + str(ROOT / 'tests' / 'ipc_session_check.c'), str(rt / 'kernel.c'), *map(str, plat), + '-o', str(exe), '-lm', *libs], + env=env) + return subprocess.run([str(exe)], env=env, timeout=300).returncode + + +if __name__ == '__main__': + sys.exit(main())