From ca3e6a7dd5b70dfa052855f44e6b6562eaeaebed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Wed, 14 Oct 2026 18:51:46 +0100 Subject: [PATCH 1/2] fix(pool): read a VS user block that would end a page where the allocator put it Under `_HEAP_VS_CONTEXT.Config.Flags.PageAlignLargeAllocs`, `nt!RtlpHpVsContextAllocate` hands out `chunk + 0x20` rather than `chunk + 0x10` when the former is page-aligned, and `RtlpHpVsContextFree` takes a page-aligned user block's chunk from `block - 0x20`: the kernel's way of keeping a `_POOL_HEADER` out of a page's last sixteen bytes, since `ExFreeHeapPool` tells a chunk from a page allocation by `P & 0xfff`. Read on x64 26100.33438 at `RtlpHpVsContextAllocate+0xf6..+0x12a`, `RtlpHpVsContextAllocateFinalize+0x3d` and `RtlpHpVsContextFree+0xf2`. The walk put every such header a unit early and decoded two stale free-tree pointers as a `_POOL_HEADER`: a tag that was the top half of a kernel address, a span a unit early and a unit long, and the real header at the page start unclaimed. Measured on `ctf-vm` 2026-10-14: 1,275 paged chunks under `0x84e7ffff`, every candidate at page offset 0xff0; the chunk at 0xffffe78459203fe0 holds `ffffe78459207e18 ffffe78459213bc8` at +0x10 and the `MmDT` header `!pool` names at 0xffffe78459204000. The flag is read from each heap's context at discovery (a new optional field, so every layout fingerprint moves), carried on the region and applied only where set; a PDB without the field says so. The dead `adjust_page_end_header`, which moved a candidate back to the previous page's end and could never fire on an aligned one, is removed. And `tree_nodes` polls the budget before its first read, which the extra read per heap exposed as a five-read stretch between polls. windbg-mcp FOLLOWUPS.md item 101. Co-Authored-By: Claude Mythos 5.1 Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT --- CHANGELOG.md | 28 ++++ src/pool/decode.rs | 26 +--- src/pool/layout.rs | 16 ++- src/pool/snapshot.rs | 313 +++++++++++++++++++++++++++++++++++++++---- 4 files changed, 338 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e072563..3906cfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -308,6 +308,34 @@ All notable changes to this project are documented here. The format follows ### Fixed +- **A VS user block that would begin in the last sixteen bytes of a page is read where the + allocator put it: at the next page start.** Under + `_HEAP_VS_CONTEXT.Config.Flags.PageAlignLargeAllocs`, `nt!RtlpHpVsContextAllocate` hands out + `chunk + 0x20` rather than `chunk + 0x10` when the former is page-aligned, and + `RtlpHpVsContextFree` takes a page-aligned user block's chunk from `block - 0x20` -- the + kernel's own way of keeping a `_POOL_HEADER` out of a page's last unit, since `ExFreeHeapPool` + tells a chunk from a page allocation by `P & 0xfff` (x64 26100.33438, read at + `RtlpHpVsContextAllocate+0xf6..+0x12a`, `RtlpHpVsContextAllocateFinalize+0x3d` and + `RtlpHpVsContextFree+0xf2`). The walk placed every such header one unit early, decoding two + stale free-tree pointers as a `_POOL_HEADER`: the tag came out as the top half of a kernel + address, the span a unit early and a unit long, and the real header at the page start went + unclaimed. Measured on a live 26100.33438 kernel 2026-10-14: 1,275 paged chunks under the "tag" + `0x84e7ffff`, every one with its candidate header at page offset 0xff0; + `0xffffe78459203fe0` is a 0x80-byte chunk holding `ffffe78459207e18 ffffe78459213bc8` at + `+0x10` and the `MmDT` header `!pool` names at `0xffffe78459204000`. The flag is read from each + heap's context at discovery -- `_HEAP_VS_CONTEXT.Config` is a new optional field, so **every + layout fingerprint moves** -- carried on the region, and applied only where set: a target + without it is walked as before, and a PDB without the field says so in a diagnostic. + `adjust_page_end_header`, which moved a candidate *back* to the end of the previous page and + could never fire on a sixteen-byte-aligned one, is gone. windbg-mcp `FOLLOWUPS.md` item 101. + `test_a_vs_user_block_that_would_end_a_page_begins_the_next_one` pins the placement, + `test_without_the_flag_a_page_end_user_block_is_read_at_its_chunk_header` the gate, + `test_a_vs_user_block_on_a_page_boundary_is_left_where_it_is` the boundary and + `test_discovery_reads_whether_a_vs_user_block_skips_a_page_end_from_the_context` the read. + Beside it, `tree_nodes` now polls the budget before its first read: the one extra read per + heap moved the sampled deadlines onto a five-read stretch between polls that + `test_the_walk_stops_reading_promptly_once_its_budget_is_gone` had never landed on. + - **A break left pending no longer empties the next reload.** `Execute` discards a pending `SetInterrupt` as a command begins; `Reload` does not. A host interrupt that landed between operations (`BreakRequest::NothingRunning`) made the next `reload_symbols("")` on a live kernel diff --git a/src/pool/decode.rs b/src/pool/decode.rs index 6497135..b9c3e22 100644 --- a/src/pool/decode.rs +++ b/src/pool/decode.rs @@ -4,6 +4,10 @@ use super::{PoolBackend, PoolState}; pub(crate) const PAGE_SIZE: u64 = 0x1000; pub(crate) const VS_SIGNATURE: u16 = 0x2bed; +/// The granularity of a VS chunk: `_HEAP_VS_CHUNK_HEADER.Sizes` counts in units of this many +/// bytes, and `nt!RtlpHpVsContextAllocate` moves a user block by one of them +/// (`snapshot::PoolRegion::vs_skips_page_end`). +pub(crate) const VS_UNIT: u64 = 16; pub(crate) const PAGE_SEGMENT_SIGNATURE: u64 = 0xa2e6_4ead_a2e6_4ead; pub(crate) const DESCRIPTOR_TREE_SIGNATURE: u32 = 0xccdd_ccdd; @@ -243,8 +247,8 @@ pub(crate) fn decode_vs_chunk( subsegment_end: u64, ) -> Result { let sizes = decode_vs_sizes(encoded, header_address, heap_key); - let size = usize::from(sizes.size).saturating_mul(16); - let previous_size = usize::from(sizes.previous_size).saturating_mul(16); + let size = usize::from(sizes.size).saturating_mul(VS_UNIT as usize); + let previous_size = usize::from(sizes.previous_size).saturating_mul(VS_UNIT as usize); let reject = |reason| { Err(VsRejection { reason, @@ -476,21 +480,6 @@ pub(crate) fn lfh_bitmap_state(bitmap: &[u8], slot: usize) -> Option }) } -/// A header that would straddle the page boundary is stored at the end of the -/// preceding page. Return the physical header location, checking all arithmetic. -pub(crate) fn adjust_page_end_header(candidate: u64, header_size: u64) -> Option { - if header_size == 0 || header_size > PAGE_SIZE { - return None; - } - let page_offset = candidate & (PAGE_SIZE - 1); - let last_header_start = PAGE_SIZE - header_size; - if page_offset > last_header_start { - candidate.checked_sub(page_offset - last_header_start) - } else { - Some(candidate) - } -} - pub(crate) fn decode_pool_header( bytes: &[u8], offset: usize, @@ -1342,9 +1331,6 @@ mod tests { // A page number that does fit in 32 bits is unaffected, which is why every user-mode // fixture kept passing. assert_eq!(big_page_hash(0x9000, 0x100), Some(0x57)); - assert_eq!(adjust_page_end_header(0x1ff0, 0x10), Some(0x1ff0)); - assert_eq!(adjust_page_end_header(0x1ff8, 0x10), Some(0x1ff0)); - assert_eq!(adjust_page_end_header(0x2000, 0x10), Some(0x2000)); let tree = 0xffff_8000_0001_0000; let root = 0xffff_8000_0002_0000; assert_eq!(decode_rb_root(root, tree, false), Some(root)); diff --git a/src/pool/layout.rs b/src/pool/layout.rs index eff0451..86556bd 100644 --- a/src/pool/layout.rs +++ b/src/pool/layout.rs @@ -324,6 +324,11 @@ const OPTIONAL_FIELDS: &[(&str, &str, &[&str])] = &[ // scaled by 64 bytes; the slot map holds AffinityMask + 1 entries. ("_HEAP_VS_CONTEXT", "SlotMapRef", &["SlotMapRef"]), ("_HEAP_VS_CONTEXT", "AffinityMask", &["AffinityMask"]), + // `_RTL_HP_VS_CONFIG`, whose `Flags.PageAlignLargeAllocs` bit decides where a VS user block + // begins when it would otherwise begin in the last sixteen bytes of a page — see + // `snapshot::PoolRegion::vs_skips_page_end`. Optional so a PDB without it still walks: read + // as unset then, with a diagnostic saying so. + ("_HEAP_VS_CONTEXT", "Config", &["Config"]), // How an affinity slot names the context it belongs to. Two canonical names rather than two // aliases of one, because they are not two spellings of the same fact: `VsContext` holds the // context's address and `VsContextOffset` holds `slot - context`. Aliased, the newer build @@ -1653,11 +1658,18 @@ mod tests { /// an inline layout built on it used to pick up a stray `_HEAP_VS_AFFINITY_SLOT.VsContext` /// through the *required* table and no longer does. That moves the synthetic digest while no /// real build's moves, which is an artefact of the double rather than a regression. + /// + /// **Re-recorded 2026-10-14**, when `_HEAP_VS_CONTEXT.Config` joined the optional fields + /// (windbg-mcp `FOLLOWUPS.md` item 101). Every build carries that field, so every digest + /// moved — the resolver found one more fact about each, which is the resolver's change and + /// not the target's — and the two below were taken from the code with the field added. The + /// pin is therefore against any *later* change moving an older build's schema, and the move + /// itself is recorded in the changelog, where a caller comparing runs across it will look. #[test] fn test_an_older_builds_schema_is_unchanged_by_the_self_relative_shape() { for (build, recorded) in [ - (&INLINE_BUILD, "fnv1a64:235676a22d0ccd01"), - (&ADDRESS_SLOT_BUILD, "fnv1a64:0084246676dae038"), + (&INLINE_BUILD, "fnv1a64:ef5b5fcd191a6c34"), + (&ADDRESS_SLOT_BUILD, "fnv1a64:f94b1d5fd7db5427"), ] { let layout = PoolLayout::resolve(&build_symbols(build), key()).unwrap(); diff --git a/src/pool/snapshot.rs b/src/pool/snapshot.rs index 5dd40df..50ad141 100644 --- a/src/pool/snapshot.rs +++ b/src/pool/snapshot.rs @@ -6,10 +6,10 @@ use std::time::{Duration, Instant}; use thiserror::Error; use super::decode::{ - MAX_POOL_HEADER_CHUNK, PAGE_SIZE, PoolHeaderLayout, SpecialPoolHeader, adjust_page_end_header, - big_page_hash, decode_descriptor_at, decode_large_requested_size, decode_lfh_subsegment, - decode_pool_header, decode_rb_root_for, decode_slist_header_next, decode_special_pool_header, - decode_vs_chunk, descriptor_backend, lfh_bitmap_state, read_u16, read_u32, read_u64, + MAX_POOL_HEADER_CHUNK, PAGE_SIZE, PoolHeaderLayout, SpecialPoolHeader, VS_UNIT, big_page_hash, + decode_descriptor_at, decode_large_requested_size, decode_lfh_subsegment, decode_pool_header, + decode_rb_root_for, decode_slist_header_next, decode_special_pool_header, decode_vs_chunk, + descriptor_backend, lfh_bitmap_state, read_u16, read_u32, read_u64, valid_descriptor_tree_signature, valid_page_segment_signature, valid_vs_signature, }; use super::{ @@ -112,6 +112,32 @@ pub(crate) struct PoolRegion { pub pool_header: PoolHeaderLayout, pub vs_header_size: usize, pub vs_sizes_offset: usize, + /// Whether this heap's VS allocator keeps a user block out of the last [`VS_UNIT`] bytes of + /// a page: `_HEAP_VS_CONTEXT.Config.Flags.PageAlignLargeAllocs`, read once at discovery. + /// + /// A VS chunk's user block ordinarily begins right after its `_HEAP_VS_CHUNK_HEADER`. With + /// this set, `nt!RtlpHpVsContextAllocate` tests whether `chunk + 0x20` is page-aligned and + /// hands out `chunk + 0x20` when it is — one unit later than usual, with the usable size one + /// unit shorter. Read on x64 26100.33438 at `+0xf6..+0x12a`: `lea rdi,[rsi+10h]`, then + /// `test byte ptr [rbp+4],1` / `lea rax,[rsi+20h]` / `test rax,0FFFh` / `lea rdi,[rsi+20h]` / + /// `add ebx,0FFFFFFF0h`, `rbp` being the context and `+4` its `Config`. + /// `RtlpHpVsContextAllocateFinalize+0x3d` does the same for a multi-allocation, and + /// `RtlpHpVsContextFree+0xf2` is the mirror: under the flag, a page-aligned user block's + /// chunk is taken from `block - 0x20`. The kernel needs it because `ExFreeHeapPool+0xd6` + /// tells a chunk from a page allocation by `P & 0xfff`, and a `_POOL_HEADER` in the last + /// sixteen bytes of a page would hand its caller a page-aligned `P`. + /// + /// What that looked like from here before the walk consulted it (windbg-mcp `FOLLOWUPS.md` + /// item 101): every chunk whose header sat at page offset 0xfe0 had its pool header decoded + /// at 0xff0, which holds whatever the chunk held when it was free — two tree pointers — so + /// the tag was the top half of a kernel address, the span began sixteen bytes early and ran + /// sixteen bytes long, and the real header at the page start went unclaimed. Measured + /// 2026-10-14 on `ctf-vm`: 1,275 paged chunks under the "tag" `0x84e7ffff`, every one with + /// its candidate at 0xff0; the chunk at `0xffffe78459203fe0` is `0x80` bytes, holds + /// `ffffe78459207e18 ffffe78459213bc8` at `+0x10`, and has at `0xffffe78459204000` the + /// `MmDT` header `!pool` names there (`size: 60`). Both pool heaps on that kernel carry the + /// flag; no target without it has been measured, which is what gating on it is for. + pub vs_skips_page_end: bool, pub known_tag: Option, /// Allocator-derived states for segment/page-range cells. pub states: Vec, @@ -410,6 +436,12 @@ fn tree_nodes( label: &str, diagnostics: &mut Vec, ) -> Result, SnapshotError> { + // Polled before the root and flag reads, not only inside the loop below: every caller + // reaches here a few unpolled reads past its own poll, which is a stretch of up to five + // between checks, and the sampled budgets in + // `test_the_walk_stops_reading_promptly_once_its_budget_is_gone` landed on it once the + // per-heap config read shifted them by one (2026-10-14). + check_budget(memory)?; let Ok(root_offset) = layout.field("_RTL_RB_TREE", "Root") else { diagnostics.push(format!("cannot resolve {label} root field")); return Ok(Vec::new()); @@ -520,8 +552,12 @@ fn insert_cached_chunk_candidates( let overhead = pool_header_size.saturating_add(vs_header_size); if let Some(header) = entry.checked_sub(overhead) { cached.insert(header); + // The lookaside's half of [`PoolRegion::vs_skips_page_end`]: data sixteen bytes into a + // page has its pool header at the page start and, under the flag, its chunk header one + // unit before where the subtraction above puts it. Both are kept, because this is a + // set of candidates and only a chunk decoded at one of them can match. if header & (PAGE_SIZE - 1) == PAGE_SIZE - pool_header_size { - cached.insert(header.saturating_sub(16)); + cached.insert(header.saturating_sub(VS_UNIT)); } } } @@ -847,6 +883,49 @@ fn vs_roots( Ok(roots) } +/// `_RTL_HP_VS_CONFIG.Flags.PageAlignLargeAllocs`: `Pos 0, 1 Bit` of the byte at +/// `_HEAP_VS_CONTEXT.Config` on the x64 26100.33438 PDB, and the first flag the type declares. +/// The byte's offset is resolved; the bit's position inside it is this. +const VS_CONFIG_PAGE_ALIGN: u64 = 1; + +/// Whether `heap_address`'s VS allocator moves a user block off the end of a page — see +/// [`PoolRegion::vs_skips_page_end`] for the rule and where it was read. +/// +/// Asked once per heap, here, because what it decides is applied to every chunk. A layout that +/// did not resolve `Config`, or a context that will not read, answers **unset** and says so: that +/// is the reading the walk made before it consulted the flag at all, and on a target that does +/// carry it the diagnostic is what points at the ~1,300 misplaced headers per boot it costs. +/// An `Err` is the budget or an interrupt, polled before the read like every other read here. +fn vs_skips_page_end( + memory: &impl PoolMemory, + layout: &PoolLayout, + heap_address: u64, + diagnostics: &mut Vec, +) -> Result { + let Ok(context_offset) = layout.field("_SEGMENT_HEAP", "VsContext") else { + return Ok(false); + }; + let Ok(config_offset) = layout.field("_HEAP_VS_CONTEXT", "Config") else { + diagnostics.push(format!( + "VS context of heap {heap_address:#x} has no resolved Config: a user block ending \ + a page is read at its chunk header" + )); + return Ok(false); + }; + check_budget(memory)?; + let address = heap_address + context_offset as u64 + config_offset as u64; + Ok(match scalar(memory, address, 1) { + Ok(flags) => flags & VS_CONFIG_PAGE_ALIGN != 0, + Err(error) => { + diagnostics.push(format!( + "cannot read VS context config at {address:#x}: {error}; a user block ending a \ + page is read at its chunk header" + )); + false + } + }) +} + fn discover_vs_evidence( memory: &impl PoolMemory, layout: &PoolLayout, @@ -1006,6 +1085,8 @@ fn discover_heap_regions( traversal_limit, &mut discovery.diagnostics, )?; + let vs_skips_page_end = + vs_skips_page_end(memory, layout, heap_address, &mut discovery.diagnostics)?; // Touch both LFH roots through guarded reads. Descriptor metadata remains the // authoritative source of active subsegments, while these probes make corrupt @@ -1042,6 +1123,7 @@ fn discover_heap_regions( traversal_limit, &reusable_chunks, &cached_chunks, + vs_skips_page_end, discovery, ) { // As in `discover_pool_regions`: a halt is about the walk, not about this @@ -1085,6 +1167,7 @@ fn discover_segment_context( traversal_limit: usize, reusable_chunks: &SharedChunks, cached_chunks: &SharedChunks, + vs_skips_page_end: bool, discovery: &mut Discovery, ) -> Result<(), SnapshotError> { let segment = layout.type_layout("_HEAP_PAGE_SEGMENT")?; @@ -1468,6 +1551,7 @@ fn discover_segment_context( pool_header, vs_header_size: layout.type_layout("_HEAP_VS_CHUNK_HEADER")?.size as usize, vs_sizes_offset: layout.field("_HEAP_VS_CHUNK_HEADER", "Sizes")?, + vs_skips_page_end, known_tag, states: vec![state], reusable_chunks: Arc::clone(reusable_chunks), @@ -1591,6 +1675,7 @@ fn discover_large_allocations( pool_header, vs_header_size: 0, vs_sizes_offset: 0, + vs_skips_page_end: false, known_tag: Some(tag), states: vec![PoolState::Allocated], reusable_chunks: Arc::default(), @@ -3145,23 +3230,18 @@ impl<'a, M: PoolMemory> SnapshotWalker<'a, M> { snapshot.complete = false; break; } - let candidate = header_address + region.vs_header_size as u64; - let physical_header = if region.pool_header.size == 0 { - candidate - } else { - let Some(header) = - adjust_page_end_header(candidate, region.pool_header.size as u64) - else { - // Where this chunk's *pool* header sits could not be worked out, so no span - // is emitted for it. Its size decoded and passed the bound check, though, so - // the chunk chain is not what was lost here and the next extent can still be - // placed. - resume = header_address.saturating_add(chunk_size as u64); - snapshot.complete = false; - break; - }; - header - }; + // Where the chunk's user block begins: the pool header on a kernel target, the + // caller's own bytes on a user-mode one. Right after the chunk header, except where + // that is the last unit of a page and the allocator hands out the next page start + // instead — see [`PoolRegion::vs_skips_page_end`]. The chain is not involved: the + // chunk's extent is what its header says, and only where the user block sits in it + // moves. + let mut physical_header = header_address + region.vs_header_size as u64; + if region.vs_skips_page_end + && physical_header.saturating_add(VS_UNIT) & (PAGE_SIZE - 1) == 0 + { + physical_header += VS_UNIT; + } let pool_offset = physical_header.saturating_sub(base) as usize; let tag = decode_pool_header(bytes, pool_offset, region.pool_header) .map_or(0, |header| header.tag); @@ -3376,6 +3456,7 @@ mod tests { }, vs_header_size: 0, vs_sizes_offset: 0, + vs_skips_page_end: false, known_tag: None, states: Vec::new(), reusable_chunks: Arc::default(), @@ -4128,6 +4209,7 @@ mod tests { pool_header: X64_POOL_HEADER, vs_header_size: 0, vs_sizes_offset: 0, + vs_skips_page_end: false, known_tag: None, states: Vec::new(), reusable_chunks: Arc::default(), @@ -4159,6 +4241,7 @@ mod tests { pool_header: X64_POOL_HEADER, vs_header_size: 0x10, vs_sizes_offset: 0, + vs_skips_page_end: false, known_tag: None, states: Vec::new(), reusable_chunks: Arc::default(), @@ -4187,7 +4270,10 @@ mod tests { } fn walk_vs_extent(bytes: &[u8]) -> PoolSnapshot { - let region = vs_region(bytes.len()); + walk_vs_region(vs_region(bytes.len()), bytes) + } + + fn walk_vs_region(region: PoolRegion, bytes: &[u8]) -> PoolSnapshot { let memory = FlatMemory::new(VS_BASE, bytes.len()); let layout = vs_layout(VsFixture::Inline); let walker = SnapshotWalker { @@ -4203,6 +4289,88 @@ mod tests { snapshot } + /// The measured shape of a VS chunk whose header sits at page offset 0xfe0 under + /// `PageAlignLargeAllocs` ([`PoolRegion::vs_skips_page_end`]): its user block is the next + /// page start, and the unit before that holds whatever the chunk held while it was free — + /// on `ctf-vm`, two tree pointers whose top half reads as a tag. + /// + /// Three chunks from [`VS_BASE`], the middle one at `0x2fe0` with its pool header at + /// `0x3000` and the stale pointers at `0x2ff0`, as `dq 0xffffe78459203fe0` read them on + /// 2026-10-14 (`c1914c0d038da756 ffffe78400000056 | ffffe78459207e18 ffffe78459213bc8 | + /// 54446d4d03060000`). + fn vs_extent_with_a_user_block_moved_off_the_page_end() -> Vec { + let mut bytes = vs_extent(&[(0xfe0, 0), (0x80, 0xfe0), (0x40, 0x80)]); + bytes[0xff0..0xff8].copy_from_slice(&0xffff_e784_5920_7e18u64.to_le_bytes()); + bytes[0xff8..0x1000].copy_from_slice(&0xffff_e784_5921_3bc8u64.to_le_bytes()); + bytes[0x1000..0x1004].copy_from_slice(&0x0306_0000u32.to_le_bytes()); + bytes[0x1004..0x1008].copy_from_slice(b"MmDT"); + bytes + } + + fn span_of_class(snapshot: &PoolSnapshot, size_class: u32) -> &PoolSpan { + snapshot + .spans + .iter() + .find(|span| span.size_class == size_class) + .unwrap_or_else(|| panic!("no span of class {size_class:#x}: {:?}", snapshot.spans)) + } + + /// Under the flag, a user block that would begin in a page's last unit begins the next page: + /// the header is where `!pool` says (`size: 60`, so 0x50 usable), the tag is the real one, + /// and the chain is untouched — the chunk after is found where the sizes put it. + #[test] + fn test_a_vs_user_block_that_would_end_a_page_begins_the_next_one() { + let bytes = vs_extent_with_a_user_block_moved_off_the_page_end(); + let mut region = vs_region(bytes.len()); + region.vs_skips_page_end = true; + + let snapshot = walk_vs_region(region, &bytes); + + let moved = span_of_class(&snapshot, 0x80); + assert_eq!(moved.header_address, VS_BASE + 0x1000); + assert_eq!(moved.usable_address, VS_BASE + 0x1010); + assert_eq!(moved.size, 0x50); + assert_eq!(moved.raw_tag, u32::from_le_bytes(*b"MmDT")); + let after = span_of_class(&snapshot, 0x40); + assert_eq!(after.header_address, VS_BASE + 0x1060 + 0x10); + assert_eq!(after.raw_tag, u32::from_le_bytes(*b"VS!!")); + assert_eq!(snapshot.spans.len(), 3); + assert!(snapshot.complete, "{:?}", snapshot.diagnostics); + } + + /// The control, and what the flag gates: without it the same bytes read as they did before + /// the walk consulted it — the stale pointer as the header, its top half as the tag, a span + /// a unit early and a unit long. That is what a target **without** the flag would hold, so + /// it is what the walk has to go on reading there. + #[test] + fn test_without_the_flag_a_page_end_user_block_is_read_at_its_chunk_header() { + let bytes = vs_extent_with_a_user_block_moved_off_the_page_end(); + + let snapshot = walk_vs_extent(&bytes); + + let unmoved = span_of_class(&snapshot, 0x80); + assert_eq!(unmoved.header_address, VS_BASE + 0xff0); + assert_eq!(unmoved.size, 0x60); + assert_eq!(unmoved.raw_tag, 0xffff_e784); + } + + /// The allocator's test is `chunk + 0x20` page-aligned, not `chunk + 0x10`: a user block + /// that begins exactly on a page boundary is not in the last unit of the page before, and + /// stays where it is. + #[test] + fn test_a_vs_user_block_on_a_page_boundary_is_left_where_it_is() { + let bytes = vs_extent(&[(0xff0, 0), (0x40, 0xff0)]); + let mut region = vs_region(bytes.len()); + region.vs_skips_page_end = true; + + let snapshot = walk_vs_region(region, &bytes); + + let on_boundary = span_of_class(&snapshot, 0x40); + assert_eq!(on_boundary.header_address, VS_BASE + 0x1000); + assert_eq!(on_boundary.size, 0x20); + assert_eq!(on_boundary.raw_tag, u32::from_le_bytes(*b"VS!!")); + } + /// A VS chunk too large for `_POOL_HEADER.BlockSize` carries no header, and its tag is in /// the big-page table like any other big-pool allocation. /// @@ -5448,6 +5616,16 @@ mod tests { struct Writes(Vec<(u64, Vec)>); impl SyntheticMemory { + /// Overwrites one byte inside a range an earlier write already covers. + fn poke(&mut self, address: u64, byte: u8) { + let (start, data) = self + .runs + .iter_mut() + .find(|(start, data)| address >= *start && address < *start + data.len() as u64) + .expect("the address is inside a written run"); + data[(address - *start) as usize] = byte; + } + fn new(writes: Writes, holes: Vec<(u64, u64)>) -> Self { // Extents first: merge every written interval that overlaps or abuts a neighbour, // which yields exactly the runs the byte map used to produce by iterating in key @@ -5680,6 +5858,7 @@ mod tests { ("FreeChunkTree", 0), ("DelayFreeContext", 0x10), ("SubsegmentList", 0x30), + ("Config", 0x38), ], ), ); @@ -6410,6 +6589,7 @@ mod tests { 1024, &SharedChunks::default(), &SharedChunks::default(), + false, &mut discovery, ) .expect("the fixture is readable throughout"); @@ -6469,6 +6649,7 @@ mod tests { 1024, &SharedChunks::default(), &SharedChunks::default(), + false, &mut discovery, ) .expect("the fixture is readable throughout"); @@ -6568,6 +6749,7 @@ mod tests { 1024, &SharedChunks::default(), &SharedChunks::default(), + false, &mut discovery, ) .expect("the fixture is readable throughout"); @@ -6704,6 +6886,7 @@ mod tests { 1024, &SharedChunks::default(), &SharedChunks::default(), + false, &mut discovery, ); @@ -7233,6 +7416,90 @@ mod tests { assert!(SPECIAL_POOL_KINDS[3].is_paged()); } + /// Whether a VS user block skips a page end is read off the target's own context, once per + /// heap, and lands on every region of that heap — bit 0 of `Config` and nothing else, since + /// bit 1 (`EnableDelayFree`) shares the byte. A layout with no `Config` reads unset and says + /// so, which is the one way a target carrying the flag can be walked without it. + #[test] + fn test_discovery_reads_whether_a_vs_user_block_skips_a_page_end_from_the_context() { + let identity = HeapIdentity { + pool_state: STATE, + heap: HEAP, + special: false, + }; + let discover = |memory: &SyntheticMemory, layout: &PoolLayout| { + let mut discovery = Discovery::default(); + discover_heap_regions( + memory, + layout, + HEAP, + 0, + PoolKind::NonPagedNx, + identity, + None, + 0x55aa_1234_9876_0000, + 0xa5c3_1357, + 1024, + &mut discovery, + ) + .expect("the fixture is readable throughout"); + discovery + }; + let config = HEAP + 0x300 + 0x38; + for (flags, expected) in [(0u8, false), (1, true), (2, false), (3, true)] { + let mut memory = synthetic_memory(); + memory.poke(config, flags); + + let discovery = discover(&memory, &synthetic_layout()); + + let vs: Vec<_> = discovery + .regions + .iter() + .filter(|region| region.backend == PoolBackend::Vs) + .collect(); + assert!(!vs.is_empty(), "the fixture has a VS subsegment"); + assert!( + vs.iter().all(|region| region.vs_skips_page_end == expected), + "config {flags:#x} should read {expected}" + ); + assert!( + !discovery + .diagnostics + .iter() + .any(|line| line.contains("Config")), + "{:?}", + discovery.diagnostics + ); + } + + let mut memory = synthetic_memory(); + memory.poke(config, 1); + let mut layout = synthetic_layout(); + layout + .types + .get_mut("_HEAP_VS_CONTEXT") + .unwrap() + .fields + .remove("Config"); + + let discovery = discover(&memory, &layout); + + assert!( + discovery + .regions + .iter() + .all(|region| !region.vs_skips_page_end) + ); + assert!( + discovery + .diagnostics + .iter() + .any(|line| line.contains("has no resolved Config")), + "{:?}", + discovery.diagnostics + ); + } + #[test] fn test_discovery_skips_missing_optional_large_layout() { let memory = big_page_memory(LARGE_VA, 4, 0); From 248b5cdbd995d51f078f5874892aa077bdebe901 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Wed, 14 Oct 2026 19:30:44 +0100 Subject: [PATCH 2/2] Review round 1 on #199: a chunk with no unit to skip keeps its user block in place Codex: `decode_vs_chunk` admits a chunk against its two headers alone, so a 0x20-byte chunk at page offset 0xfe0 under the flag had its user block placed at its own end, which is the next chunk's header, and a zero-size span outside the chunk. Right. The remedy is not to refuse the chunk, as proposed: a refusal loses the chain for the rest of the region, and the chunk is a well-formed free remainder, the smallest a split leaves. The block is moved only where the chunk has the unit to move it by; one with no room is read in place, and an allocated one that short is reported, since the allocator asks for one more unit than a request needs under the flag (`RtlpHpVsContextAllocate+0x1a..+0x3c`) and so never hands one out. `test_a_vs_chunk_too_short_to_move_its_user_block_is_read_in_place` fails with the gate removed: the span's header then sits at the next page start. Co-Authored-By: Claude Mythos 5.1 Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT --- src/pool/snapshot.rs | 50 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/pool/snapshot.rs b/src/pool/snapshot.rs index 50ad141..2b77bfe 100644 --- a/src/pool/snapshot.rs +++ b/src/pool/snapshot.rs @@ -3240,7 +3240,23 @@ impl<'a, M: PoolMemory> SnapshotWalker<'a, M> { if region.vs_skips_page_end && physical_header.saturating_add(VS_UNIT) & (PAGE_SIZE - 1) == 0 { - physical_header += VS_UNIT; + // Only where the chunk has the unit to skip. `decode_vs_chunk` admitted this + // chunk against its two headers alone, and a chunk exactly that size — the + // smallest free remainder a split leaves — would otherwise have its user block + // placed at its own end, which is the next chunk's header. The allocator asks + // for one more unit than a request needs under the flag + // (`RtlpHpVsContextAllocate+0x1a..+0x3c`: `and eax,1` / `inc eax` / + // `add eax,r10d`), so a chunk here with no room is one nothing was allocated + // from, and it is read in place; an *allocated* one contradicts that and is + // reported. + if chunk_size >= header_bytes.saturating_add(VS_UNIT as usize) { + physical_header += VS_UNIT; + } else if chunk.allocated { + snapshot.diagnostics.push(format!( + "VS chunk at {header_address:#x} is {chunk_size:#x} bytes, too short to \ + hold a user block moved off the page end; read in place" + )); + } } let pool_offset = physical_header.saturating_sub(base) as usize; let tag = decode_pool_header(bytes, pool_offset, region.pool_header) @@ -4371,6 +4387,38 @@ mod tests { assert_eq!(on_boundary.raw_tag, u32::from_le_bytes(*b"VS!!")); } + /// A chunk at offset 0xfe0 with no unit to skip — two headers' worth, the smallest remainder + /// a split leaves — keeps its user block in place rather than having it placed at its own + /// end, which is the next chunk's header; and since the fixture marks every chunk allocated, + /// which the allocator would never do to one this short under the flag, the walk says so. + /// Raised by Codex on dbgscope#199. + #[test] + fn test_a_vs_chunk_too_short_to_move_its_user_block_is_read_in_place() { + let bytes = vs_extent(&[(0xfe0, 0), (0x20, 0xfe0), (0x40, 0x20)]); + let mut region = vs_region(bytes.len()); + region.vs_skips_page_end = true; + + let snapshot = walk_vs_region(region, &bytes); + + let short = span_of_class(&snapshot, 0x20); + assert_eq!(short.header_address, VS_BASE + 0xff0); + assert_eq!(short.usable_address, VS_BASE + 0x1000); + assert_eq!(short.size, 0); + let after = span_of_class(&snapshot, 0x40); + assert_eq!(after.header_address, VS_BASE + 0x1010); + assert_eq!(after.raw_tag, u32::from_le_bytes(*b"VS!!")); + assert_eq!(snapshot.spans.len(), 3); + assert!( + snapshot + .diagnostics + .examples() + .iter() + .any(|line| line.contains("too short to hold a user block moved off the page end")), + "{:?}", + snapshot.diagnostics + ); + } + /// A VS chunk too large for `_POOL_HEADER.BlockSize` carries no header, and its tag is in /// the big-page table like any other big-pool allocation. ///