From 1159eed4de1c30e508d1b546c882dcea461829b3 Mon Sep 17 00:00:00 2001 From: Bernie Innocenti Date: Mon, 28 Sep 2026 07:32:57 +0800 Subject: [PATCH] sanitize: clear the freed-block poison when a block is reallocated A block that FreeVec poisoned keeps that marking when the heap hands the same addresses back out, so the allocating handler's own header write or zeroing, and then every write the guest makes, are all reported as use-after-free. Writing over the block does not clear it either: check_write_byte heals Uninit bytes but records and keeps Unaddressable ones. Clear the shadow where the block is handed out, before the handler writes anything into it. AddressSpace::clear_fresh_block does it, so the allocators that only hold a &mut dyn AddressSpace can call it too. BenchWork's "backdrop lha-pack" under --sanitize went from 6 minutes and 691416 violations to 10 seconds and none. Fixes #95 Co-Authored-By: Claude Opus 5 (1M context) --- crates/volamos-core/src/dosfile.rs | 1 + crates/volamos-core/src/execlist.rs | 2 + crates/volamos-core/src/execmem.rs | 82 +++++++++++++++++++++++++++-- crates/volamos-core/src/memory.rs | 21 ++++++++ crates/volamos-core/src/utility.rs | 1 + 5 files changed, 102 insertions(+), 5 deletions(-) diff --git a/crates/volamos-core/src/dosfile.rs b/crates/volamos-core/src/dosfile.rs index 39d0640..cbacf3f 100644 --- a/crates/volamos-core/src/dosfile.rs +++ b/crates/volamos-core/src/dosfile.rs @@ -1369,6 +1369,7 @@ fn alloc_dos_object_handler(ctx: &mut HandlerContext<'_, C>) -> Result<( "AllocDosObject(type={object_type}): guest heap allocation failed: {e}" ), })?; + ctx.mem.clear_fresh_block(addr, struct_size); for i in 0..struct_size { ctx.mem.write_u8(addr.wrapping_add(i), 0); } diff --git a/crates/volamos-core/src/execlist.rs b/crates/volamos-core/src/execlist.rs index 3ec0d60..98ec33b 100644 --- a/crates/volamos-core/src/execlist.rs +++ b/crates/volamos-core/src/execlist.rs @@ -505,6 +505,7 @@ fn create_msg_port_handler(ctx: &mut HandlerContext<'_, C>) -> Result<() return Ok(()); } }; + ctx.mem.clear_fresh_block(port, MSGPORT_SIZE); init_msg_port_fields(ctx.mem, port, ctx.current_task); // After the field initialisation, never before: those writes heal // shadow bytes, so poisoning first would flag this handler's own @@ -591,6 +592,7 @@ fn create_io_request( size: u32, ) -> Option { let addr = heap.alloc(size).ok()?; + mem.clear_fresh_block(addr, size); for i in 0..size { mem.write_u8(addr.wrapping_add(i), 0); } diff --git a/crates/volamos-core/src/execmem.rs b/crates/volamos-core/src/execmem.rs index 0030d58..1fe9772 100644 --- a/crates/volamos-core/src/execmem.rs +++ b/crates/volamos-core/src/execmem.rs @@ -309,6 +309,7 @@ fn alloc_mem_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Disp // knows the unrounded number. match ctx.heap.alloc_with_requested(rounded, byte_size) { Ok(addr) => { + ctx.mem.clear_fresh_block(addr, rounded); let cleared = requirements & MEMF_CLEAR != 0; if cleared { for i in 0..rounded { @@ -421,6 +422,7 @@ fn alloc_vec_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Disp let total = user_rounded + ALLOCVEC_HEADER_SIZE; match ctx.heap.alloc(total) { Ok(block) => { + ctx.mem.clear_fresh_block(block, total); // Header: total block size (u32) followed by 4 bytes of // reserved padding (see the module docs). ctx.mem.write_u32(block, total); @@ -445,12 +447,22 @@ fn alloc_vec_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Disp // included -- the header is ours, not the guest's, but it // sits inside the same heap block, so the redzones bracket // both). Done after the header write and the MEMF_CLEAR - // loop above, per `poison_allocation_edges`' doc. The data - // bytes are left un-marked rather than marked uninit: the - // block address the heap knows is `block`, not `user_ptr`, - // so an uninit range starting at `block` would wrongly - // cover the header this handler just wrote. + // loop above, per `poison_allocation_edges`' doc. `false` + // because the data range cannot be described from `block`: + // that is what the heap knows, so an uninit range starting + // there would cover the header this handler just wrote. poison_allocation_edges(ctx, block, false); + // Which leaves the guest's own range marked valid by the + // call above -- right for the header, but it loses the + // uninitialized state of the part the guest owns. Mark that + // from `user_ptr`, the one address that excludes the header. + if let Some(shadow) = ctx.mem.shadow_mut() { + if cleared { + shadow.mark_valid(user_ptr, user_rounded); + } else { + shadow.mark_uninit(user_ptr, user_rounded); + } + } ctx.cpu.set_data_register(DataRegister(0), user_ptr); } Err(_) => { @@ -532,6 +544,7 @@ fn create_pool_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Di match ctx.heap.alloc(POOL_HEADER_SIZE) { Ok(pool) => { + ctx.mem.clear_fresh_block(pool, POOL_HEADER_SIZE); ctx.mem.write_u32(pool, requirements); ctx.cpu.set_data_register(DataRegister(0), pool); } @@ -590,6 +603,7 @@ fn alloc_pooled_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), D let rounded = round_up_8(byte_size); match ctx.heap.alloc_with_requested(rounded, byte_size) { Ok(addr) => { + ctx.mem.clear_fresh_block(addr, rounded); let cleared = requirements & MEMF_CLEAR != 0; if cleared { for i in 0..rounded { @@ -969,6 +983,64 @@ mod tests { ); } + #[test] + fn a_recycled_alloc_vec_block_is_no_longer_a_freed_block() { + // Issue #95: FreeVec poisons the block, and the heap hands the + // same addresses back out once the free quarantine releases + // them. If the allocation path leaves that poison in place, + // both the handler's own AllocVec header write and every write + // the guest makes into its brand-new buffer are reported as + // use-after-free -- and at one violation per byte the run slows + // to a crawl. + let mut words = Vec::new(); + words.push(move_imm_to_d(0)); // D0 = byteSize + words.push(0); + words.push(64); + words.push(move_imm_to_d(1)); // D1 = 0, no MEMF_CLEAR + words.push(0); + words.push(0); + words.extend_from_slice(&jsr_disp16_a6(-684)); // AllocVec + words.push(0x2240); // movea.l d0,a1 (A1 = the block to free) + words.extend_from_slice(&jsr_disp16_a6(-690)); // FreeVec + words.push(move_imm_to_d(0)); // same size again + words.push(0); + words.push(64); + words.push(move_imm_to_d(1)); + words.push(0); + words.push(0); + words.extend_from_slice(&jsr_disp16_a6(-684)); // AllocVec + words.push(0x2040); // movea.l d0,a0 + words.push(0x10bc); // move.b #1,(a0) -- the guest's first write + words.push(0x0001); + words.push(RTS); + + let mut rt = program(&words); + rt.memory_mut().enable_sanitizer(); + rt.enable_heap_sanitizer(); + // No quarantine, so the second AllocVec is handed the block the + // FreeVec just poisoned -- the whole point of the test. With the + // default budget a block this small is held back instead, and + // the run never recycles anything. + rt.heap_mut().set_quarantine_budget(0); + let mut out = Vec::new(); + let code = rt.run(&mut out, None).expect("run should succeed"); + let addr = code as u32; + + assert_ne!(addr, 0, "both allocations must succeed for this to test anything"); + let shadow = rt.memory().shadow().expect("sanitizer was enabled above"); + assert_eq!( + shadow.violation_count(), + 0, + "writing into a freshly allocated block must not be reported, \ + whatever that memory was used for before" + ); + assert_ne!( + shadow.state(addr), + crate::sanitize::ShadowState::Unaddressable, + "the recycled block must not still be marked as freed" + ); + } + #[test] fn alloc_mem_with_memf_clear_zeroes_and_alloc_free_round_trips() { let mut words = Vec::new(); diff --git a/crates/volamos-core/src/memory.rs b/crates/volamos-core/src/memory.rs index 392aa79..2817e7d 100644 --- a/crates/volamos-core/src/memory.rs +++ b/crates/volamos-core/src/memory.rs @@ -50,6 +50,21 @@ pub trait AddressSpace { /// Writes a single byte. Silently ignored if `addr` is out of range. fn write_u8(&mut self, addr: u32, value: u8); + /// Clears any sanitizer poison over `[addr, addr + len)`, a block the + /// guest heap has just handed out. + /// + /// Call this as soon as an allocation succeeds, *before* the handler + /// writes a header or zeroes the block: the heap recycles addresses, + /// so those bytes may still carry the `PoisonReason::Freed` marking + /// of the previous allocation, and writing over them does not clear + /// it -- the shadow map heals `Uninit` bytes but keeps + /// `Unaddressable` ones. Without this, the handler's own + /// initialization is reported as a use-after-free, and so is every + /// write the guest then makes. + /// + /// Default: nothing to do, for address spaces with no shadow map. + fn clear_fresh_block(&mut self, _addr: u32, _len: u32) {} + /// Reads a big-endian 16-bit value. Any byte that falls out of range /// reads as `0`. fn read_u16(&self, addr: u32) -> u16 { @@ -224,6 +239,12 @@ impl AddressSpace for FlatMemory { self.bytes.get(addr as usize).copied().unwrap_or(0) } + fn clear_fresh_block(&mut self, addr: u32, len: u32) { + if let Some(shadow) = &mut self.shadow { + shadow.mark_valid(addr, len); + } + } + fn write_u8(&mut self, addr: u32, value: u8) { if let Some(shadow) = &mut self.shadow { shadow.check_write(addr, 1); diff --git a/crates/volamos-core/src/utility.rs b/crates/volamos-core/src/utility.rs index 7698dfd..fe179a6 100644 --- a/crates/volamos-core/src/utility.rs +++ b/crates/volamos-core/src/utility.rs @@ -276,6 +276,7 @@ fn allocate_tag_items_handler( let byte_size = num_tags.saturating_mul(TAG_ITEM_SIZE); match ctx.heap.alloc(byte_size) { Ok(addr) => { + ctx.mem.clear_fresh_block(addr, byte_size); for i in 0..byte_size { ctx.mem.write_u8(addr.wrapping_add(i), 0); }