Repository navigation
fix(pool): read a VS user block that would end a page where the allocator put it - #199
Conversation
…ator 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca3e6a7dd5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if region.vs_skips_page_end | ||
| && physical_header.saturating_add(VS_UNIT) & (PAGE_SIZE - 1) == 0 | ||
| { | ||
| physical_header += VS_UNIT; |
There was a problem hiding this comment.
Reject chunks too small for the shifted header
When the flag is set and a chunk starts at page offset 0xfe0, this adds a 16-byte padding unit after decode_vs_chunk has validated the chunk against only vs_header_size + pool_header.size. A validly decoded 0x20-byte chunk therefore places physical_header at the chunk's end and the usable address beyond it (and may read the header outside bytes), producing a span outside the chunk instead of rejecting it. Include the conditional padding in the minimum-size validation before emitting the span.
Useful? React with 👍 / 👎.
…lock 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
What
walk_vsplaced the_POOL_HEADERof every VS chunk whose header sits at page offset0xfe0sixteen bytes early, and read two stale free-tree pointers as the header. windbg-mcpFOLLOWUPS.mditem 101, filed as "the chain drifts 0x10": it does not — the chain is right, and what moves is where the user block sits inside the chunk.The mechanism, read out of
nt(x64 26100.33438)Under
_HEAP_VS_CONTEXT.Config.Flags.PageAlignLargeAllocs,nt!RtlpHpVsContextAllocate+0xf6..+0x12a:RtlpHpVsContextAllocateFinalize+0x3ddoes the same for a multi-allocation;RtlpHpVsContextFree+0xf2is the mirror (test r10,0FFFh/add rdi,-10h: a page-aligned user block's chunk isblock - 0x20). The kernel needs it becauseExFreeHeapPool+0xd6tells a chunk from a page allocation byP & 0xfff.Measured
ctf-vm, 2026-10-14, through windbg-mcp's tool surface on the pinned dbgscope:pool_census: 1,275 paged allocations under the "tag"0x84e7ffff— the top half of a paged-pool address.pool_find_tagon it: every one has itsheader_addressat page offset0xff0.dq 0xffffe78459203fe0:c1914c0d038da756 ffffe78400000056(the VS header) |ffffe78459207e18 ffffe78459213bc8(what the walk read as a header) |54446d4d03060000(the real one,MmDT, BlockSize 6).!pool 0xffffe78459204000:*ffffe78459204000 size: 60 previous size: 0 (Allocated) *MmDT.VsContext.Config.Flags.PageAlignLargeAllocsread1.What changed
_HEAP_VS_CONTEXT.Configis a new optional field; discovery reads bit 0 once per heap intoPoolRegion::vs_skips_page_end, andwalk_vsmoves the user block by one unit where the flag is set andchunk + 0x20is page-aligned. A PDB without the field reads unset, with a diagnostic. Every layout fingerprint moves, since one more field is resolved; the two pinned intest_an_older_builds_schema_is_unchanged_by_the_self_relative_shapewere re-recorded and the test's doc says why.adjust_page_end_headeris removed: it moved a candidate back to the previous page's end and could not fire on a sixteen-byte-aligned one.tree_nodespolls the budget before its first read. The extra read per heap moved the sampled deadlines intest_the_walk_stops_reading_promptly_once_its_budget_is_goneonto a stretch of up to five reads between polls (FirstDescriptorIndex,SegmentMask, the free-page tree's root and flag) that no sampled budget had landed on before; the probe that found which reads those were is not in the diff.Tests
test_a_vs_user_block_that_would_end_a_page_begins_the_next_one— the measured bytes, as a fixture.test_without_the_flag_a_page_end_user_block_is_read_at_its_chunk_header— the control: same bytes, flag unset, the old reading (and what a target without the flag holds).test_a_vs_user_block_on_a_page_boundary_is_left_where_it_is— the test ischunk + 0x20, notchunk + 0x10.test_discovery_reads_whether_a_vs_user_block_skips_a_page_end_from_the_context— bit 0 and nothing else; a layout withoutConfigsays so.cargo test --lib: 459 passed.cargo clippy --all-targets -- -D warningson this host's rustc 1.99.0 reports lints inarm64.rs,dbgeng.rs,heap.rsand achunks_exactsite indecode.rsthat this diff does not touch.🤖 Generated with Claude Code
https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT