[PATCH 2/2] x86/cacheinfo: Match sibling leaves by level and type, not by index

[Yunseong Kim]

The previous patch stops the out-of-bounds write, but it leaves the
assumption that caused it in place: that leaf index N describes the same
cache on every CPU the APIC-ID tests select as a sibling. Bounding the
index only handles the case where the sibling's array is too short. When
two CPUs enumerate different leaves but the index happens to be in range,
the code still cross-links whichever leaf sits at that index, so a CPU can
be recorded as sharing a cache of a level and type it does not have there.

The generic implementation stopped doing this in
commit 198102c9103f ("cacheinfo: Fix shared_cpu_map to handle shared
caches at different levels"): cache_shared_cpu_map_setup() walks the
sibling's own leaves and matches on level and type. x86 keeps a parallel
implementation that was not updated.

Do the same here. sibling_cache_leaf() looks the sibling's leaf up by
level and type over that CPU's own num_leaves, which cannot leave the
array, and returns NULL when the sibling has no such cache - so the
explicit bounds checks are no longer needed and are folded into it. All
three sibling-indexing sites use it, including both branches of
__cache_amd_cpumap_setup().

It also removes a smaller hazard: allocate_cache_info() kzalloc()s
info_list before populate_cache_leaves() fills it, so a sibling can have a
zeroed array. Indexing it by number wrote into a leaf describing no cache;
matching by level and type skips it, since a zeroed leaf has level 0 and
type CACHE_TYPE_NOCACHE and can never match a real one.

No functional change on machines whose CPUs enumerate identical leaves:
there the level-and-type match resolves to the same leaf the index did.

What this cannot fix is CPUID data that is inconsistent in the first
place. Two CPUs presented as SMT siblings while reporting different cache
geometry still derive the same cache id (apicid >> index_msb), so they are
still linked - by this code and by the generic matcher alike. That is
wrong topology from wrong input; the point of this patch is that it is no
longer memory-unsafe, and that a sibling's array is never addressed with
another CPU's index.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Yunseong Kim <yunseong.kim@xxxxxxxx>
---
 arch/x86/kernel/cpu/cacheinfo.c | 82 +++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 35 deletions(-)

diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 3a1c10699646..1024a6697d0e 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -482,13 +482,46 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	intel_cacheinfo_0x2(c);
 }
 
+/*
+ * Find the leaf of @cpu that describes the same cache level and type as
+ * @this_leaf, or NULL if it has none.
+ *
+ * The number of cache leaves is per-CPU since commit 9677be09e5e4
+ * ("x86/cacheinfo: Delete global num_cache_leaves"), so CPUs that the APIC-ID
+ * tests below treat as cache siblings may enumerate different leaves - on
+ * hybrid parts, or under a VMM that does not normalise CPUID leaf 4 across
+ * vCPUs. A leaf index is therefore only meaningful on the CPU it came from:
+ * index N need not describe the same cache on a sibling, and need not exist
+ * there at all. Match on level and type instead, the way
+ * cache_shared_cpu_map_setup() does in drivers/base/cacheinfo.c.
+ */
+static struct cacheinfo *sibling_cache_leaf(unsigned int cpu,
+					    const struct cacheinfo *this_leaf)
+{
+	struct cpu_cacheinfo *sib_cpu_ci = get_cpu_cacheinfo(cpu);
+	unsigned int i;
+
+	if (!sib_cpu_ci->info_list)
+		return NULL;
+
+	for (i = 0; i < sib_cpu_ci->num_leaves; i++) {
+		struct cacheinfo *sibling_ci = sib_cpu_ci->info_list + i;
+
+		if (sibling_ci->level == this_leaf->level &&
+		    sibling_ci->type == this_leaf->type)
+			return sibling_ci;
+	}
+
+	return NULL;
+}
+
 /*
  * <linux/cacheinfo.h> shared_cpu_map setup, AMD/Hygon
  */
 static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
