Cross-thread UAF: a spawned worker's arena-allocated return value is freed by arena_destroy at detach before the joiner reads it

[InauguralPhysicist]

Summary

A spawn'd worker stores its function's return value directly into h->result and then runs eigs_thread_detach(), which calls arena_destroy() and frees the worker's per-thread arena. If the return value was arena-allocated (the worker used arena_mark), it is freed at detach. Because thread_join/handle_table_drain pthread_join the worker first, the read of h->result happens strictly after the arena is gone — a deterministic cross-thread use-after-free.

This is the return-value analogue of the #293 channel fix: channel sends are deep-copied via val_clone_for_send precisely "so the copy survives the sender's arena_reset as well as detach" (src/eigenscript.c), but thread_entry applies no such copy to the worker's return value.

Mechanism

src/builtins.c thread_entry (VAL_FN path):

result = vm_execute(fn_chunk, call_env);
...
h->result = result;          // no promotion / deep-copy
env_decref(call_env);
...
eigs_thread_detach();        // -> arena_destroy() frees the worker arena
return NULL;

eigs_thread_detach() (src/state.c) calls arena_destroy() (src/arena.c), which does free(g_arena.blocks[i]) for every block — freeing all arena-allocated Values.

Value constructors allocate from the arena whenever g_arena.active is set:

// src/eigenscript.c  make_num / make_str / make_list / make_dict
int from_arena = g_arena.active;
v = from_arena ? arena_alloc(sizeof(Value)) : xcalloc(1, sizeof(Value));
v->arena = from_arena;

g_arena.active is turned on only by arena_mark_pos() (src/arena.c:173, via the arena_mark builtin) and off again by arena_reset_to_mark().

So a worker that calls arena_mark and then returns a value built while active (e.g. forgets / intends to defer arena_reset) returns arena memory. pthread_join guarantees the worker — and thus arena_destroy() — has completed before builtin_thread_join does:

pthread_join(h->tid, NULL);
Value *result = h->result ? h->result : make_null();   // reads freed arena memory
...
return result;                                          // hands UAF to the caller

handle_table_drain hits the same dangling pointer via val_decref(h->result) (and val_decref of an arena value reads v->arena/v->refcount out of the freed block).

In the single-threaded case the same "return an arena value" pattern is benign only because the main thread's arena lives until process exit; the per-worker arena destroyed at detach is what makes this asymmetric.

Reachability

arena_mark / arena_reset are globally registered builtins, so a worker can use the arena fast-path (it exists for the tensor/training and tight-loop paths used by the model and consumer projects). spawn + arena_mark + returning an arena-built value is the trigger. Uncommon, but reachable and silent in release builds (reads freed-but-not-yet-reused memory). Under an EIGS_VALGRIND build the arena reset/poison annotations would flag the access.

Suggested fix

Promote the worker result to the heap before detach, mirroring the channel path — e.g. in thread_entry for both the VAL_FN and VAL_BUILTIN results:

Value *owned = val_clone_for_send(result);   // forces heap allocation, survives arena_destroy
val_decref(result);
h->result = owned;

val_clone_for_send already toggles g_arena.active = 0 around a deep copy for exactly this "survive detach" reason. (A lighter-weight alternative is to promote only when result->arena is set, but a deep copy also covers a heap container holding arena-allocated children built during an active region.)

Suggested regression coverage

A worker function that does arena_mark, builds and returns a number/string/list, with no arena_reset, then thread_join and use the result — run under ASan and an EIGS_VALGRIND build; both should currently flag the freed-arena read.

Notes

Pre-existing/latent (not introduced by the v0.22.0 threading pass), but in the same family as #293 and surfaced while reviewing that pass. Related: #301 (the embed-teardown path that omits handle_table_drain).