[v4,3/6] cacheinfo: Check 'cache-unified' property to count cache leaves

Commit Message

Pierre Gondois Jan. 4, 2023, 6:30 p.m. UTC

The DeviceTree Specification v0.3 specifies that the cache node
'[d-|i-|]cache-size' property is required. The 'cache-unified'
property is specifies whether the cache level is separate
or unified.

If the cache-size property is missing, no cache leaves is accounted.
This can lead to a 'BUG: KASAN: slab-out-of-bounds' [1] bug.

Check 'cache-unified' property and always account for at least
one cache leave when parsing the device tree.

[1] https://lore.kernel.org/all/[e-mail]/

Reported-by: Krzysztof Kozlowski <[e-mail]>
Signed-off-by: Pierre Gondois <[e-mail]>
---
 drivers/base/cacheinfo.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

Comments

Krzysztof Kozlowski Jan. 9, 2023, 4:13 p.m. UTC | #1

On 04/01/2023 19:30, Pierre Gondois wrote:
> The DeviceTree Specification v0.3 specifies that the cache node
> '[d-|i-|]cache-size' property is required. The 'cache-unified'
> property is specifies whether the cache level is separate
> or unified.
> 
> If the cache-size property is missing, no cache leaves is accounted.
> This can lead to a 'BUG: KASAN: slab-out-of-bounds' [1] bug.
> 
> Check 'cache-unified' property and always account for at least
> one cache leave when parsing the device tree.
> 
> [1] https://lore.kernel.org/all/[e-mail]/
> 
> Reported-by: Krzysztof Kozlowski <[e-mail]>
> Signed-off-by: Pierre Gondois <[e-mail]>

Thanks, solves the issue:

Tested-by: Krzysztof Kozlowski <[e-mail]>

Best regards,
Krzysztof

Patch

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 3e965ef6ee42..339cd1e3d5af 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -225,12 +225,9 @@  static int cache_setup_of_node(unsigned int cpu)
 	return 0;
 }
 
-int init_of_cache_level(unsigned int cpu)
+static int of_count_cache_leaves(struct device_node *np)
 {
-	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
-	struct device_node *np = of_cpu_device_node_get(cpu);
-	struct device_node *prev = NULL;
-	unsigned int levels = 0, leaves = 0, level;
+	unsigned int leaves = 0;
 
 	if (of_property_read_bool(np, "cache-size"))
 		++leaves;
@@ -238,6 +235,28 @@  int init_of_cache_level(unsigned int cpu)
 		++leaves;
 	if (of_property_read_bool(np, "d-cache-size"))
 		++leaves;
+
+	if (!leaves) {
+		/* The '[i-|d-|]cache-size' property is required, but
+		 * if absent, fallback on the 'cache-unified' property.
+		 */
+		if (of_property_read_bool(np, "cache-unified"))
+			return 1;
+		else
+			return 2;
+	}
+
+	return leaves;
+}
+
+int init_of_cache_level(unsigned int cpu)
+{
+	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+	struct device_node *np = of_cpu_device_node_get(cpu);
+	struct device_node *prev = NULL;
+	unsigned int levels = 0, leaves, level;
+
+	leaves = of_count_cache_leaves(np);
 	if (leaves > 0)
 		levels = 1;
 
@@ -251,12 +270,8 @@  int init_of_cache_level(unsigned int cpu)
 			goto err_out;
 		if (level <= levels)
 			goto err_out;
-		if (of_property_read_bool(np, "cache-size"))
-			++leaves;
-		if (of_property_read_bool(np, "i-cache-size"))
-			++leaves;
-		if (of_property_read_bool(np, "d-cache-size"))
-			++leaves;
+
+		leaves += of_count_cache_leaves(np);
 		levels = level;
 	}