Repository navigation
Null-fill every unsent parameter at every call entry (heap use-after-free, #1661) - #1664
Merged
Merged
Conversation
Root cause. On the fresh-env call path (CASE(CALL), jit_helper_call),
a 2+-param callee called under-arity bound only min(argc, param_count)
params; the null fill of [argc, param_count) ran only `if (can_default)`.
For a defaults-free callee the env kept count < param_count, and when
local_count == param_count no env_reserve_slots ran either. A printf at
the bind site on the issue's reproducer showed, on every call:
call bind: argc=1 param_count=2 local_count=2 count=1 cap=16 v1=0
call bind: ... count=1 cap=16 v1=fffb55ac1ff6ea80 (stale heap ptr)
The interpreter's GET_LOCAL bounds-checks and reads null, but the JIT's
GET_LOCAL reads values[slot] unchecked, trusting count >= local_count.
env_new recycles freelist envs without clearing values[] past count, so
the JIT read a previous occupant's slot: under ASan a heap-use-after-free
(read in slot_as_double from OP_EQ; the list freed by env_decref in
CASE(RETURN) -> slot_decref -> cycle collector). EIGS_JIT_OFF=1 is clean.
Without the JIT the unbound name still leaked: a closure over an unsent
param resolved it in an outer scope instead of reading null.
Fix: the fill now runs for every unsent param, defaults or not, at
CASE(CALL), jit_helper_call, CASE(DISPATCH), the C fallback of the
`dispatch` builtin, and task_start. spawn (thread entry) and
call_eigs_fn (sort_by and the other callbacks) already null-filled.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Six programs, each at the default, interpreter and forced-JIT tiers: the issue's reproducer, depth 1000 and 3000 with and without defaults, the try/catch variant, a closure-over-unsent-param check across direct call / spawn / task_spawn / dispatch / sort_by plus a JIT stale-slot witness, a JIT-to-JIT (jit_helper_call) witness, and the eval-forced `dispatch` C fallback. Unfixed: 4/18 pass under release and ASan (only the interpreter rows of the JIT-only cases); fixed: 18/18 under both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Open
2 tasks
Collaborator
Author
|
The Opus 4.8 blind critic PASSED this PR. It built fixed and base binaries itself and checked every entry across the default, interpreter and forced-JIT tiers:
The residual hazard class is filed as #1666: a mechanical guard, not a JIT runtime check, which it measured as too costly on the hottest op. CI: 26 green, 6 skipped. |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the heap use-after-free reported in #1661. It is reachable from ordinary code and was also present in v0.44.0.
Root cause
CASE(CALL)andjit_helper_call, a callee with 2+ params called with fewer args bound onlymin(argc, param_count)of its params. The null fill for the rest ran onlyif (can_default).local_count == param_count, the env did not reserve the unsent slots. A recycled freelist env therefore kept a stale pointer in them.values[slot]unchecked, so it dereferenced the stale pointer, a value the cycle collector had already freed.EIGS_JIT_OFF=1there was no crash, but the bug was still visible: a closure over the unsent param resolved an outer variable of the same name instead of null.Fix
Every unsent param is null-filled by name, whether or not the callee has defaults. Call-path audit:
CASE(CALL)fresh envjit_helper_callCASE(DISPATCH)dispatchC fallbacktask_startTests
Suite section [0fd] holds 18 rows: the reproducer, depth 1000 and 3000, the try/catch double-free variant, JIT-to-JIT, dispatch and task_spawn, each across the default, interpreter and forced-JIT tiers.
make test-changed: 1959/1959.make precheck: 18 passed, 1 skipped.libSDL2_mixer; they fail identically on unfixed main.This goes out as patch release v0.45.1.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF