From dfa4b9165b2f7bae6ff0a90187c5a56dafd3cb14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 24 Sep 2026 11:00:18 +0100 Subject: [PATCH] fix(pool): a VS chunk too large for a pool header is big pool too The previous fix asked the page range descriptor which allocations carry no `_POOL_HEADER`, and that is only where most of them are. `nt` also puts them inside VS subsegments, where the descriptor says `0x0f` and the chunk chain runs straight through them -- so `0xffffac09da29f000` on a live 26100.33438 kernel is an `MiRr` allocation of `0xe1c0` bytes to `!pool` and was 57,792 untagged bytes to the walk. The same length; only the name lost. The size answers it, and structurally rather than by measurement. `_POOL_HEADER.BlockSize` is **eight bits** of sixteen-byte units (`dt nt!_POOL_HEADER`, x64 26100.33438), so `0xff * 16` -- 4080 bytes of chunk, its own header included -- is the most it can describe: 4080 of payload plus the header is exactly a page, and one byte more has nowhere to record its own length. That is *why* such an allocation is in `nt!PoolBigPageTable`, and it is what the table says back: every one of the 7,639 live entries on that guest recorded `NumberOfBytes` of `0x1000` or more, and none fewer. A chunk past the limit is matched against the entries discovery resolved inside its region, **by containment and length** rather than by arithmetic on the chunk header. The table's `Va` is where the allocation begins and `NumberOfBytes` is how long it is; taking both as given costs nothing and assumes nothing about what sits between the chunk header and the data -- which on that guest is 0x10 that is measured and not yet explained, and which fitting the tag to would have been a guess. Three rules, each mutation-verified by breaking it: the limit itself, the containment match, and the length check -- the last of which the first draft did not pin at all, its fixture's entry fitting either way. windbg-mcp FOLLOWUPS.md item 99. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MUhLUt9rB6zd42Y25btB3h --- CHANGELOG.md | 16 ++++ src/pool/decode.rs | 14 +++ src/pool/snapshot.rs | 211 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 225 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c47ee4d..070584c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,22 @@ All notable changes to this project are documented here. The format follows ### Fixed +- **And a big-pool allocation served out of a VS subsegment kept no tag either.** The fix above + asked the page range descriptor which allocations have no `_POOL_HEADER`, and that is only where + *most* of them are: `nt` also puts them inside VS subsegments, where the descriptor says `0x0f` + and the chunk chain runs straight through them. The size answers it instead, and structurally + rather than by measurement — `_POOL_HEADER.BlockSize` is **eight bits** of sixteen-byte units + (`dt nt!_POOL_HEADER`, x64 26100.33438), so 4080 bytes of chunk is the most it can describe and + an allocation needing a page has nowhere to record its own length. That is why every one of the + 7,639 live entries on a 26100 guest recorded `NumberOfBytes` of `0x1000` or more, and none + fewer. A chunk past that limit is now matched against the entries discovery found inside its + region, **by containment and length** rather than by arithmetic on the chunk header: the table's + `Va` is where the allocation starts and `NumberOfBytes` is how long it is, and taking both as + given assumes nothing about what sits between the chunk header and the data — which on that + guest is 0x10 that is measured and not yet explained. Measured there: `!pool` calls + `0xffffac09da29f000` an `MiRr` allocation of `0xe1c0` bytes and the walk called it 57,792 + untagged bytes — the same length, with only the name lost. windbg-mcp `FOLLOWUPS.md` item 99. + - **Every big-pool allocation was reported with a tag read out of the caller's own data.** `ExAllocatePoolWithTag` sends anything that will not fit inside a page to `ExpAllocateBigPool`, which takes whole pages from the segment allocator and records the tag and length in diff --git a/src/pool/decode.rs b/src/pool/decode.rs index 2adb2b4..6497135 100644 --- a/src/pool/decode.rs +++ b/src/pool/decode.rs @@ -687,6 +687,20 @@ pub(crate) fn decode_large_requested_size( allocated_bytes.checked_sub(unused) } +/// The largest chunk a `_POOL_HEADER` can describe, its own sixteen bytes included. +/// +/// `BlockSize` is **eight bits**, in sixteen-byte units — `dt nt!_POOL_HEADER` on x64 26100.33438, +/// `+0x002 BlockSize : Pos 0, 8 Bits` — so the largest it can encode is `0xff * 16`. An allocation +/// that needs a whole page therefore cannot carry a header at all: 4080 bytes of payload plus the +/// header is exactly 4096, and one byte more has nowhere to record its own length. +/// +/// That is *why* `nt` keeps such an allocation in `nt!PoolBigPageTable` instead, and it makes the +/// question "does this chunk have a header?" answerable from its size rather than from where it +/// came from — which matters, because the two allocators that produce them are indistinguishable +/// by their page range descriptors. It also matches the table: every one of the 7,639 live entries +/// on a 26100 guest (2026-09-24) recorded `NumberOfBytes` of `0x1000` or more, and none fewer. +pub(crate) const MAX_POOL_HEADER_CHUNK: u64 = 0xff * 16; + pub(crate) fn is_kernel_pointer(pointer: u64) -> bool { pointer == 0 || pointer >= 0xffff_8000_0000_0000 } diff --git a/src/pool/snapshot.rs b/src/pool/snapshot.rs index 27e1eb1..fcdfcee 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::{ - 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, 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, valid_descriptor_tree_signature, valid_page_segment_signature, valid_vs_signature, }; use super::{ @@ -126,6 +126,13 @@ pub(crate) struct PoolRegion { /// VS chunk-header addresses present in delay-free/lookaside lists. Shared for the /// reason [`Self::reusable_chunks`] is. pub cached_chunks: SharedChunks, + /// Every `nt!PoolBigPageTable` entry whose allocation begins inside this region, by the + /// address it begins at. + /// + /// Resolved during discovery, where the table is, so that walking a chunk needs no reads of + /// its own. Empty on every region that holds no such allocation, which is most of them — and + /// on every user-mode one, which has no such table. + pub big_pool: Arc>, } pub(crate) trait PoolMemory { @@ -1353,6 +1360,27 @@ fn discover_segment_context( // Free ranges are deliberately not looked up: `ExpRemoveTagForBigPages` takes the // entry out as the allocation goes away, so a miss there would be the expected // answer rather than a finding. + // Every big-pool allocation inside a VS subsegment, resolved here because the table + // is here and a chunk walk has no reads to spare. `_POOL_HEADER.BlockSize` cannot + // describe a chunk of `MAX_POOL_HEADER_CHUNK` or more, so `nt` gives those no header + // at all and records them in `nt!PoolBigPageTable` — **wherever they came from**. + // They are not only the plain page ranges below: a VS subsegment holds them too, and + // every entry's `Va` is page-aligned, so one probe per page of the chunk area finds + // each one. Measured on a live 26100 kernel, where `0xffffac09da29f000` is an + // `MiRr` allocation of `0xe1c0` bytes inside the 17-page VS range at page 0x53 of + // its segment (`windbg-mcp` FOLLOWUPS item 99). + let mut big_pool = HashMap::new(); + if backend == PoolBackend::Vs && !identity.special { + let mut page = region_address.next_multiple_of(PAGE_SIZE); + let end = region_address.saturating_add(region_size as u64); + while page < end { + if let Some(entry) = discovery.big_page(memory, page)? { + big_pool.insert(page, entry); + } + page = page.saturating_add(PAGE_SIZE); + } + } + let big_pool = Arc::new(big_pool); let mut pool_header = layout.pool_header_layout()?; let mut known_tag = None; if backend == PoolBackend::Segment @@ -1389,6 +1417,7 @@ fn discover_segment_context( states: vec![state], reusable_chunks: Arc::clone(reusable_chunks), cached_chunks: Arc::clone(cached_chunks), + big_pool: Arc::clone(&big_pool), }); descriptor_index += unit_size; } @@ -1511,6 +1540,7 @@ fn discover_large_allocations( states: vec![PoolState::Allocated], reusable_chunks: Arc::default(), cached_chunks: Arc::default(), + big_pool: Arc::default(), }); } Ok(()) @@ -1527,11 +1557,11 @@ const BIG_PAGE_BATCH: usize = 256; /// What `nt!PoolBigPageTable` records about one allocation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -struct BigPageEntry { - tag: u32, +pub(crate) struct BigPageEntry { + pub tag: u32, /// The length the caller asked for, which is *not* the length of the pages it was given: /// a 0xa270-byte request occupies 0xb000 of pool and this field says 0xa270. - size: u64, + pub size: u64, } /// `nt!PoolBigPageTable`: where the kernel keeps the tag of every allocation too large to @@ -2846,6 +2876,49 @@ impl<'a, M: PoolMemory> SnapshotWalker<'a, M> { snapshot.complete = false; break; } + let state = if region.cached_chunks.contains(&header_address) { + PoolState::CachedFree + } else if region.reusable_chunks.contains(&header_address) { + PoolState::ReusableFree + } else if chunk.allocated { + PoolState::Allocated + } else { + PoolState::ReusableFree + }; + // **A chunk too large for `_POOL_HEADER.BlockSize` carries no header**, so decoding + // one out of it reads the caller's own first sixteen bytes as a tag. Its name is in + // `nt!PoolBigPageTable`, which discovery resolved into `region.big_pool`. + // + // Matched by **containment** rather than by arithmetic on the chunk header: the + // table's `Va` is where the allocation starts, and taking it as given costs nothing + // and assumes nothing about what sits between the two — which is the one thing here + // that is measured and not yet explained (`windbg-mcp` FOLLOWUPS item 99). The size + // has to fit inside the chunk as well, so an entry can only be claimed by a chunk + // that could really hold it. + let big_pool = (chunk_size as u64 > MAX_POOL_HEADER_CHUNK) + .then(|| { + let end = header_address.saturating_add(chunk_size as u64); + region.big_pool.iter().find(|(address, entry)| { + (header_address..end).contains(*address) + && address.saturating_add(entry.size) <= end + }) + }) + .flatten() + .map(|(address, entry)| (*address, *entry)); + if let Some((address, entry)) = big_pool { + let mut span = + self.base_span(region, address, address, entry.size, entry.tag, state); + span.size_class = chunk_size.min(u32::MAX as usize) as u32; + snapshot.record_span(span); + previous_chunk = Some(chunk_size); + offset += chunk_size; + resume = base + offset as u64; + chunks += 1; + if snapshot.match_limit_reached() { + break; + } + continue; + } let candidate = header_address + region.vs_header_size as u64; let physical_header = if region.pool_header.size == 0 { candidate @@ -2866,15 +2939,6 @@ impl<'a, M: PoolMemory> SnapshotWalker<'a, M> { 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); - let state = if region.cached_chunks.contains(&header_address) { - PoolState::CachedFree - } else if region.reusable_chunks.contains(&header_address) { - PoolState::ReusableFree - } else if chunk.allocated { - PoolState::Allocated - } else { - PoolState::ReusableFree - }; let overhead = physical_header .saturating_sub(header_address) .saturating_add(region.pool_header.size as u64); @@ -3087,6 +3151,7 @@ mod tests { states: Vec::new(), reusable_chunks: Arc::default(), cached_chunks: Arc::default(), + big_pool: Arc::default(), } } @@ -3838,6 +3903,7 @@ mod tests { states: Vec::new(), reusable_chunks: Arc::default(), cached_chunks: Arc::default(), + big_pool: Arc::default(), } } @@ -3868,6 +3934,7 @@ mod tests { states: Vec::new(), reusable_chunks: Arc::default(), cached_chunks: Arc::default(), + big_pool: Arc::default(), } } @@ -3907,6 +3974,118 @@ mod tests { snapshot } + /// 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. + /// + /// `BlockSize` is eight bits of sixteen-byte units, so 4080 bytes -- header included -- is + /// the most it can describe and anything needing a page has no header at all. The two + /// allocators that produce such a chunk are indistinguishable by their page range + /// descriptors, which is why the size answers this rather than the backend: measured on a + /// live 26100 kernel, `!pool` calls `0xffffac09da29f000` an `MiRr` allocation of `0xe1c0` + /// bytes and the walk called it 57,792 untagged bytes -- the same length, with the name + /// lost (`windbg-mcp` FOLLOWUPS item 99). + /// + /// The fixture writes a *valid* pool header into the big chunk on purpose. Without one the + /// wrong answer would be an absent tag, which a fixture of zeroes cannot tell from the right + /// one -- and the tag it writes is the one the small chunk beside it legitimately carries, + /// so a walk that reads the header anyway reports something entirely plausible. + #[test] + fn test_a_vs_chunk_too_large_for_a_pool_header_is_tagged_from_the_big_page_table() { + // One chunk of 0x2000 -- far past what `BlockSize` can encode -- then an ordinary one. + let mut bytes = vs_extent(&[(0x2000, 0), (0x40, 0x2000)]); + let mut region = vs_region(bytes.len()); + // The allocation the table names: page-aligned, as every live entry's `Va` is, and + // short of the chunk that holds it by the header space `nt` did not need. + let allocation = (VS_BASE + 0x10).next_multiple_of(PAGE_SIZE); + region.big_pool = Arc::new(HashMap::from([( + allocation, + BigPageEntry { + tag: u32::from_le_bytes(*b"BIGV"), + size: 0x1000, + }, + )])); + // And a second entry, for an allocation this extent does not hold, so the match has to + // be by containment rather than by "the region names exactly one". + Arc::get_mut(&mut region.big_pool) + .unwrap() + .insert(VS_BASE + 0x9000, BigPageEntry { tag: 0, size: 0x10 }); + let _ = &mut bytes; + let memory = FlatMemory::new(VS_BASE, bytes.len()); + let layout = vs_layout(VsFixture::Inline); + let walker = SnapshotWalker { + memory: &memory, + layout: &layout, + traversal_limit: 1000, + }; + let mut snapshot = PoolSnapshot { + complete: true, + ..PoolSnapshot::default() + }; + + walker.walk_vs(®ion, VS_BASE, &bytes, Some(VS_BASE), &mut snapshot); + + let big = snapshot + .spans + .iter() + .find(|span| span.size_class == 0x2000) + .unwrap_or_else(|| panic!("the large chunk is missing: {:?}", snapshot.spans)); + assert_eq!(super::super::decode::display_tag(big.raw_tag), "BIGV"); + // The table's `Va` is where the allocation begins and `NumberOfBytes` is how long it is; + // neither is the chunk, and the phantom header is gone from both. + assert_eq!(big.header_address, allocation); + assert_eq!(big.usable_address, allocation); + assert_eq!(big.size, 0x1000); + // The chunk beside it is under the limit, so it keeps the header it really has. + let small = snapshot + .spans + .iter() + .find(|span| span.size_class == 0x40) + .unwrap_or_else(|| panic!("the small chunk is missing: {:?}", snapshot.spans)); + assert_eq!(super::super::decode::display_tag(small.raw_tag), "VS!!"); + } + + /// An entry a chunk could not really hold is not that chunk's, however well it lines up. + /// + /// Containment alone is the cheap half of the match: an address inside the chunk. The length + /// is what says the two are the same allocation, and without it a chunk would adopt the + /// entry of whatever happens to begin inside it -- reporting somebody else's tag against a + /// length that does not fit, which is worse than the untagged answer it replaces. + #[test] + fn test_a_big_page_entry_too_long_for_its_chunk_is_not_claimed_by_it() { + let bytes = vs_extent(&[(0x2000, 0), (0x40, 0x2000)]); + let mut region = vs_region(bytes.len()); + let allocation = (VS_BASE + 0x10).next_multiple_of(PAGE_SIZE); + region.big_pool = Arc::new(HashMap::from([( + allocation, + BigPageEntry { + tag: u32::from_le_bytes(*b"BIGV"), + // One byte past what the chunk holding `allocation` can contain. + size: (VS_BASE + 0x2000 - allocation) + 1, + }, + )])); + let memory = FlatMemory::new(VS_BASE, bytes.len()); + let layout = vs_layout(VsFixture::Inline); + let walker = SnapshotWalker { + memory: &memory, + layout: &layout, + traversal_limit: 1000, + }; + let mut snapshot = PoolSnapshot { + complete: true, + ..PoolSnapshot::default() + }; + + walker.walk_vs(®ion, VS_BASE, &bytes, Some(VS_BASE), &mut snapshot); + + let big = snapshot + .spans + .iter() + .find(|span| span.size_class == 0x2000) + .unwrap_or_else(|| panic!("the large chunk is missing: {:?}", snapshot.spans)); + assert_ne!(super::super::decode::display_tag(big.raw_tag), "BIGV"); + assert_eq!(big.header_address, VS_BASE + 0x10); + } + /// glslang/dbgscope#93: "884x rejecting implausible VS chunk size # at #" counted the /// *extents* that contained a refusal, because the message was latched behind a bool. How /// many chunks were refused was reported nowhere — and a refusal is not free, since the