|

find_pte() has no PT_PAGESIZE check: 4KiB map into a 2MiB leaf corrupts RAM #7

Open Chris Tusa opened this issue 8 days ago
Chris Tusa opened this issue 8 days ago

find_pte() has no PT_PAGESIZE check, so a 4KiB map onto a 2MiB leaf writes a PTE through mapped RAM.

The defect

find_pte() (uts/i86pc/boot/boot_mmu.c) walks down levels testing only
PT_VALID. It never tests PT_PAGESIZE, so when it meets a large leaf it
treats the leaf's PFN as a page-table pointer and descends into it. A
kbm_map() of a 4KiB page whose VA falls inside an existing 2MiB leaf
therefore writes a PTE into whatever RAM that leaf maps.

Why it is still here

The MISC window depends on the behaviour today: GDT/IDT allocations are placed
at fixed VAs inside a 2MiB leaf and reached exactly this way. Fixing
find_pte() without first changing MISC would break boot, which is why rev 326
recorded it and moved on rather than fixing it.

It has already shaped two other decisions, so it is not inert:

  • kbm_map_kernel_pts() is in the tree and must stay uncalled — it would
    kbm_map() a 4KiB PTE on top of a 2MiB KPM leaf.
  • Any new kernel VA window (e.g. the design's section 9 scratch move) must not
    overlap an existing 2MiB leaf, for the same reason.

Filed so it is not rediscovered

This is the third time it has been re-derived from scratch. Filing it as a
known constraint rather than a surprise.

Context: docs/design/LIMINE_ARCHITECTURE.md section 5.6.

Chris Tusa commented 6 days ago

Evaluation (2026-09-02, rev 363, measured on alpha12 with mdb -k)

What actually happens today

find_pte() (boot_mmu.c) descends into a PT_VALID entry without testing
PT_PAGESIZE, so a level-0 request under a 2 MiB leaf uses the leaf's physical
base
as a page-table address: the "PTE" it returns points into the RAM the leaf
maps, at offset ((va >> 12) & 511) * 8. Every 4 KiB kbm_map() inside a leaf
therefore (a) writes 8 bytes of PTE-shaped garbage into the first 4 KiB page of
that leaf's RAM and (b) has no effect on translation -- the CPU keeps resolving
the VA through the leaf.

Observed, MISC leaf at 0xfffffffffb600000 -> physical 0x10c600000:

::vtop GDT_VA        fb7fe000 -> 10c7fe000      (leaf base + offset, not a dedicated page)
::vtop IDT_VA        fb7fd000 -> 10c7fd000
::vtop DEBUG_INFO_VA fb7ff000 -> 10c7ff000
::vtop BDA_VA        fb7ea000 -> 10c7ea000      (NOT physical page 0)
gdt0 = fffffffffb7fe000

fb600f50: 0000000000000403               <- BDA mapping (pa 0) landed here as garbage
fb600f60: 000000010ff7e503 000000010ff805 03 ... (16 LDT pages, GDT, IDT, TSSes)
fb600ff8: 000000011ec02503               <- the debug_info page (the one #6 found exposed)

So the 22 MISC mappings produce 22 wasted physical pages (88 KiB), 176 bytes of
garbage in the leaf's first page (unused RAM today), and the descriptor tables
"work" only because writer and CPU both resolve those VAs through the leaf.

One mapping is actually broken: BDA_VA is meant to keep the BIOS data area
(physical page 0) reachable after the direct map is dropped (kbm_map_persistent(),
kb_bda in boot_keyboard.c). It resolves to the leaf's RAM instead. Invisible
under OVMF because the legacy BDA is all zeros there anyway (0x400,20::dump -p
confirms), but it is a wrong mapping, not a harmless one.

krtld is affected too, and masked. lg_pagesize is 4 MiB, so
kobj_segbrk()'s BOP_ALLOC requests miss do_bsys_alloc()'s 2 MiB leaf path
(a == pgsize compares against TWO_MEG) and are mapped 4 KiB at a time --
through the pre-mapped nucleus/heaptext leaves. The stray writes fill the target
leaf's first page with PTEs, and then the bzero() restored in #5 wipes them
before krtld copies anything in. Net effect: no corruption (measured: the nucleus
first page holds intact code), but every such allocation is double memory -- the
pre-mapped leaf RAM that the VA still resolves to, plus the pages do_bsys_alloc
handed out and nothing can reach. From the bop-early totals, the kernel-class
allocations beyond the known pre-maps are ~7 MiB, so the waste is a few MiB plus
the 88 KiB above. Small, but it is memory the kernel believes it owns twice.

Why "cannot be fixed while MISC relies on it" is only true of one fix

A find_pte() that refuses a leaf would break boot: the descriptor tables are
placed at fixed VAs inside the MISC leaf by design. But the correct fix is not to
refuse. It is to demote: when the walk meets a PT_PAGESIZE entry above the
requested level and probe_only is not set, allocate a table (make_ptable(),
already used by find_pte() for missing tables), fill its entries to reproduce
the leaf's translation (base + i * subpage, flags copied, PT_PAGESIZE kept
only if the sub-level is still large, the large-page PAT bit moved to bit 7 for
4 KiB entries), swap the leaf for the table pointer, and continue. The caller then
overwrites exactly the one 4 KiB entry it asked for. In probe_only mode return
NULL, which is already what every probe caller handles (kbm_unmap returns,
kbm_probe handles leaves itself).

Consequences: gdt0 and the other tables move to the pages that were allocated
for them; BDA_VA resolves to physical 0; no more stray writes; the
kbm_map_kernel_pts() prohibition and the "no new VA window may overlap a 2 MiB
leaf" rule both go away. It does not by itself recover the double allocation
-- that needs kobj_segbrk() (or do_bsys_alloc()) to stop allocating for VAs
that are already mapped, which is a separate, smaller change.

Cost and risk

~50 lines in find_pte(), one file compiled into unix (25 s rebuild). Risk is
moderate: it changes the boot page-table walker, but only on the collision path,
which today fires a known, countable number of times. Gates for a fix: a marker
counting demotions per boot (expect the MISC leaf plus a handful of nucleus
leaves, and zero on the 2 GiB/4 GiB layouts if they differ), ::vtop GDT_VA on a
page that is not leaf + offset, ::vtop BDA_VA == physical 0, the leaf's
first page free of PTE-shaped qwords, and the full boot matrix.

Recommendation

Fix it, by demotion, as described. The constraint it imposes is exactly the kind
that gets re-derived (this issue is already the third time), the BDA mapping is
wrong today, and the walker will be needed to handle leaves the moment anyone
moves BOP scratch (design §9) or re-enables kbm_map_kernel_pts().

Chris Tusa commented 6 days ago

External review of the evaluation above (grok, 2026-09-02), checked against the tree

Full text: /tmp/grok-i7-review.md (549 lines, every claim cited file:line).
Each point below was re-verified in the source before being recorded here.

Confirmed: the mechanism (walker treats a PT_PAGESIZE leaf as a table;
stores land at leaf + ((va>>12)&511)*8), the BDA_VA mapping being wrong, the
krtld path (lg_pagesize is 4 MiB via bi_kseg_size, do_bsys_alloc() only
takes the large-page path at exactly TWO_MEG, so kobj_segbrk() maps 4 KiB
through the pre-mapped grow leaves and double-allocates).

Corrections to the evaluation:

  • 21 stray stores, 20 wasted pages (80 KiB): DFTSS_VA is reserved but never
    BOP_ALLOCed (the dump's 0xf58 slot is indeed zero), and the BDA store
    allocates nothing.
  • "Every probe caller handles NULL" is false: kbm_probe() and kbm_remap()
    panic on NULL. Returning NULL from probe_only on a leaf is acceptable only
    because kbm_probe() stops at leaves itself (PTE_ISPAGE) and kbm_remap()
    is only used on 4 KiB-mapped text.
  • The "nucleus first page holds intact code" dump does not demonstrate that
    bzero() wipes krtld's stray PTEs: that page is used from the pre-map without
    a BOP_ALLOC (the first kobj_segbrk() allocation rounds up to 0x...fc000000).
    The hiding argument remains valid for the 4 MiB-aligned chunks; the cited
    measurement was the wrong page.

Defects in the demotion sketch (all real):

  1. PT_PADDR_LGPG(l) does not exist; the constant excludes bit 12 but has no
    level. A wrong mask steals PT_PAT_LARGE and mis-bases a PAT-typed leaf.
  2. LEVEL_SIZE() is mmu.level_size[], filled by mmu_init() in
    startup_memlist(); the first collision is init_desctbls() in mlsetup().
    Must use 1 << shift_amt[l-1] (set in kbm_init()).
  3. No TLB flush at the swap. set_pteval() only reloads CR3 for 32-bit PAE; the
    MISC leaf is PT_GLOBAL (is_kernel maps), which reload_cr3() does not
    flush anyway. invlpg the large page inside find_pte() after installing
    the table.
  4. PTP bits must come from make_ptable() (VALID|REF|USER|WRITABLE), never
    copied from the leaf: htable_attach()/unlink_ptp() panic Bad PTP on
    anything but MAKEPTP.
  5. Heaptext (128 MiB below KERNEL_TEXT) is outside hat_kern_alloc()'s
    KERNEL_TEXT-upward reserve walk and outside the FB/BDA reserve; a demotion
    there would need an L0 htable nobody reserved. Latent, not hit today.
  6. If kbm_map_persistent() ever moves before boot_mapin(misc_2m, …), a
    correct BDA mapping to pfn 0 makes boot_mapin() panic (avail_filter()
    drops page zero, so it has no page_t).

Gates that would pass on a no-op: "zero demotions on 2/4 GiB" (MISC and the
grow window are fixed VAs; RAM size does not change whether a 4 KiB map hits a
leaf), a bare demotion counter, and ::vtop alone (walks tables, not the TLB).
Required instead, as serial markers on every arm: hh: pte-demote n= with
n >= 1; hh: bda pfn=0 printed after kbm_map_persistent(); hh: gdt pfn=
not equal to the MISC leaf pfn + 0x1fe; plus the existing accounting markers.

Alternatives the reviewer put first:

  • A. Stop pre-mapping MISC as a 2 MiB leaf (drop the do_bsys_alloc(misc_2m, TWO_MEG, TWO_MEG) in limine_fixup.c, or map it 4 KiB). Then the descriptor
    tables and BDA take the ordinary "missing table" path with no walker change.
    The fixup comment says uncovering MISC once broke boot; that is a claim to
    re-test (one build, one boot), not a constraint.
  • B. do_bsys_alloc() skips already-mapped VAs (via kbm_probe), or
    lg_pagesize becomes TWO_MEG so krtld replaces leaves instead of walking
    into them. Recovers the double allocation; does not fix BDA.
  • C. Demotion, corrected per 1-6 above, if a walker that is safe in general is
    wanted (design §9 scratch move, any future overlapping window). Keep
    kbm_map_kernel_pts() uncalled regardless: KPM already covers those PAs.

Revised recommendation: A first (cheapest; fixes BDA and the wasted
descriptor pages; tests the premise the whole constraint rests on), B second
for the few MiB of double allocation, C only if the general walker is wanted,
built from the corrected design and gated as above.

Chris Tusa commented 6 days ago

Option 1 re-tested: unmapped MISC boots (rev 364, fb4df29a05c4)

The pre-map do_bsys_alloc(NULL, misc_2m, TWO_MEG, TWO_MEG) in limine_fixup.c
is gone, and so is the now-dead boot_mapin() of the leaf's lower part in
startup_memlist(). startup_vm() prints the properties after
hat_kern_alloc() (i.e. after kbm_map_persistent()), so the matrix gates on
them instead of on "it came up":

hh: misc-leaf va=fffffffffb600000 pfn=ffffffffffffffff     <- no 2MiB leaf
hh: gdt pfn=10fb80 idt pfn=10fb7d dbg pfn=11e802 bda pfn=0  <- BOP pages; BDA = page 0

::vtop agrees on all five VAs (GDT_VA -> 10fb80000, IDT_VA -> 10fb7d000,
DEBUG_INFO_VA -> 11e802000, BDA_VA -> 0, 0xfffffffffb600000 -> no mapping).
17 daemons / 0 failed, DTrace and mdb -k fine, bop-verify claimed=0. The CPU
has taken every interrupt through the new IDT/GDT translations since
init_desctbls(), which covers the TLB concern for this path.

The old "uncovering MISC broke boot" comment was wrong; whatever broke that
boot was the window move it accompanied, not the missing leaf.

Side effect: BOP pins 4 MiB less before page_ts exist (bop-early kernel
total 0x1b754000 -> 0x1b554000, watermark 0x11ec02 -> 0x11e802): the 2 MiB
leaf plus the 2 MiB alignment gap its TWO_MEG-aligned allocation forced into
the ascending watermark, which the interval claim used to swallow. One extra
page table (+0x1000 in ptable) for the MISC 4 KiB entries.

Matrix (every boot gated on hh: misc-leaf ... pfn=ffffffffffffffff,
hh: gdt pfn=... bda pfn=0, no unmapped descriptor page, plus the existing
bop-verify claimed=0 / bop-early held=0 / adapter held=0 / kernel-span /
#9-property gates): 8 GiB 2 vCPU 4/4, 4 vCPU 6/6; 4 GiB 4 vCPU 4/4. No BUG,
no panic. At 4 GiB the descriptor pages land elsewhere (gdt pfn=11381), BDA is
still pfn 0. Docs: rev 365, LIMINE_ARCHITECTURE.md §5.6.

What this does and does not close

Closed: the wrong BDA_VA mapping, the 20 unreachable descriptor pages, the
stray stores in the MISC leaf, and the one live dependency on the walker
misbehaving.

Still open, per the reviewed evaluation above:

  • find_pte() itself is unchanged. Any 4 KiB map into an existing 2 MiB leaf
    still stores into the leaf's RAM. The constraint "no new VA window may overlap
    a 2 MiB leaf" stands until demotion (option 3) is implemented from the
    corrected design (real large-page address mask, shift_amt not
    LEVEL_SIZE, invlpg at the swap, PTP bits from make_ptable(), heaptext
    reserve, kbm_remap/kbm_probe NULL semantics).
  • krtld's kobj_segbrk() still allocates through the pre-mapped nucleus
    leaves (lg_pagesize 4 MiB vs do_bsys_alloc()'s exact-2 MiB test) and
    double-allocates those pages; bzero() hides the stray stores. Option 2.

Leaving #7 open for those two.

Assignees

No assignees

Labels

No labels

Severity

Medium