x86: size CacheInfo::levels to hold all leaf 2 cache descriptors by EylonKrause · Pull Request #462 · google/cpu_features

EylonKrause · 2026-06-30T08:56:48Z

Description

ParseLeaf2 (legacy CPUID leaf-2 cache parsing in src/impl_x86__base_implementation.inl) appends a CacheLevelInfo to CacheInfo::levels — a fixed CPU_FEATURES_MAX_CACHE_LEVEL (10) array — for every non-zero descriptor byte, but the loop is bounded only by sizeof(data) == 16 and never checks info->size against the array capacity:

for (size_t i = 0; i < sizeof(data); ++i) {
    const uint8_t descriptor = data[i];
    if (descriptor == 0) continue;
    info->levels[info->size] = GetCacheLevelInfo(descriptor);  // OOB once size >= 10
    info->size++;
}

A CPUID leaf 2 advertising more than 10 non-zero descriptors — up to 15 are representable (16 bytes minus the AL count byte) — writes past levels[10], an out-of-bounds write into adjacent memory, reachable from public GetX86CacheInfo() on hardware/hypervisor-controlled CPUID input.

This is the same class of memory corruption fixed back in #190 for the leaf-4 / 0x8000001D path: the sibling ParseCacheInfo already guards its loop with info.size < CPU_FEATURES_MAX_CACHE_LEVEL. The legacy leaf-2 path (ParseLeaf2, added in #80) was left unbounded — this applies the same guard there.

Changes & Impact

src/impl_x86__base_implementation.inl: add && info->size < CPU_FEATURES_MAX_CACHE_LEVEL to the ParseLeaf2 loop condition, mirroring ParseCacheInfo. Behavior is byte-identical on real CPUs (which advertise far fewer than 10 leaf-2 descriptors); it only stops the overflow on pathological/hostile input.
test/cpuinfo_x86_test.cc: add Leaf2_TooManyDescriptors_DoesNotOverflow, a regression test feeding a leaf-2 with 15 non-zero descriptors.

Testing

Built cpuinfo_x86_test with -fsanitize=address,undefined (CMake + Ninja):

Before the fix: the new test triggers AddressSanitizer: stack-buffer-overflow WRITE ... in ParseLeaf2 src/impl_x86__base_implementation.inl:1876.
After the fix: the new test passes, and the full cpuinfo_x86_test suite is green (69/69) under ASan/UBSan.

gchatelet

Thx for the PR.

It might be simpler to just increase CPU_FEATURES_MAX_CACHE_LEVEL to 16 and call it a day.

gchatelet · 2026-06-30T09:45:55Z

@@ -1399,6 +1399,21 @@ flags           : fpu mmx sse sse2 pni ssse3 sse4_1 sse4_2
 }

 // https://www.felixcloutier.com/x86/cpuid#example-3-1--example-of-cache-and-tlb-interpretation


This line should stay next to its original test.

This doesn't seem to be addressed. Can you move this line so its stays close to the original test below?

gchatelet · 2026-06-30T09:51:05Z

+  });
+
+  const auto info = GetX86CacheInfo();
+  EXPECT_LE(info.size, CPU_FEATURES_MAX_CACHE_LEVEL);


We should test for equality since the purpose of this test is to saturate the array.

gchatelet · 2026-06-30T12:37:31Z

+// Regression test: a CPUID leaf 2 advertising more cache/TLB descriptors
+// than CPU_FEATURES_MAX_CACHE_LEVEL must not overflow CacheInfo::levels. AL
+// (the low byte of EAX) is the ignored count byte; the other 15 bytes here
+// are non-zero descriptors, exceeding the 10-entry levels array.


AL (the low byte of EAX) is the ignored count byte; the other 15 bytes here are non-zero descriptors, exceeding the 10-entry levels array.

This part of the comment is not very clear. It would be best to explain what the crafted value does.

Each byte is a descriptor so this is 16 descriptors total which is more than CPU_FEATURES_MAX_CACHE_LEVEL

The bit 31 must be set to zero for the leaf to be taken into account

The lower byte of EAX is the zero descriptor and so is not taken into account.

gchatelet · 2026-06-30T12:39:26Z

