bug(aarch64): UB negative shift + wrong EntryCount for level-(-1) root table in CheckPageAccess (5-level paging / T0SZ < 16)

[Dhakshin2007]

Summary

Commit aee730d (PR #158 — "Fix aarch64 page table parsing for 5 level paging") introduced two related bugs in CheckPageAccess() inside DebuggerFeaturePkg/Library/DebugAgent/AARCH64/DebugAarch64.c.

---

Affected File

DebuggerFeaturePkg/Library/DebugAgent/AARCH64/DebugAarch64.c — CheckPageAccess(), lines 408–409

---

Bug 1 — C Undefined Behavior: right-shift by negative amount

Code (current)

TableLevel = (T0SZ < MIN_T0SZ) ? -1 : (INTN)(T0SZ - MIN_T0SZ) / BITS_PER_LEVEL;
EntryCount = TT_ENTRY_COUNT >> (INTN)(T0SZ - MIN_T0SZ) % BITS_PER_LEVEL;

When T0SZ < MIN_T0SZ (5-level paging, e.g. T0SZ=12):

1. T0SZ and MIN_T0SZ are both UINTN. T0SZ - MIN_T0SZ wraps to a large unsigned value (e.g. 0xFFFFFFFFFFFFFFFC).
2. (INTN)(0xFFFFFFFFFFFFFFFC) = -4.
3. (-4) % BITS_PER_LEVEL = -4 (C99 truncation toward zero).
4. TT_ENTRY_COUNT >> -4 — right-shifting by a negative amount is undefined behavior in C (ISO/IEC 9899:2011 §6.5.7p3).

---

Bug 2 — Wrong EntryCount for the level-(-1) root table

Even after patching the UB, the formula TT_ENTRY_COUNT >> shift is architecturally incorrect for the level-(-1) root table. The ARM Architecture Reference Manual (D8.1.8) specifies that for 5-level paging with T0SZ=12 (52-bit VA), the root table has a 4-bit index (bits [51:48]) = 16 entries. The shift-based formula cannot correctly derive this.

Passing the wrong EntryCount to TT_LAST_BLOCK_ADDRESS() causes pointer underflow, and ParsePageTableLevel() then walks garbage page table entries — causing CheckPageAccess() / IsPageReadable() / IsPageWritable() to silently return wrong results on any system with T0SZ < 16.

---

Proposed Fix

Rework TableLevel/EntryCount into an explicit if/else that correctly handles each case:

if (T0SZ < MIN_T0SZ) {
  // 5-level paging (52-bit VA): root table is level -1 with
  // (MIN_T0SZ - T0SZ) index bits, giving 2^(MIN_T0SZ-T0SZ) entries.
  TableLevel = -1;
  EntryCount = (UINTN)1 << (MIN_T0SZ - T0SZ);
} else {
  TableLevel = (INTN)(T0SZ - MIN_T0SZ) / BITS_PER_LEVEL;
  EntryCount = TT_ENTRY_COUNT >> ((T0SZ - MIN_T0SZ) % BITS_PER_LEVEL);
}

Verification

T0SZ	Level	EntryCount	ARM ARM D8.1.8 Expected
12	-1 (5-level)	1 << 4 = 16	16 (bits [51:48])
16	0	512 >> 0 = 512	512
17	0	512 >> 1 = 256	256
25	1	512 >> 0 = 512	512

All arithmetic is on UINTN with no signed shifts and no negative values.

---

Impact

• Severity: Silent wrong-result bug on AARCH64 systems with 5-level paging (T0SZ < 16, 52-bit VA)
• Effect: IsPageReadable() / IsPageWritable() return incorrect values, potentially allowing or blocking debugger memory access based on garbage page table attributes
• Introduced by: PR Fix aarch64 page table parsing for 5 level paging #158 / commit aee730d

---

References

• PR Fix aarch64 page table parsing for 5 level paging #158 (introducing commit): Fix aarch64 page table parsing for 5 level paging #158
• Closed PR fix(aarch64): fix undefined-behavior negative shift in EntryCount for 5-level paging #161 (attempted fix): fix(aarch64): fix undefined-behavior negative shift in EntryCount for 5-level paging #161
• ARM Architecture Reference Manual D8.1.8 — Translation table walk, 5-level paging
• ISO/IEC 9899:2011 §6.5.7p3 — Undefined behavior for negative shift amounts

[mu-automation]

This issue has been automatically marked as stale because it has not had activity in 45 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.