From 5e63918a08f6e932f3fea3c8ca3f6a3ddb5882e7 Mon Sep 17 00:00:00 2001 From: jdalton Date: Sun, 16 Aug 2026 15:21:53 -0400 Subject: [PATCH] fix(gc): reject fabricated Map/Set headers in plausible_gc_header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit classify_arena validates that an address and addr-8 are in heap space, then pattern-matches a GcHeader at addr-8 — but never checks addr-8 is an object START. An 8-aligned interior arena pointer (a word in a live object's payload) supplies a fabricated GcHeader whose: - obj_type = low byte = 0x08 = GC_TYPE_MAP (the only valid type that is a multiple of 8) - size = top 32 bits ≈ 1024 (always passes the old [8, 2^34] range check) - gc_flags = second byte, needs GC_FLAG_ARENA (0x02) — ~coin flip move_young then copy_nonoverlapping's ~1024 bytes from that interior address, and the remembered-set rebuild reads byte 8 of the copied data as MapHeader.entries — a NaN-boxed JSValue carrying 0x7FFD in the top bits, which crashes on dereference. Three-layer fix: 1. Primary: tighten plausible_gc_header so fixed-layout types (Map, Set) must have size == their known constant total (GC_HEADER_SIZE + 16 = 24). A fabricated header has size ≈ 1024, which is rejected. The invariant holds because: - The nursery bump-allocator always sets size = total - The old-gen allocator uses exact-match free-list reuse and sets size = total - The nursery free-list reuse path is inert (hot_arena_free_list is never populated); it is also fixed to set size = total and use exact-match for safety 2. Allocator hardening: the nursery free-list reuse path now sets size = total (was: retained the stale slot size) and uses exact-match only (was: best-fit), mirroring arena_alloc_gc_old's old_free_take_exact. This prevents a future activation of the free list from breaking the fixed-layout invariant. 3. Defensive tripwire: the Map and Set descriptor arms now reject an entries/elements pointer whose top bits (>>47) are non-zero — an impossible x86-64 user-space address, and the exact signature of a NaN-boxed JSValue misread as a pointer. This is a backstop; the primary fix stops fabrication at classify_arena. Regression test: four deterministic tests in gc/tests/copying/fabricated_map_rejection.rs that drive a fabricated Map header (size=1024) through plausible_gc_header and classify_arena and assert rejection, plus positive tests for genuine headers and variable-size types. Verification on the sfw-registry --help workload (firewall repo, iovalkey forced, loop polls compiled and run): - Plain arm: 120/120 PASS (0% failure; 95% upper bound ~2.5%) - Seeded arm (rate=0.05, 120 seeds): 110/120 PASS, 10 FAIL — all TypeError ("Cannot convert undefined or null to object"), 0 SIGSEGV. The TypeError failures are a SEPARATE rooting bug, not the fabricated-Map bug. The fabricated-Map SIGSEGV is eliminated. - Full cargo test -p perry-runtime --lib: 2535/2535 PASS The #7161 stopgap (moving loop polls default-OFF) is already reverted (default ON since #7682). This fix eliminates the SIGSEGV class of failures that justified the stopgap. However, a second bug (rooting TypeError) remains under seeded schedules, so the default-ON state is not yet fully safe under adversarial GC timing. --- crates/perry-runtime/src/arena/allocators.rs | 26 +- .../src/gc/copying_pointer_set.rs | 51 +++- .../perry-runtime/src/gc/layout_slot_visit.rs | 17 ++ crates/perry-runtime/src/gc/tests/copying.rs | 1 + .../tests/copying/fabricated_map_rejection.rs | 224 ++++++++++++++++++ crates/perry-runtime/src/set.rs | 12 + 6 files changed, 317 insertions(+), 14 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/copying/fabricated_map_rejection.rs diff --git a/crates/perry-runtime/src/arena/allocators.rs b/crates/perry-runtime/src/arena/allocators.rs index 5276d02af8..49010d2e0d 100644 --- a/crates/perry-runtime/src/arena/allocators.rs +++ b/crates/perry-runtime/src/arena/allocators.rs @@ -434,19 +434,23 @@ pub fn arena_alloc_gc(size: usize, align: usize, obj_type: u8) -> *mut u8 { let reused = if crate::gc::hot_arena_free_list_nonempty().get() { { let mut fl = crate::gc::hot_arena_free_list().borrow_mut(); - // Find a slot that fits (exact or slightly larger) - let mut best_idx = None; - let mut best_waste = usize::MAX; + // Exact-fit only: a best-fit reuse into a larger slot would + // leave `GcHeader.size` equal to the SLOT size (set below), + // but the arena block walker steps by `size`, so a mismatch + // would either skip the padding and misalign the walk, or — + // if we kept the stale larger size — break the fixed-layout + // invariant `plausible_gc_header` relies on. Exact match + // makes both correct: `size == total`, and the walker steps + // to the next real object. Mirrors `arena_alloc_gc_old`'s + // `old_free_take_exact`. + let mut found_idx = None; for (idx, &(_, slot_size)) in fl.iter().enumerate() { - if slot_size >= size && slot_size - size < best_waste { - best_waste = slot_size - size; - best_idx = Some(idx); - if best_waste == 0 { - break; // Perfect fit - } + if slot_size == total { + found_idx = Some(idx); + break; } } - if let Some(idx) = best_idx { + if let Some(idx) = found_idx { let (ptr, _slot_size) = fl.swap_remove(idx); if fl.is_empty() { crate::gc::hot_arena_free_list_nonempty().set(false); @@ -469,7 +473,7 @@ pub fn arena_alloc_gc(size: usize, align: usize, obj_type: u8) -> *mut u8 { (*header).gc_flags = GC_FLAG_ARENA | crate::gc::gc_birth_extra_flags(); crate::gc::gc_note_black_birth(header); (*header)._reserved = 0; - // size field already set from original allocation + (*header).size = total as u32; } return user_ptr; } diff --git a/crates/perry-runtime/src/gc/copying_pointer_set.rs b/crates/perry-runtime/src/gc/copying_pointer_set.rs index c690fa1d34..9c7492feb6 100644 --- a/crates/perry-runtime/src/gc/copying_pointer_set.rs +++ b/crates/perry-runtime/src/gc/copying_pointer_set.rs @@ -236,13 +236,58 @@ pub(super) unsafe fn plausible_gc_header(header: *mut GcHeader, arena: bool) -> return false; } let obj_type = (*header).obj_type; - if gc_type_info(obj_type).is_none() { - return false; - } + let info = match gc_type_info(obj_type) { + Some(info) => info, + None => return false, + }; let size = (*header).size as usize; if size < GC_HEADER_SIZE || size as u64 > (1u64 << 34) { return false; } + // Fixed-layout types have a known, constant total allocation size + // (header + payload). A fabricated header — produced when + // `classify_arena` reads an interior arena pointer's preceding bytes + // as a `GcHeader` — supplies a `size` of ~1024 (the top 32 bits of an + // arena address near 0x400_0000_0000), which can never equal the real + // total. This rejects the fabricated-Map corruption path at its + // source: the only descriptor arm that derives a slot base from a + // payload word is `GcRewriteDescriptorKind::Map` (`(*map).entries`), + // and GC_TYPE_MAP (8) is the only valid type ID that is a multiple of + // 8, so it is the only one an 8-aligned interior pointer can + // fabricate. + // + // The invariant "GcHeader.size == fixed_total" holds for every + // arena-allocated Map/Set because: + // * The nursery bump-allocator sets `size = total` (allocators.rs). + // * The old-gen allocator uses exact-match free-list reuse and + // also sets `size = total`. + // * The nursery free-list reuse path is inert — + // `hot_arena_free_list` is never populated — so the branch that + // would retain a stale larger `size` is dead code. That path is + // also fixed to set `size = total` for safety. + if let Some(fixed_total) = fixed_layout_total_size(info) { + if size != fixed_total { + return false; + } + } let is_arena = (*header).gc_flags & GC_FLAG_ARENA != 0; is_arena == arena } + +/// Total allocation size (GcHeader + payload) for types whose payload +/// layout is fixed regardless of content. `None` for variable-size types +/// (arrays, objects, strings, closures, …) whose `size` field reflects +/// runtime content and cannot be checked against a constant. +/// +/// Used by `plausible_gc_header` to reject fabricated headers: an +/// interior arena pointer read as a `GcHeader` yields a `size` of +/// ~1024 (the top 32 bits of the address), which never matches the +/// fixed total. +fn fixed_layout_total_size(info: &GcTypeInfo) -> Option { + // MapHeader { size: u32, capacity: u32, entries: *mut f64 } = 16 B. + // SetHeader { size: u32, capacity: u32, elements: *mut f64 } = 16 B. + match info.type_id { + crate::gc::GC_TYPE_MAP | crate::gc::GC_TYPE_SET => Some(GC_HEADER_SIZE + 16), + _ => None, + } +} diff --git a/crates/perry-runtime/src/gc/layout_slot_visit.rs b/crates/perry-runtime/src/gc/layout_slot_visit.rs index 673c04e571..27bf896c19 100644 --- a/crates/perry-runtime/src/gc/layout_slot_visit.rs +++ b/crates/perry-runtime/src/gc/layout_slot_visit.rs @@ -201,6 +201,23 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( if size > capacity || size > 16_000_000 || (*map).entries.is_null() { return; } + // Defensive tripwire (# fabricated-Map): if a fabricated Map + // header ever slips past `plausible_gc_header`, the `entries` + // field would be a NaN-boxed JSValue (top bits 0x7FFD…) read + // from the original object's payload — an impossible pointer + // on x86-64 where user-space addresses stay below bit 47. + // Reject it rather than dereference a derived slot range from + // a garbage base. This is a backstop; the primary fix is the + // fixed-layout size check in `plausible_gc_header`. + if ((*map).entries as usize) >> 47 != 0 { + if crate::gc::gc_diag_enabled() { + eprintln!( + "[gc-tripwire] Map entries has implausible top bits: {:#x} (fabricated Map?)", + (*map).entries as usize + ); + } + return; + } visit(GcMutableSlotDescriptor::Range { range: HeapSlotRange::new((*map).entries as *mut u64, size as usize * 2), layout_kind: None, diff --git a/crates/perry-runtime/src/gc/tests/copying.rs b/crates/perry-runtime/src/gc/tests/copying.rs index dc057f5cc4..424f5973e1 100644 --- a/crates/perry-runtime/src/gc/tests/copying.rs +++ b/crates/perry-runtime/src/gc/tests/copying.rs @@ -1,6 +1,7 @@ mod adaptive_tenuring; mod all_pointer_elements_7469; mod deferred_finalize_7635; +mod fabricated_map_rejection; mod latch; mod pointer_publish_7154; mod promise_side_tables; diff --git a/crates/perry-runtime/src/gc/tests/copying/fabricated_map_rejection.rs b/crates/perry-runtime/src/gc/tests/copying/fabricated_map_rejection.rs new file mode 100644 index 0000000000..dbe9cf95d9 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/copying/fabricated_map_rejection.rs @@ -0,0 +1,224 @@ +//! Regression test for the fabricated-Map classification bug. +//! +//! `classify_arena` validates that an address and `addr - 8` are both in +//! heap space, then pattern-matches a `GcHeader` at `addr - 8`. It never +//! checked that `addr - 8` is an object START, so an interior arena pointer +//! — a word in a live object's payload that happens to be an arena address +//! — would fabricate a fake object from the bytes preceding it. +//! +//! For an 8-aligned arena pointer near `0x400_0000_0000`, the fabricated +//! `GcHeader` supplies: +//! * `obj_type` = low byte = 0x08 = `GC_TYPE_MAP` (the only valid type +//! that is a multiple of 8) +//! * `size` = top 32 bits ≈ 1024 (always passes the `[8, 2^34]` range +//! check in the old `plausible_gc_header`) +//! * `gc_flags` = second byte, needs `GC_FLAG_ARENA` (0x02) — ~coin flip +//! +//! The fix: `plausible_gc_header` now requires `size == 24` (the known +//! fixed total for Map: 8-byte header + 16-byte `MapHeader` payload) for +//! fixed-layout types. A fabricated header has `size ≈ 1024`, which is +//! rejected. + +use super::*; + +/// Size of `MapHeader` (and `SetHeader`): `{ size: u32, capacity: u32, +/// entries/elements: *mut f64 }` = 16 bytes. +const MAP_HEADER_PAYLOAD: usize = 16; +const MAP_FIXED_TOTAL: usize = GC_HEADER_SIZE + MAP_HEADER_PAYLOAD; + +/// Directly test that `plausible_gc_header` rejects a fabricated Map +/// header with the ~1024-byte size that an interior arena pointer +/// produces, while accepting a genuine Map header with size = 24. +#[test] +fn test_plausible_gc_header_rejects_fabricated_map_size() { + // A genuine Map header: obj_type = MAP, size = 24, GC_FLAG_ARENA set. + let mut genuine = GcHeader { + obj_type: GC_TYPE_MAP, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: MAP_FIXED_TOTAL as u32, + }; + assert!( + unsafe { plausible_gc_header(&mut genuine as *mut GcHeader, true) }, + "genuine Map header (size={MAP_FIXED_TOTAL}) must be plausible" + ); + + // A fabricated Map header: obj_type = MAP, size = 1024 (the top 32 + // bits of an arena address near 0x400_0000_0000), GC_FLAG_ARENA set. + // This is what classify_arena would read from 8 bytes preceding an + // interior arena pointer. + let mut fabricated = GcHeader { + obj_type: GC_TYPE_MAP, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: 1024, + }; + assert!( + !unsafe { plausible_gc_header(&mut fabricated as *mut GcHeader, true) }, + "fabricated Map header (size=1024) must be rejected" + ); + + // Edge: size = 24 + 8 = 32 (what free-list reuse into a larger slot + // would produce) must also be rejected — only the exact fixed total + // is accepted. + let mut wrong_size = GcHeader { + obj_type: GC_TYPE_MAP, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: 32, + }; + assert!( + !unsafe { plausible_gc_header(&mut wrong_size as *mut GcHeader, true) }, + "Map header with non-fixed size (32) must be rejected" + ); +} + +/// Same check for Set (GC_TYPE_SET = 12). Set cannot be fabricated from +/// an 8-aligned pointer (12 is not a multiple of 8), but the size check +/// applies to it nonetheless — it is free for genuine objects and +/// guards against any future fabrication path. +#[test] +fn test_plausible_gc_header_rejects_fabricated_set_size() { + let mut genuine = GcHeader { + obj_type: GC_TYPE_SET, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: MAP_FIXED_TOTAL as u32, + }; + assert!( + unsafe { plausible_gc_header(&mut genuine as *mut GcHeader, true) }, + "genuine Set header (size={MAP_FIXED_TOTAL}) must be plausible" + ); + + let mut fabricated = GcHeader { + obj_type: GC_TYPE_SET, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: 1040, + }; + assert!( + !unsafe { plausible_gc_header(&mut fabricated as *mut GcHeader, true) }, + "fabricated Set header (size=1040) must be rejected" + ); +} + +/// Variable-size types (arrays, objects, strings) must NOT be rejected +/// by the fixed-layout check — their `size` reflects runtime content. +#[test] +fn test_plausible_gc_header_still_accepts_variable_size_types() { + let mut array_header = GcHeader { + obj_type: GC_TYPE_ARRAY, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: 128, // arbitrary, > GC_HEADER_SIZE + }; + assert!( + unsafe { plausible_gc_header(&mut array_header as *mut GcHeader, true) }, + "variable-size type (array) with arbitrary size must be plausible" + ); + + let mut string_header = GcHeader { + obj_type: GC_TYPE_STRING, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: 64, + }; + assert!( + unsafe { plausible_gc_header(&mut string_header as *mut GcHeader, true) }, + "variable-size type (string) with arbitrary size must be plausible" + ); +} + +/// End-to-end: allocate a real Map in the nursery, verify its header +/// has the fixed total size, and verify classify_arena accepts it. +/// Then fabricate an interior-pointer scenario and verify rejection. +#[test] +fn test_classify_arena_rejects_interior_pointer_as_map() { + let _guard = CopyingNurseryTestGuard::new(1); + + // Allocate a genuine Map. Its GcHeader.size must be 24. + let map_ptr = crate::map::js_map_alloc(4) as *mut u8; + assert!(!map_ptr.is_null(), "Map allocation must succeed"); + + let map_header = unsafe { (map_ptr as *mut u8).sub(GC_HEADER_SIZE) as *mut GcHeader }; + let map_total = unsafe { (*map_header).size as usize }; + assert_eq!( + map_total, MAP_FIXED_TOTAL, + "genuine nursery Map must have fixed total size {MAP_FIXED_TOTAL}, got {map_total}" + ); + + // The Map's user pointer must classify as a valid arena pointer. + let ptrs = CopyingPointerSet::new(); + let classified = ptrs.classify_arena(map_ptr as usize); + assert!( + classified.is_some(), + "genuine Map user pointer must classify in arena" + ); + + // Now fabricate the bug scenario: write a GcHeader-shaped word into + // an array's payload, then check that the address immediately after + // it (which would be "addr" in classify_arena, with the fabricated + // header at "addr - 8") is NOT classified as a Map. + // + // We allocate an array with enough elements to hold our fabricated + // header, write the header bytes into it, and try to classify the + // address right after the header. + let array_ptr = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_ARRAY) as *mut u64; + assert!(!array_ptr.is_null(), "array allocation must succeed"); + + // Write a fabricated Map GcHeader into the first 8 bytes of the + // array's payload: obj_type=MAP, gc_flags=ARENA, size=1024. + let fabricated_header = GcHeader { + obj_type: GC_TYPE_MAP, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: 1024, + }; + unsafe { + std::ptr::write(array_ptr as *mut GcHeader, fabricated_header); + } + + // The address "array_ptr + 8" would have the fabricated header at + // "array_ptr" (i.e., addr - 8 = array_ptr). If the array is in a + // registered arena range, classify_arena would previously accept + // this as a Map. After the fix, it must reject it because + // size=1024 != 24. + let fabricated_user_addr = unsafe { (array_ptr as *mut u8).add(8) } as usize; + let result = ptrs.classify_arena(fabricated_user_addr); + assert!( + result.is_none(), + "interior pointer with fabricated Map header (size=1024) must NOT classify" + ); + + // Also verify a genuine-size fabricated header (size=24) at the same + // location WOULD have been accepted without the fix — but with the + // fix, the fixed-layout check passes, so classify_arena returns Some. + // This confirms the check is specific to the size, not a blanket + // rejection of the address. + let genuine_size_header = GcHeader { + obj_type: GC_TYPE_MAP, + gc_flags: GC_FLAG_ARENA, + _reserved: 0, + size: MAP_FIXED_TOTAL as u32, + }; + unsafe { + std::ptr::write(array_ptr as *mut GcHeader, genuine_size_header); + } + let result_genuine = ptrs.classify_arena(fabricated_user_addr); + // This SHOULD classify — the header now has the correct fixed size. + // (If it doesn't, the array payload may not be in a recognized heap + // space, which would also block the fabricated bug — so None is + // acceptable here. The key assertion is the previous one: size=1024 + // is rejected.) + if result_genuine.is_some() { + // It classified — verify it claims to be a Map. + let ptr = result_genuine.unwrap(); + unsafe { + assert_eq!( + (*ptr.header).obj_type, + GC_TYPE_MAP, + "classified fabricated-as-genuine should be Map" + ); + } + } +} diff --git a/crates/perry-runtime/src/set.rs b/crates/perry-runtime/src/set.rs index 9ecf8d68be..1a8b54a9db 100644 --- a/crates/perry-runtime/src/set.rs +++ b/crates/perry-runtime/src/set.rs @@ -555,6 +555,18 @@ pub(crate) unsafe fn gc_element_slot_range( if size > capacity || size > 16_000_000 || (*set).elements.is_null() { return None; } + // Defensive tripwire (cf. Map's entries check in layout_slot_visit): + // a NaN-boxed JSValue read as `elements` carries 0x7FFD in the top + // bits — an impossible x86-64 user-space pointer. + if ((*set).elements as usize) >> 47 != 0 { + if crate::gc::gc_diag_enabled() { + eprintln!( + "[gc-tripwire] Set elements has implausible top bits: {:#x} (fabricated Set?)", + (*set).elements as usize + ); + } + return None; + } Some(crate::gc::HeapSlotRange::new( (*set).elements as *mut u64, size,