+  cpu().SetLeaves({
+      {{0x00000000, 0}, Leaf{0x00000002, 0x756E6547, 0x6C65746E, 0x49656E69}},
+      {{0x00000001, 0}, Leaf{0x00000F0A, 0x00010808, 0x00000000, 0x3FEBFBFF}},
+      {{0x00000002, 0}, Leaf{0x01010101, 0x01010101, 0x01010101, 0x01010101}},


Add a comment below to indicate that we're forging 16 descriptors (one per byte). AL being discarded. so 15 descriptors.

EylonKrause · 2026-07-05T14:12:44Z

Thanks @gchatelet — good call, that's cleaner. Done:

Raised CPU_FEATURES_MAX_CACHE_LEVEL from 10 to 16 and dropped the loop guard. Since leaf 2 reports at most 15 descriptors (the AL count byte is ignored), 16 is enough to hold them all — so this prevents the overflow and stops truncating cache levels the guard would have dropped.
Updated the regression test to feed 15 descriptors and assert all 15 are recorded (info.size == 15).

Built with -fsanitize=address,undefined: the leaf-2 test passes and the full x86 suite is 69/69 green.

gchatelet · 2026-07-06T07:29:56Z

Thx for the patch, ca you address the remaining comments?

ParseLeaf2 appends a CacheLevelInfo for every non-zero descriptor byte in CPUID leaf 2 (up to 15, since the AL count byte is ignored) into the fixed CacheInfo::levels[CPU_FEATURES_MAX_CACHE_LEVEL] array. Sized at 10, a CPU reporting more than 10 descriptors overflowed the array; this is reachable from the public GetX86CacheInfo(). Raise CPU_FEATURES_MAX_CACHE_LEVEL from 10 to 16 so the array can hold every descriptor a single leaf 2 can report. This both prevents the overflow and avoids silently dropping cache levels. Add a regression test that feeds 15 descriptors and checks they are all recorded without overflow.

EylonKrause · 2026-07-06T08:36:10Z

Thanks @gchatelet! Addressed the review:

Switched the assertion to EXPECT_EQ(info.size, 15) (the point is to saturate the array), dropping the redundant EXPECT_LE.
Added a comment on the leaf-2 line spelling out that leaf 2 encodes one descriptor per byte across its four registers (16 bytes), AL is the ignored count byte, and the all-0x01 registers therefore forge 15 non-zero descriptors — more than the old 10-entry array and exactly filling the new 16.

On the red clang-format check: it's flagging pre-existing violations throughout cpuinfo_x86_test.cc (e.g. lines 121, 1128, 1691 — far from this change), not the couple of lines I touched. The CI's clang-format (Alpine 22.1.8) seems to disagree with the file's current formatting, so any PR that touches this file trips it. The lines I added are ≤80 cols and clang-format-clean; I'm happy to send a separate whole-file clang-format -i commit if you'd like the file brought up to the newer linter, but didn't want to bury this fix under an unrelated reformat.

EylonKrause · 2026-07-07T17:28:25Z

Disclosure: this contribution was authored with an AI coding assistant (Claude) and reviewed before submission.

gchatelet

Sorry I forgot to send this comment.

gchatelet · 2026-07-06T07:27:11Z

+// Increase this value if more cache levels are needed. It must be large enough
+// to hold every descriptor a single CPUID leaf 2 can report (up to 15, since
+// the AL count byte is ignored) so that ParseLeaf2 cannot overflow
+// CacheInfo::levels.


The "Size must at least 15" part is specific to x86, note that this data structure is cpu agnostic and can be filled for other CPUs. Can you update the comment to say that for x86 it needs to be at least 15?

gchatelet reviewed Jun 30, 2026

View reviewed changes

EylonKrause changed the title ~~x86: bound ParseLeaf2 cache-descriptor loop by CPU_FEATURES_MAX_CACHE_LEVEL~~ x86: size CacheInfo::levels to hold all leaf 2 cache descriptors Jul 5, 2026

EylonKrause force-pushed the fix/x86-parseleaf2-cache-overflow branch from bab8339 to 49a3939 Compare July 5, 2026 14:12

EylonKrause force-pushed the fix/x86-parseleaf2-cache-overflow branch from 49a3939 to 871faf5 Compare July 6, 2026 08:36

gchatelet reviewed Jul 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

x86: size CacheInfo::levels to hold all leaf 2 cache descriptors#462

x86: size CacheInfo::levels to hold all leaf 2 cache descriptors#462
EylonKrause wants to merge 1 commit into
google:mainfrom
EylonKrause:fix/x86-parseleaf2-cache-overflow

EylonKrause commented Jun 30, 2026

Uh oh!

gchatelet left a comment

Uh oh!

gchatelet Jun 30, 2026

Uh oh!

gchatelet Jul 8, 2026

Uh oh!

gchatelet Jun 30, 2026

Uh oh!

gchatelet Jun 30, 2026

Uh oh!

gchatelet Jun 30, 2026

Uh oh!

EylonKrause commented Jul 5, 2026

Uh oh!

gchatelet commented Jul 6, 2026

Uh oh!

EylonKrause commented Jul 6, 2026

Uh oh!

EylonKrause commented Jul 7, 2026

Uh oh!

gchatelet left a comment

Uh oh!

gchatelet Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1399,6 +1399,21 @@ flags : fpu mmx sse sse2 pni ssse3 sse4_1 sse4_2
		}

		// https://www.felixcloutier.com/x86/cpuid#example-3-1--example-of-cache-and-tlb-interpretation

Uh oh!

Conversation

EylonKrause commented Jun 30, 2026

Description

Changes & Impact

Testing

Uh oh!

gchatelet left a comment

Choose a reason for hiding this comment

Uh oh!

gchatelet Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gchatelet Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

gchatelet Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gchatelet Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gchatelet Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

EylonKrause commented Jul 5, 2026

Uh oh!

gchatelet commented Jul 6, 2026

Uh oh!

EylonKrause commented Jul 6, 2026

Uh oh!

EylonKrause commented Jul 7, 2026

Uh oh!

gchatelet left a comment

Choose a reason for hiding this comment

Uh oh!

gchatelet Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants