fix: gcroot janet_vm.top_dyns stop plugin-worker SIGSEGV - #830
Merged
Conversation
`mirror_capture_buffers_into_top_dyns` created the top-level dyn table via `janet_setdyn` but only rooted the buffers inside it. Janet's collector never marks `top_dyns` (absent from the mark phase in 1.37.2 and 1.41.3 alike), so the first full GC freed the table and the next no-fiber `janet_dyn` (the stack-trace printer's `:err-color` lookup) read freed memory, faulting in `janet_dict_find`. - Root the table itself after the mirror; access the TLS `janet_vm` through `janet_local_vm()`. - Add `plugin_worker_uaf_stress` regression test (env-driven, `#[ignore]`-gated) replaying the hook sequence that preceded the crash; previously SIGSEGV'd in 3 turns, now passes 3/30. Refs: dirge-eona
- Collapse nested `if load_user_plugins { if let Ok(home) ... }` into a
let-chain condition (clippy::collapsible-if).
- Reflow three long lines the way rustfmt 1.98.0 (CI stable) formats
them; the local stable-aarch64 rustfmt is older and accepted the
single-line forms.
Refs: dirge-eona
The fix rests on top_dyns sitting at offset 8 of a struct that is private to janet.c, and janetrs depends on evil-janet "1", so a cargo update can move it. A wrong offset wouldn't fail the build — it would hand janet_gcroot whatever now lives there. The stress harness can't catch that: it's #[ignore]d and needs the user's plugin dir, so CI had no coverage of the fix at all. top_dyns_offset_is_stable runs in the normal suite on a bare VM of its own. It leans on the null-before-first-setdyn transition to discriminate the field (abstract_registry is already non-null after janet_init; core_env stays null and never gains dyn entries), then checks the entry count and that janet_dyn reads the same table. Verified it fails when a field is inserted ahead of top_dyns. Also use the janet_local_vm already in the bindings instead of re-declaring it. Two extern declarations of one symbol with different return types only compiled because they sit in different crates, so clashing_extern_declarations never fired. The pointer read moves into top_dyns_ptr, which is what the test drives.
yogthos
pushed a commit
that referenced
this pull request
Aug 31, 2026
#830 landed without an entry; it's a user-facing hard crash.
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.
mirror_capture_buffers_into_top_dynscreated the top-level dyn table viajanet_setdynbut only rooted the buffers inside it. Janet's collector never markstop_dyns(absent from the mark phase in 1.37.2 and 1.41.3 alike), so the first full GC freed the table and the next no-fiberjanet_dyn(the stack-trace printer's:err-colorlookup) read freed memory, faulting injanet_dict_find.janet_vmthroughjanet_local_vm().plugin_worker_uaf_stressregression test (env-driven,#[ignore]-gated) replaying the hook sequence that preceded the crash; previously SIGSEGV'd in 3 turns, now passes 3/30.Refs: dirge-eona