Skip to content

Fix MT use-after-free: module-env observer-slot growth vs unlocked reads (#694) #701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
InauguralPhysicist merged 1 commit into from
Jul 25, 2026
Merged

Fix MT use-after-free: module-env observer-slot growth vs unlocked reads (#694) #701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,27 @@ All notable changes to EigenScript are documented here.
by pointing the instrument at itself.

### Fixed
- **MT use-after-free: module-env observer-slot growth raced unlocked worker
reads (#694).** `observer_slot_update_e` grew `env->obs` with a bare
`realloc` + non-atomic pointer store while holding no lock, and the
`report of` / `when is` / predicate opcodes read a chain-resolved env's
`obs` array outside any lock. A worker reading a MODULE binding's observer
slot concurrently with the main thread first-observing a NEW module binding
therefore walked a freed array — the exact use-after-free class #607 fixed
for `names`/`values`/`assign_counts`, which `obs` was never brought into.
The window is narrow (module obs grows only on a binding's *first*
observation, only on the main thread), which is why it never bit in
practice. `obs` now follows the same #607 discipline: growth on a shared
module env copies into a fresh block under `g_module_env_lock`, RETIRES the
old one (freed at park/destroy) instead of freeing it, and publishes the
pointer with a release store — then the cap, also release. Readers go
through the new `env_obs_slot()` accessor, which loads cap *then* pointer
with acquires; the reverse order would admit new-cap-with-old-block, an
out-of-bounds read of the retired array. Single-threaded and thread-local
frame envs keep the plain realloc and plain loads behind one
predicted-false branch, so there is no cost off the MT path. New TSan
regression `test_obs_mt_race.eigs` (in the `test_tsan.sh` race-free slice)
reports 5 data races without the fix and 0 with it.
- **`report_value` no longer calls a monotone converging sequence
`diverging` (#674).** The value channel's raw-step divergence test (#422)
judged a window's steps "non-vanishing" when the recent half's mean
Expand Down
76 changes: 63 additions & 13 deletions src/eigenscript.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,18 +536,22 @@ void observer_slot_record_value(ObserverSlot *s, double v) {
s->v_used = 1;
}

/* #694: grow e->obs to cover `idx`. Defined with the #607 module-env MT
* helpers below (it needs the lock + retire machinery). Returns 0 on OOM. */
static int observer_obs_grow(Env *e, int idx);

/* Core: fold a precomputed entropy into binding slot `idx` of env `e`. */
static void observer_slot_update_e(Env *e, int idx, double new_entropy) {
if (!e || idx < 0) return;
if (idx >= e->obs_cap) {
int ncap = idx + 8;
ObserverSlot *no = realloc(e->obs, (size_t)ncap * sizeof(ObserverSlot));
if (!no) return;
memset(no + e->obs_cap, 0, (size_t)(ncap - e->obs_cap) * sizeof(ObserverSlot));
e->obs = no;
e->obs_cap = ncap;
}
ObserverSlot *s = &e->obs[idx];
if (idx >= e->obs_cap && !observer_obs_grow(e, idx)) return;
/* Acquire-load the (possibly just-republished) block rather than reading
Comment on lines 544 to +547
* e->obs plainly: a worker assigning a module binding can reach here while
* the main thread grows. Which block a racing writer lands in is #607's
* declared out-of-scope case (same-slot value semantics, not array memory
* safety) — retirement keeps either block alive — but the POINTER word
* itself must still be synchronized. */
ObserverSlot *s = env_obs_slot(e, idx);
if (!s) return;
s->prev_dH = s->dH;
if (!s->used || s->obs_age == 0) {
s->dH = 0; /* first observation of this binding */
Expand All @@ -565,17 +569,19 @@ void observer_slot_update(Env *e, int idx, Value *newval) {
observer_slot_update_e(e, idx, compute_entropy(newval));
/* #294 also fold the raw value into the value-signal channel (numbers only:
* the relative-delta step is only defined for a scalar trajectory). */
if (newval && newval->type == VAL_NUM && e && idx >= 0 && idx < e->obs_cap)
observer_slot_record_value(&e->obs[idx], newval->data.num);
if (newval && newval->type == VAL_NUM) {
ObserverSlot *s = env_obs_slot(e, idx);
if (s) observer_slot_record_value(s, newval->data.num);
}
}

/* #262 Phase-3 D: update a binding's observer slot from a raw immediate
* number, so the default path can observe without promoting the num to a
* tracked Value. Same trajectory math as observer_slot_update. */
void observer_slot_update_num(Env *e, int idx, double num) {
observer_slot_update_e(e, idx, entropy_of_num(num));
if (e && idx >= 0 && idx < e->obs_cap) /* #294 value-signal channel */
observer_slot_record_value(&e->obs[idx], num);
ObserverSlot *vs = env_obs_slot(e, idx); /* #294 value-signal channel */
if (vs) observer_slot_record_value(vs, num);
}

void observer_slot_reset(Env *e) {
Expand Down Expand Up @@ -1697,6 +1703,50 @@ static void env_grow_retire(Env *env, int new_cap) {
env->capacity = new_cap;
}

/* #694: grow the observer-slot array to cover binding `idx`.
*
* Single-threaded (and for every thread-local frame/loop env) this stays the
* plain realloc it always was. On a SHARED module env under MT it mirrors the
* #607 discipline exactly, because the hazard is identical: worker threads
* read a chain-resolved module env's obs array with no lock held (report of /
* when is on a module binding), while the main thread first-observes a new
* module binding and grows it. A realloc there frees the block a worker is
* still walking — the same use-after-free class #607 fixed for
* names/values/assign_counts, and the reason this one survived is that module
* obs grows only on the FIRST observation of a binding, so the window is
* narrow enough never to have been hit in practice.
*
* So: copy into a fresh block, RETIRE the old one (freed at park/destroy,
* bounded by the same geometric-waste argument), publish the pointer with a
* release store, and only then publish the wider cap — see env_obs_slot for
* why that store order is load-bearing. */
static int observer_obs_grow(Env *e, int idx) {
int ncap = idx + 8;
size_t nsz = (size_t)ncap * sizeof(ObserverSlot);
size_t osz = (size_t)e->obs_cap * sizeof(ObserverSlot);
if (!env_mt_shared(e)) {
ObserverSlot *no = realloc(e->obs, nsz);
if (!no) return 0;
memset((char *)no + osz, 0, nsz - osz);
e->obs = no;
e->obs_cap = ncap;
return 1;
}
pthread_mutex_lock(&g_module_env_lock);
if (idx >= e->obs_cap) { /* re-check: another grow may have won */
osz = (size_t)e->obs_cap * sizeof(ObserverSlot);
ObserverSlot *no = malloc(nsz);
if (!no) { pthread_mutex_unlock(&g_module_env_lock); return 0; }
if (e->obs) memcpy(no, e->obs, osz);
memset((char *)no + osz, 0, nsz - osz);
env_retire_block(e, e->obs);
__atomic_store_n(&e->obs, no, __ATOMIC_RELEASE);
__atomic_store_n(&e->obs_cap, ncap, __ATOMIC_RELEASE);
}
pthread_mutex_unlock(&g_module_env_lock);
return 1;
}

/* Hash rebuild for a shared env under MT: same sizing as
* env_hash_rebuild, but the old bucket arrays are retired, not freed —
* belt-and-suspenders (probes are lock-serialized, but a freed bucket
Expand Down
27 changes: 27 additions & 0 deletions src/eigenscript.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,33 @@ static inline int *env_assign_counts_ptr(Env *e) {
return e->assign_counts;
}

/* #694: bounds-checked observer-slot access for an env the MAIN thread may
* grow concurrently. Same class as env_values_ptr, but obs needs the CAP and
* the POINTER to be read consistently, so the two are ordered against each
* other: the writer (observer_obs_grow) publishes the new block with a
* release store and only THEN the new cap, also release. A reader that loads
* cap first (acquire) and the pointer second (acquire) therefore sees either
* - the old cap, with an old-or-new block — both hold >= old_cap slots,
* since the new block is a superset copy and the old one is retired, or
* - the new cap, which by the release/acquire chain guarantees the new
* block is already visible.
* Reading them in the other order would admit new-cap-with-old-block, i.e. an
* out-of-bounds read of the retired array. Returns NULL when idx is out of
* range, so callers replace the `idx < e->obs_cap && e->obs[idx].used` idiom
* with a null check. Single-threaded: plain loads behind one predicted-false
* branch. */
static inline struct ObserverSlot *env_obs_slot(Env *e, int idx) {
if (!e || idx < 0) return NULL;
if (__builtin_expect(g_vm_multithreaded, 0)) {
int cap = __atomic_load_n(&e->obs_cap, __ATOMIC_ACQUIRE);
if (idx >= cap) return NULL;
struct ObserverSlot *o = __atomic_load_n(&e->obs, __ATOMIC_ACQUIRE);
return o ? &o[idx] : NULL;
}
if (idx >= e->obs_cap || !e->obs) return NULL;
return &e->obs[idx];
}

Env* env_new(Env *parent);
void env_set(Env *env, const char *name, Value *val);
Value* env_get(Env *env, const char *name);
Expand Down
73 changes: 38 additions & 35 deletions src/vm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,8 @@ Value* builtin_observe(Value *arg);
* disagree on loop classification (the same lockstep invariant the opcode
* encoding enforces). Returns 1 if (*dH, *ent) were filled. */
static inline int obs_stall_trajectory(double *dH, double *ent) {
if (g_last_obs_slot_env &&
g_last_obs_slot_idx >= 0 &&
g_last_obs_slot_idx < g_last_obs_slot_env->obs_cap &&
g_last_obs_slot_env->obs[g_last_obs_slot_idx].used) {
const ObserverSlot *s = &g_last_obs_slot_env->obs[g_last_obs_slot_idx];
const ObserverSlot *s = env_obs_slot(g_last_obs_slot_env, g_last_obs_slot_idx);
if (s && s->used) {
*dH = s->dH; *ent = s->entropy;
return 1;
}
Expand Down Expand Up @@ -163,7 +160,7 @@ static void obs_dump_scope(const char *scope, Env *e, int shared,
* (OBSERVE_ASSIGN_LOCAL / OBSERVE_NAME_POST), and recycled call envs
* reset both counters (vm_park_call_env), preserving per-call
* semantics: max() of the two is the true assignment count. */
const ObserverSlot *os = (e->obs && i < e->obs_cap) ? &e->obs[i] : NULL;
const ObserverSlot *os = env_obs_slot(e, i);
long when = e->assign_counts ? e->assign_counts[i] : -1;
if (os && os->used && os->obs_age > when) when = os->obs_age;
char whenbuf[16];
Expand Down Expand Up @@ -1260,8 +1257,9 @@ void jit_helper_report_slot(int slot) {
CallFrame *frame = &g_vm.frames[g_vm.frame_count - 1];
Env *e = frame->fn_env;
Value *result;
if (e && slot < e->obs_cap && e->obs[slot].used) {
result = make_str(observer_slot_report(&e->obs[slot]));
const ObserverSlot *os_r = env_obs_slot(e, (int)slot);
if (os_r && os_r->used) {
result = make_str(observer_slot_report(os_r));
} else {
result = make_str("equilibrium"); /* unobserved binding — no trajectory */
}
Expand Down Expand Up @@ -1291,8 +1289,8 @@ void jit_helper_observe_name_post(EigsChunk *chunk, int name_idx) {
/* #262 Phase-3 D2: slot-source the temporal snapshot — mirror of the
* interpreter CASE(OBSERVE_NAME_POST). */
if (__builtin_expect(g_trace_obs_hist, 0)) {
const ObserverSlot *os = &oe->obs[oidx];
trace_record_obs(name, os->entropy, os->dH, os->last_entropy);
const ObserverSlot *os = env_obs_slot(oe, oidx);
if (os) trace_record_obs(name, os->entropy, os->dH, os->last_entropy);
}
}
}
Expand Down Expand Up @@ -4374,8 +4372,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
uint16_t slot = read_u16(ip); ip += 2;
Env *e = frame->fn_env;
Value *result;
if (e && (int)slot < e->obs_cap && e->obs[slot].used) {
result = make_str(observer_slot_report(&e->obs[slot]));
const ObserverSlot *os_l = env_obs_slot(e, (int)slot);
if (os_l && os_l->used) {
result = make_str(observer_slot_report(os_l));
} else {
result = make_str("equilibrium"); /* unobserved binding */
}
Expand All @@ -4399,8 +4398,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
DISPATCH();
}
Value *result;
if (oidx >= 0 && oidx < oe->obs_cap && oe->obs[oidx].used) {
result = make_str(observer_slot_report(&oe->obs[oidx]));
const ObserverSlot *os_n = env_obs_slot(oe, oidx);
if (os_n && os_n->used) {
result = make_str(observer_slot_report(os_n));
} else {
result = make_str("equilibrium"); /* unobserved binding */
}
Expand All @@ -4415,8 +4415,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
uint16_t slot = read_u16(ip); ip += 2;
Env *e = frame->fn_env;
Value *result;
if (e && (int)slot < e->obs_cap)
result = make_str(observer_slot_report_value(&e->obs[slot]));
const ObserverSlot *vs_l = env_obs_slot(e, (int)slot);
if (vs_l)
result = make_str(observer_slot_report_value(vs_l));
else
result = make_str("equilibrium");
vm_push(result);
Expand All @@ -4438,8 +4439,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
DISPATCH();
}
Value *result;
if (oidx >= 0 && oidx < oe->obs_cap)
result = make_str(observer_slot_report_value(&oe->obs[oidx]));
const ObserverSlot *vs_n = env_obs_slot(oe, oidx);
if (vs_n)
result = make_str(observer_slot_report_value(vs_n));
else
result = make_str("equilibrium");
vm_push(result);
Expand All @@ -4453,7 +4455,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
* (classifies "equilibrium", mirroring report_value's fallback). */
uint16_t slot = read_u16(ip); ip += 2;
Env *e = frame->fn_env;
const ObserverSlot *s = (e && (int)slot < e->obs_cap) ? &e->obs[slot] : NULL;
const ObserverSlot *s = env_obs_slot(e, (int)slot);
vm_push(observer_slot_trajectory(s));
DISPATCH();
}
Expand All @@ -4472,7 +4474,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
vm_push_slot(slot_null());
DISPATCH();
}
const ObserverSlot *s = (oidx >= 0 && oidx < oe->obs_cap) ? &oe->obs[oidx] : NULL;
const ObserverSlot *s = env_obs_slot(oe, oidx);
vm_push(observer_slot_trajectory(s));
DISPATCH();
}
Expand All @@ -4482,8 +4484,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
* from the local's slot trajectory; no-observation tuple if unpopulated. */
uint16_t slot = read_u16(ip); ip += 2;
Env *e = frame->fn_env;
if (e && (int)slot < e->obs_cap && e->obs[slot].used) {
const ObserverSlot *s = &e->obs[slot];
const ObserverSlot *s = env_obs_slot(e, (int)slot);
if (s && s->used) {
Value *list = make_list(4);
list_append_owned(list, make_str(observer_slot_report(s)));
list_append_owned(list, make_num(s->entropy));
Expand All @@ -4505,8 +4507,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
if (h == 0) { h = env_hash_name(name); if (chunk->const_hashes) chunk->const_hashes[name_idx] = h; }
int oidx = -1, odepth = 0;
Env *oe = env_resolve_chain(frame->env, name, h, &oidx, &odepth);
if (oe && oidx >= 0 && oidx < oe->obs_cap && oe->obs[oidx].used) {
const ObserverSlot *s = &oe->obs[oidx];
const ObserverSlot *s = env_obs_slot(oe, oidx);
if (s && s->used) {
Value *list = make_list(4);
list_append_owned(list, make_str(observer_slot_report(s)));
list_append_owned(list, make_num(s->entropy));
Expand Down Expand Up @@ -4546,8 +4548,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
* fresh; its history entry was created by the SET's
* trace_assign). */
if (__builtin_expect(g_trace_obs_hist, 0)) {
const ObserverSlot *os = &oe->obs[oidx];
trace_record_obs(name, os->entropy, os->dH, os->last_entropy);
const ObserverSlot *os = env_obs_slot(oe, oidx);
if (os) trace_record_obs(name, os->entropy, os->dH, os->last_entropy);
}
}
}
Expand Down Expand Up @@ -4623,8 +4625,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
* yet, so the answer matches the value path. */
int oidx = -1, odepth = 0;
Env *oe = env_resolve_chain(frame->env, name, h, &oidx, &odepth);
if (oe && oidx >= 0 && oidx < oe->obs_cap && oe->obs[oidx].used) {
const ObserverSlot *s = &oe->obs[oidx];
const ObserverSlot *s = env_obs_slot(oe, oidx);
if (s && s->used) {
if (kind == 3) result = make_num(s->entropy);
else if (kind == 4) result = make_num(s->dH);
else result = make_num(observer_settledness(s->dH));
Expand Down Expand Up @@ -4835,9 +4837,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
* the predicate is false. */
uint16_t kind = read_u16(ip); ip += 2;
int result = 0;
if (g_last_obs_slot_idx >= 0 && g_last_obs_slot_env &&
g_last_obs_slot_idx < g_last_obs_slot_env->obs_cap) {
const ObserverSlot *s = &g_last_obs_slot_env->obs[g_last_obs_slot_idx];
const ObserverSlot *s = env_obs_slot(g_last_obs_slot_env, g_last_obs_slot_idx);
if (s) {
switch (kind) {
case 0: result = observer_slot_converged(s); break;
case 1: result = observer_slot_stable(s); break;
Expand All @@ -4859,8 +4860,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
uint16_t slot = read_u16(ip); ip += 2;
Env *e = frame->fn_env;
int result = 0;
if (e && (int)slot < e->obs_cap && e->obs[slot].used)
result = vm_slot_predicate(&e->obs[slot], kind);
const ObserverSlot *ps_l = env_obs_slot(e, (int)slot);
if (ps_l && ps_l->used)
result = vm_slot_predicate(ps_l, kind);
vm_push(make_num(result ? 1.0 : 0.0));
DISPATCH();
}
Expand All @@ -4881,8 +4883,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
DISPATCH();
}
int result = 0;
if (oidx >= 0 && oidx < oe->obs_cap && oe->obs[oidx].used)
result = vm_slot_predicate(&oe->obs[oidx], kind);
const ObserverSlot *ps_n = env_obs_slot(oe, oidx);
if (ps_n && ps_n->used)
result = vm_slot_predicate(ps_n, kind);
vm_push(make_num(result ? 1.0 : 0.0));
DISPATCH();
}
Expand Down
Loading
Loading