Repository navigation
A VS user block the allocator moved off a page end is placed where !pool places it (item 101) - #499
Conversation
…ool places it (item 101) The item was filed as "the VS chunk chain drifts 0x10", with a plan to find the chunk that introduces it. None does: the chain was right, and what moves is where the user block sits inside one class of chunk. Under `_HEAP_VS_CONTEXT.Config.Flags.PageAlignLargeAllocs`, `nt!RtlpHpVsContextAllocate` hands out `chunk + 0x20` rather than `chunk + 0x10` when the former is page-aligned, so a chunk whose header sits at page offset 0xfe0 has its `_POOL_HEADER` at the next page start, and the walk read the unit before it - two stale free-tree pointers, or zeroes - as the header. `RtlpHpVsContextFree` mirrors it, and `ExFreeHeapPool` is why: it tells a chunk from a page allocation by `P & 0xfff`. Measured 2026-10-14 through the tool surface, on the pinned dbgscope: - `ctf-vm` (26100.33438): `pool_census` lists 1,275 paged allocations under `0x84e7ffff`, the top half of a paged-pool address, and every one of their `header_address`es ends in `ff0`. `dq 0xffffe78459203fe0` reads the chunk header, then `ffffe78459207e18 ffffe78459213bc8`, then the `MmDT` header `!pool` names at `0xffffe78459204000`. Both pool heaps carry the flag. - `lab-nt` (29671): both pool heaps read `0x3`; the first 300 rows of `pool_find_tag 0x00000000` are all small VS chunks of paged pool with their header at `0xff0`, one per page. What changes here: - dbgscope#199 reads the flag at discovery and applies it in `walk_vs`; the pin moves to its branch head ca3e6a7 while it is open, and to the merge afterwards. Every `layout.fingerprint` moves with the field it now resolves: `bbc9a6daae9ff7a2` -> `38566b14fd9f4b50` on `ctf-vm`. - The live-kernel tier's `compare_vs_page_end_placement` puts both halves to the engine: no small VS chunk with its header at `0xff0`, and `!pool` reading a header at every page-aligned one the walk reports, with the walk's tag and sixteen bytes more than its usable size. Sampled from the walk on purpose, since this defect is one the walk's own answer shows. Run both ways against `ctf-vm`: on the old pin it fails with 255 chunks at `0xff0` (347s, target released cleanly); on the new pin it passes, 4 page-start VS headers agreeing with `!pool` and 231 small VS chunks checked with none at `0xff0` (242s). - `FOLLOWUPS.md` item 101 moves to `DONE.md` with what its entry got wrong, which was its title. Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly summarises the main change: placing the VS user block where !pool places it when the allocator moves it off a page end. It also identifies item 101. |
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks each page-bound seam
And matches pool tags by the stream
The headers settle in their place
The test confirms each sampled case
Then hops away, content and clean
Comment @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc8f5a1f7f
ℹ️ 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".
| assert!( | ||
| at_page_end.is_empty(), |
There was a problem hiding this comment.
On a target where _HEAP_VS_CONTEXT.Config.Flags.PageAlignLargeAllocs is clear or unavailable, a small VS allocation may legitimately retain its pool header at offset 0xff0; the pinned decoder explicitly preserves the old placement when that flag is unset. This unconditional assertion therefore makes the generic live-kernel smoke test fail on a correctly decoded target merely because it lacks the flag. Read/expose the per-heap flag and apply this invariant only when it is set, while using the !pool comparison for other targets.
Useful? React with 👍 / 👎.
…flag per heap Codex: the `0xff0` assertion was unconditional while the decoder it pins applies its rule only where `PageAlignLargeAllocs` is set, so a target without the flag would fail the tier for decoding correctly. Right. The remedy proposed was to expose the per-heap flag through the tool surface; the tier reads it the way its other two oracles read their facts instead - `dx` on `nt!ExPoolState`, one byte per (node, pool) a sampled chunk is from, no new server surface. On a heap without the flag the `0xff0` half withholds itself and says so, and the `!pool` half still runs, since a page-aligned header is where `!pool` reads one either way. A flag that will not read fails the tier rather than switching the assertion off; a heap `dx` cannot read through is one no chunk is from (`Heaps[0]` is null on `ctf-vm`), and a chunk claiming such a heap fails it too. The pin moves to dbgscope#199's new head (248b5cd), which keeps a chunk with no unit to skip read in place. Every census tag is scanned now, because a scan that stopped at its fourth page-start header had read one heap. Run against `ctf-vm`: three heaps read the flag set (nonpaged NX, paged, prototype paged), 744 small VS chunks checked with none at `0xff0`, four page-start headers agreeing with `!pool`, 270s. Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 065b085ccd
ℹ️ 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 answer.contains("Unable to read memory") { | ||
| return None; | ||
| } | ||
| panic!("the VS config of pool node {numa_node} heap {heap} could not be read:\n{answer}"); |
There was a problem hiding this comment.
On a kernel whose PDB lacks the optional _HEAP_VS_CONTEXT.Config field, dx returns a field-resolution error rather than Unable to read memory, so this branch panics even though the pinned decoder deliberately treats the missing field as an unset flag and correctly preserves the old placement. Fresh evidence after the earlier review comment is the new vs_page_align_flag implementation itself: it handles a clear byte and an unreadable heap, but not the documented missing-field case. Treat that result as false/unsupported and retain the !pool comparison instead of failing the live-kernel tier.
Useful? React with 👍 / 👎.
| let flagged = *flags.entry((numa_node, pool.clone())).or_insert_with(|| { | ||
| vs_heap_index(&pool) | ||
| .and_then(|heap| vs_page_align_flag(server, session, numa_node, heap)) |
There was a problem hiding this comment.
When the first VS chunk from a heap is encountered, this closure runs execute for the dx query; worker::raw_command unconditionally invalidates allocator snapshots, so the next pool_find_tag cannot use the cached snapshot and performs another live pool walk. This happens at least once whenever another census tag remains, and can recur as new heaps appear in later tags, adding one or more roughly 120-second walks to an already quarter-hour opt-in test despite the comment claiming subsequent calls are cached. Collect the sampled chunks/heap keys first and issue the raw flag queries only after the tag scan.
Useful? React with 👍 / 👎.
…without the field reads unset Two Codex findings at 065b085, both taken. A raw command drops the walk's cached snapshot (`raw_command` invalidates unconditionally), so a `dx` issued inside the tag scan cost the next `pool_find_tag` a fresh walk, once per heap met - three of them on `ctf-vm`, against a comment claiming the calls after the first were cached. The scan now collects the chunks and the heaps they are from, and the flags are read once it is done. A PDB without `_HEAP_VS_CONTEXT.Config` makes `dx` fail to resolve the field, not to read memory, and the helper panicked on that where the decoder reads the absent field as unset. The field's presence is now probed once with `dt nt!_HEAP_VS_CONTEXT`, and without it every heap reads unflagged, as the walk reads it: the `0xff0` half withholds itself and says why, the `!pool` half still runs. No PDB without the field has been met, so the `dt` line looked for is the one both lab kernels print. Run against `ctf-vm`: the same three heaps read the flag set, 744 small VS chunks checked with none at `0xff0`, four page-start headers agreeing with `!pool`, in 220s against 270s with the reads inside the scan. Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
dbgscope#199 was rebase-merged, its two commits landing on `main` as 32899a3 and 45c44eb, so the branch head 248b5cd this PR pinned while it was open is an ancestor of nothing there and would stop resolving once the branch is deleted. 45c44eb's tree is identical to 248b5cd's (`git diff --stat` between them is empty), so nothing the tier measured against the branch head changes. Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
…flag per heap Codex: the `0xff0` assertion was unconditional while the decoder it pins applies its rule only where `PageAlignLargeAllocs` is set, so a target without the flag would fail the tier for decoding correctly. Right. The remedy proposed was to expose the per-heap flag through the tool surface; the tier reads it the way its other two oracles read their facts instead - `dx` on `nt!ExPoolState`, one byte per (node, pool) a sampled chunk is from, no new server surface. On a heap without the flag the `0xff0` half withholds itself and says so, and the `!pool` half still runs, since a page-aligned header is where `!pool` reads one either way. A flag that will not read fails the tier rather than switching the assertion off; a heap `dx` cannot read through is one no chunk is from (`Heaps[0]` is null on `ctf-vm`), and a chunk claiming such a heap fails it too. The pin moves to dbgscope#199's new head (248b5cd), which keeps a chunk with no unit to skip read in place. Every census tag is scanned now, because a scan that stopped at its fourth page-start header had read one heap. Run against `ctf-vm`: three heaps read the flag set (nonpaged NX, paged, prototype paged), 744 small VS chunks checked with none at `0xff0`, four page-start headers agreeing with `!pool`, 270s. Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
…without the field reads unset Two Codex findings at 065b085, both taken. A raw command drops the walk's cached snapshot (`raw_command` invalidates unconditionally), so a `dx` issued inside the tag scan cost the next `pool_find_tag` a fresh walk, once per heap met - three of them on `ctf-vm`, against a comment claiming the calls after the first were cached. The scan now collects the chunks and the heaps they are from, and the flags are read once it is done. A PDB without `_HEAP_VS_CONTEXT.Config` makes `dx` fail to resolve the field, not to read memory, and the helper panicked on that where the decoder reads the absent field as unset. The field's presence is now probed once with `dt nt!_HEAP_VS_CONTEXT`, and without it every heap reads unflagged, as the walk reads it: the `0xff0` half withholds itself and says why, the `!pool` half still runs. No PDB without the field has been met, so the `dt` line looked for is the one both lab kernels print. Run against `ctf-vm`: the same three heaps read the flag set, 744 small VS chunks checked with none at `0xff0`, four page-start headers agreeing with `!pool`, in 220s against 270s with the reads inside the scan. Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT
1 participant
Closes
FOLLOWUPS.mditem 101. Stacked on dbgscope#199, which has merged: the pin is its rebase-merge head45c44eb(identical tree to the branch head the tier was run against).What the item was, and what it turned out to be
Filed as "the VS chunk chain drifts 0x10, and not from where it starts", with a plan to find the chunk that introduces the drift. The chain never drifted. Under
_HEAP_VS_CONTEXT.Config.Flags.PageAlignLargeAllocs,nt!RtlpHpVsContextAllocatehands outchunk + 0x20rather thanchunk + 0x10when the former is page-aligned — the kernel's way of keeping a_POOL_HEADERout of a page's last sixteen bytes, sinceExFreeHeapPooltells a chunk from a page allocation byP & 0xfff. So a chunk whose header sits at page offset0xfe0has its pool header at the next page start, and the walk read the unit before it as the header: two stale free-tree pointers (a "tag" that is the top half of a kernel address) or zeroes (the....bulk). The mechanism, instruction by instruction, is in the dbgscope PR and in theDONE.mdentry.Measured, 2026-10-14
ctf-vm26100.33438lab-nt29671VsContext.Config.Flags.PageAlignLargeAllocs, both pool heaps0x3)pool_census0x84e7ffff: 1,275 paged, 1.9 MBpool_find_tagon itheader_addressendsff00x00000000: all small VS, all at0xff0dq 0xffffe78459203fe0: header · two pointers ·MmDTat…4000, which!poolnames (size: 60)What changes here
layout.fingerprintmoves, since one more field is resolved:bbc9a6daae9ff7a2→38566b14fd9f4b50onctf-vm.compare_vs_page_end_placement, beside the two oracles ina_live_kernel_pool_walk_is_bounded_and_leaves_its_session_usable. It samples the walk on purpose — this defect is a class of header offsets the allocator cannot produce — scans every census tag, readsPageAlignLargeAllocsper heap through the engine after the scan (adxonnt!ExPoolState, oncedthas said the PDB carries the field; a raw command inside the scan would drop the cached snapshot), asserts no small VS chunk of a flagged heap has its header at0xff0, then asks!poolabout up to four page-aligned ones and requires a header there with the walk's tag and sixteen bytes more than its usable size. On a heap without the flag, or a PDB without the field, the0xff0half withholds itself and says so; the!poolhalf still runs. Big-pool chunks are excluded by size. Run both ways againstctf-vm:255 small VS chunk(s) have their header in the last sixteen bytes of a page(347 s, target released cleanly);744 small VS chunk(s) checked, none ending a page,4 page-start VS header(s) agreed with !pool(220 s).FOLLOWUPS.mditem 101 →DONE.md, with what its entry got wrong (the title, the "certain subsegments", the splitter it pointed at) and what is not established (a target without the flag, ARM64, x86, where the allocator finds the room).CHANGELOG.md,docs/smoke-test.md.Review rounds
0xff0assertion on the flag — taken, read through the engine rather than through a new server surface.dxinside the scan drops the cached snapshot and costs a walk per heap met — taken, reads moved after the scan (270 s → 220 s); a PDB without_HEAP_VS_CONTEXT.Configmakesdxfail to resolve the field — taken, probed once withdtand read as unset, as the decoder reads it.Gates
cargo fmt --all --check,cargo clippy --all-targets -- -D warnings, markdownlint (73 files, 0 issues),engine::tests::every_followups_citation_names_an_item_that_exists, the binary's 1,404 unit tests.🤖 Generated with Claude Code
https://claude.ai/code/session_011Gsd7tFcMV8ozSUCFWrusT