x86: bound ParseLeaf2 cache-descriptor loop by CPU_FEATURES_MAX_CACHE_LEVEL#462
Open
EylonKrause wants to merge 1 commit into
Open
x86: bound ParseLeaf2 cache-descriptor loop by CPU_FEATURES_MAX_CACHE_LEVEL#462EylonKrause wants to merge 1 commit into
EylonKrause wants to merge 1 commit into
Conversation
…_LEVEL ParseLeaf2 appends a CacheLevelInfo to CacheInfo::levels (a fixed CPU_FEATURES_MAX_CACHE_LEVEL=10 array) for every non-zero leaf-2 descriptor byte, but the loop is bounded only by sizeof(data)==16 and never checks info->size against the array capacity. A CPUID leaf 2 advertising more than 10 non-zero descriptors (up to 15 are possible) writes past levels[10] -- an out-of-bounds write reachable from public GetX86CacheInfo() on hardware/hypervisor-controlled CPUID input. The sibling ParseCacheInfo already guards its loop with 'info.size < CPU_FEATURES_MAX_CACHE_LEVEL' (added for the same class of corruption in google#190); apply the same bound to ParseLeaf2. Adds a regression test (Leaf2_TooManyDescriptors_DoesNotOverflow) that feeds a leaf 2 with 15 descriptors: it triggers an ASan stack-buffer-overflow before the fix and passes after.
gchatelet
reviewed
Jun 30, 2026
gchatelet
left a comment
gchatelet
left a comment
Collaborator
There was a problem hiding this comment.
Thx for the PR.
It might be simpler to just increase CPU_FEATURES_MAX_CACHE_LEVEL to 16 and call it a day.
| @@ -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 | |||
Collaborator
There was a problem hiding this comment.
This line should stay next to its original test.
| }); | ||
|
|
||
| const auto info = GetX86CacheInfo(); | ||
| EXPECT_LE(info.size, CPU_FEATURES_MAX_CACHE_LEVEL); |
Collaborator
There was a problem hiding this comment.
We should test for equality since the purpose of this test is to saturate the array.
| // 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. |
Collaborator
There was a problem hiding this comment.
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.
| cpu().SetLeaves({ | ||
| {{0x00000000, 0}, Leaf{0x00000002, 0x756E6547, 0x6C65746E, 0x49656E69}}, | ||
| {{0x00000001, 0}, Leaf{0x00000F0A, 0x00010808, 0x00000000, 0x3FEBFBFF}}, | ||
| {{0x00000002, 0}, Leaf{0x01010101, 0x01010101, 0x01010101, 0x01010101}}, |
Collaborator
There was a problem hiding this comment.
Add a comment below to indicate that we're forging 16 descriptors (one per byte). AL being discarded. so 15 descriptors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
ParseLeaf2(legacy CPUID leaf-2 cache parsing insrc/impl_x86__base_implementation.inl) appends aCacheLevelInfotoCacheInfo::levels— a fixedCPU_FEATURES_MAX_CACHE_LEVEL(10) array — for every non-zero descriptor byte, but the loop is bounded only bysizeof(data) == 16and never checksinfo->sizeagainst the array capacity: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 publicGetX86CacheInfo()on hardware/hypervisor-controlled CPUID input.This is the same class of memory corruption fixed back in #190 for the leaf-4 /
0x8000001Dpath: the siblingParseCacheInfoalready guards its loop withinfo.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_LEVELto theParseLeaf2loop condition, mirroringParseCacheInfo. 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: addLeaf2_TooManyDescriptors_DoesNotOverflow, a regression test feeding a leaf-2 with 15 non-zero descriptors.Testing
Built
cpuinfo_x86_testwith-fsanitize=address,undefined(CMake + Ninja):AddressSanitizer: stack-buffer-overflow WRITE ... in ParseLeaf2 src/impl_x86__base_implementation.inl:1876.cpuinfo_x86_testsuite is green (69/69) under ASan/UBSan.