Skip to content

Commit 66fc330

Browse files
author
Yogthos
committed
Pin the top_dyns offset with a test, drop the duplicate extern
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.
1 parent a4346e5 commit 66fc330

2 files changed

Lines changed: 120 additions & 9 deletions

File tree

src/plugin/mod_tests.rs

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3519,3 +3519,88 @@ fn plugin_worker_uaf_stress() {
35193519
}
35203520
println!("stress completed without crashing: {turns} turns");
35213521
}
3522+
3523+
/// dirge-eona: pins the `JanetVMPrefix` layout assumption that the fix rests
3524+
/// on. `top_dyns_ptr` reads offset 8 of a struct whose definition is private
3525+
/// to janet.c, and `janetrs` depends on `evil-janet = "1"`, so a `cargo
3526+
/// update` can move the field under us. A wrong offset would not fail the
3527+
/// build — it would hand `janet_gcroot` whatever now lives there, which is
3528+
/// far worse than the crash it replaced. The stress harness above cannot
3529+
/// catch that: it is `#[ignore]`d and needs the user's plugin dir.
3530+
///
3531+
/// This runs in the normal suite instead. It owns a bare VM on its own
3532+
/// thread (no default env, so nothing but this test writes a dyn), and leans
3533+
/// on the fact that no fiber is live here: `janet_setdyn` therefore writes
3534+
/// `top_dyns` (janet.c:4657), which is the same state the worker mirrors in.
3535+
///
3536+
/// The null-before assertion is what discriminates the field: every nearby
3537+
/// `JanetTable *` in `struct JanetVM` behaves differently across these
3538+
/// steps. `abstract_registry` is already non-null after `janet_init`
3539+
/// (janet.c:35577), and `core_env` stays null and never gains entries from
3540+
/// `janet_setdyn`.
3541+
#[cfg(feature = "plugin")]
3542+
#[test]
3543+
fn top_dyns_offset_is_stable() {
3544+
use janetrs::lowlevel::{janet_equals, janet_setdyn, janet_table, janet_wrap_table};
3545+
3546+
let _client = janetrs::client::JanetClient::init().expect("bare VM on a fresh test thread");
3547+
3548+
assert!(
3549+
worker::top_dyns_ptr().is_null(),
3550+
"top_dyns must start null (janet_init sets it NULL, janet.c:35592); \
3551+
a non-null read here means offset 8 is no longer top_dyns"
3552+
);
3553+
3554+
// A table, not a number: `janet_wrap_integer` is a macro under some
3555+
// nanbox configs and is not exported from libjanet, so it does not link.
3556+
// SAFETY: the VM is initialized on this thread, so `janet_table`
3557+
// allocates on its heap and `janet_wrap_table` only tags the pointer.
3558+
// Nothing collects during this test, so the value stays live unrooted.
3559+
let probe = unsafe { janet_wrap_table(janet_table(0)) };
3560+
// SAFETY: VM live on this thread and no fiber is running, so
3561+
// `janet_setdyn` lazily creates `top_dyns` and writes into it.
3562+
unsafe { janet_setdyn(c"dirge-eona-probe".as_ptr(), probe) };
3563+
3564+
let top = worker::top_dyns_ptr();
3565+
assert!(
3566+
!top.is_null(),
3567+
"the first janet_setdyn must create top_dyns"
3568+
);
3569+
// SAFETY: `top` is the table janet_setdyn just created; reading `count`
3570+
// is an in-bounds field read on a live JanetTable.
3571+
assert_eq!(
3572+
unsafe { (*top).count },
3573+
1,
3574+
"the table at offset 8 must be the one janet_setdyn wrote to"
3575+
);
3576+
3577+
// A second key lands in the SAME table, and the pointer does not move.
3578+
// SAFETY: as above — VM live on this thread, still no fiber.
3579+
unsafe { janet_setdyn(c"dirge-eona-probe-2".as_ptr(), probe) };
3580+
assert_eq!(
3581+
worker::top_dyns_ptr(),
3582+
top,
3583+
"top_dyns is created once, then reused (janet.c:4657)"
3584+
);
3585+
// SAFETY: as above.
3586+
assert_eq!(
3587+
unsafe { (*top).count },
3588+
2,
3589+
"both dyns must be in this table"
3590+
);
3591+
3592+
// And it is the table the public reader consults: with no fiber live,
3593+
// `janet_dyn` reads `top_dyns` directly (janet.c:4645).
3594+
// SAFETY: VM live on this thread, and both operands are live Janet
3595+
// values — `probe` and whatever the dyn lookup returns for its key.
3596+
assert_eq!(
3597+
unsafe {
3598+
janet_equals(
3599+
janetrs::lowlevel::janet_dyn(c"dirge-eona-probe".as_ptr()),
3600+
probe,
3601+
)
3602+
},
3603+
1,
3604+
"janet_dyn must read back what janet_setdyn wrote"
3605+
);
3606+
}