-				    const struct _cpuid4_info *id4)
+				    const struct _cpuid4_info *id4,
+				    const struct cacheinfo *this_leaf)
 {
-	struct cpu_cacheinfo *this_cpu_ci;
 	struct cacheinfo *ci;
 	int i, sibling;
 
@@ -498,19 +531,10 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
 	 */
 	if (index == 3) {
 		for_each_cpu(i, cpu_llc_shared_mask(cpu)) {
-			this_cpu_ci = get_cpu_cacheinfo(i);
-			if (!this_cpu_ci->info_list)
-				continue;
-
-			/*
-			 * The leaf count is per-CPU, so a CPU sharing the LLC
-			 * may have enumerated fewer leaves than this one.
-			 * Never index past the end of its array.
-			 */
-			if (index >= this_cpu_ci->num_leaves)
+			ci = sibling_cache_leaf(i, this_leaf);
+			if (!ci)
 				continue;
 
-			ci = this_cpu_ci->info_list + index;
 			for_each_cpu(sibling, cpu_llc_shared_mask(cpu)) {
 				if (!cpu_online(sibling))
 					continue;
@@ -526,20 +550,14 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
 		last = first + nshared - 1;
 
 		for_each_online_cpu(i) {
-			this_cpu_ci = get_cpu_cacheinfo(i);
-			if (!this_cpu_ci->info_list)
-				continue;
-
 			apicid = cpu_data(i).topo.apicid;
 			if ((apicid < first) || (apicid > last))
 				continue;
 
-			/* Same per-CPU leaf count caveat as above. */
-			if (index >= this_cpu_ci->num_leaves)
+			ci = sibling_cache_leaf(i, this_leaf);
+			if (!ci)
 				continue;
 
-			ci = this_cpu_ci->info_list + index;
-
 			for_each_online_cpu(sibling) {
 				apicid = cpu_data(sibling).topo.apicid;
 				if ((apicid < first) || (apicid > last))
@@ -561,16 +579,16 @@ static void __cache_cpumap_setup(unsigned int cpu, int index,
 {
 	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
-	struct cacheinfo *ci, *sibling_ci;
+	struct cacheinfo *ci = this_cpu_ci->info_list + index;
+	struct cacheinfo *sibling_ci;
 	unsigned long num_threads_sharing;
 	int index_msb, i;
 
 	if (c->x86_vendor == X86_VENDOR_AMD || c->x86_vendor == X86_VENDOR_HYGON) {
-		if (__cache_amd_cpumap_setup(cpu, index, id4))
+		if (__cache_amd_cpumap_setup(cpu, index, id4, ci))
 			return;
 	}
 
-	ci = this_cpu_ci->info_list + index;
 	num_threads_sharing = 1 + id4->eax.split.num_threads_sharing;
 
 	cpumask_set_cpu(cpu, &ci->shared_cpu_map);
@@ -581,23 +599,17 @@ static void __cache_cpumap_setup(unsigned int cpu, int index,
 
 	for_each_online_cpu(i)
 		if (cpu_data(i).topo.apicid >> index_msb == c->topo.apicid >> index_msb) {
-			struct cpu_cacheinfo *sib_cpu_ci = get_cpu_cacheinfo(i);
-
-			/* Skip if itself or no cacheinfo */
-			if (i == cpu || !sib_cpu_ci->info_list)
+			if (i == cpu)
 				continue;
 
 			/*
-			 * CPUs that the APIC-ID test treats as cache siblings
-			 * may still enumerate a different number of leaves,
-			 * e.g. on hybrid parts or under a VMM that does not
-			 * normalise CPUID leaf 4 across vCPUs. Never index
-			 * past the end of the sibling's array.
+			 * Skip siblings without cacheinfo yet, and those that
+			 * have no cache of this level and type.
 			 */
-			if (index >= sib_cpu_ci->num_leaves)
+			sibling_ci = sibling_cache_leaf(i, ci);
+			if (!sibling_ci)
 				continue;
 
-			sibling_ci = sib_cpu_ci->info_list + index;
 			cpumask_set_cpu(i, &ci->shared_cpu_map);
 			cpumask_set_cpu(cpu, &sibling_ci->shared_cpu_map);
 		}

-- 
2.47.3