src/plugin/worker.rs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2192,20 +2192,38 @@ fn run_command_loop(
21922192

21932193
/// dirge-eona: `janet_vm` is `__thread` in evil-janet's janet.c (that is
21942194
/// how the plugin and notebook VMs coexist on separate threads), and its
2195-
/// layout is file-local to janet.c so the bindings cannot expose it. Only
2196-
/// the first two fields are needed; `top_dyns` sits at offset 8 in
2197-
/// `struct JanetVM`. Access goes through `janet_local_vm`, which returns
2198-
/// the calling thread's copy of the VM.
2195+
/// layout is file-local to janet.c — `janet.h` only forward-declares
2196+
/// `struct JanetVM`, so bindgen renders it opaque (`_unused: [u8; 0]`) and
2197+
/// the bindings cannot reach a field. Only the first two are needed;
2198+
/// `top_dyns` sits at offset 8, right after the `void *user` slot
2199+
/// (janet.c:172).
2200+
///
2201+
/// That offset is an assumption about a private C layout, and it has to hold
2202+
/// across any `evil-janet` 1.x (`janetrs` depends on `"1"`, so a `cargo
2203+
/// update` can move it). `top_dyns_offset_is_stable` in the plugin test
2204+
/// module pins it, so an upstream field insertion fails a test rather than
2205+
/// silently rooting whatever now sits at offset 8.
21992206
#[cfg(feature = "plugin")]
22002207
#[repr(C)]
22012208
struct JanetVMPrefix {
2209+
#[allow(dead_code)]
22022210
user: *mut core::ffi::c_void,
22032211
top_dyns: *mut janetrs::lowlevel::JanetTable,
22042212
}
22052213

2214+
/// The calling thread's `janet_vm.top_dyns` — null until the first
2215+
/// `janet_setdyn` on this thread creates it (janet.c:4657).
2216+
///
2217+
/// Reads through [`JanetVMPrefix`]; see there for the layout assumption.
22062218
#[cfg(feature = "plugin")]
2207-
unsafe extern "C" {
2208-
fn janet_local_vm() -> *mut JanetVMPrefix;
2219+
pub(super) fn top_dyns_ptr() -> *mut janetrs::lowlevel::JanetTable {
2220+
// SAFETY: `janet_local_vm` returns `&janet_vm`, this thread's own
2221+
// thread-local VM, so it is never null once Janet is initialized on the
2222+
// thread — true for every caller, each of which holds a live
2223+
// `JanetClient`. The cast reinterprets the bindings' opaque `JanetVM` as
2224+
// its own leading fields, so reading `top_dyns` touches only bytes
2225+
// inside the real struct.
2226+
unsafe { (*(janetrs::lowlevel::janet_local_vm() as *mut JanetVMPrefix)).top_dyns }
22092227
}
22102228

22112229
/// Point Janet's TOP-LEVEL dyn table at the same `:out`/`:err` buffers the
@@ -2247,9 +2265,17 @@ fn mirror_capture_buffers_into_top_dyns(client: &JanetClient) {
22472265
// collection frees it, and the next no-fiber `janet_dyn` (the stack
22482266
// trace printer's `:err-color` lookup) reads the freed table and
22492267
// segfaults the VM. Root the table itself, not just the buffers in it.
2250-
unsafe {
2251-
let top = (*janet_local_vm()).top_dyns;
2252-
if !top.is_null() {
2268+
// Janet roots `abstract_registry` the same way, for the same reason
2269+
// (janet.c:35577) — a VM-struct field is not a GC root on its own.
2270+
let top = top_dyns_ptr();
2271+
if !top.is_null() {
2272+
// SAFETY: `top` is the table `janet_setdyn` just created on this
2273+
// thread, so it is a live Janet object; `janet_wrap_table` only
2274+
// tags the pointer. Rooting is permanent by design and cannot leak
2275+
// past one entry per VM: both callers run once at thread init, and
2276+
// `top_dyns` is assigned nowhere but `janet_init`/`janet_deinit`
2277+
// (NULL) and this lazy create, so the root can never go stale.
2278+
unsafe {
22532279
janetrs::lowlevel::janet_gcroot(janetrs::lowlevel::janet_wrap_table(top));
22542280
}
22552281
}

0 commit comments

Comments
 (0)