[PATCH] kcpuid: Fix potential dereferencing of null pointers

[PATCH] kcpuid: Fix potential dereferencing of null pointers

[Remington Brasga]

Clang reported "clang-analyzer-core.NullDereference" on the `leaf` and `range`
variables in kcpuid.c, which makes sense if malloc/realloc fail.

These changes will ensure that the variables are not dereferenced while null.

Signed-off-by: Remington Brasga <rbrasga@uci.edu>
---
 tools/arch/x86/kcpuid/kcpuid.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tools/arch/x86/kcpuid/kcpuid.c b/tools/arch/x86/kcpuid/kcpuid.c
index 1b25c0a95d3f..c05226d105b6 100644
--- a/tools/arch/x86/kcpuid/kcpuid.c
+++ b/tools/arch/x86/kcpuid/kcpuid.c
@@ -144,19 +144,29 @@ static bool cpuid_store(struct cpuid_range *range, u32 f, int subleaf,
 
 	if (!func->leafs) {
 		func->leafs = malloc(sizeof(struct subleaf));
-		if (!func->leafs)
+		if (!func->leafs) {
 			perror("malloc func leaf");
+			return false; // On malloc failure
+		}
 
 		func->nr = 1;
 	} else {
 		s = func->nr;
 		func->leafs = realloc(func->leafs, (s + 1) * sizeof(*leaf));
-		if (!func->leafs)
+		if (!func->leafs) {
 			perror("realloc f->leafs");
+			return false; // On realloc failure
+		}
 
 		func->nr++;
 	}
 
+	// Check for valid index
+	if (s >= func->nr) {
+		fprintf(stderr, "Error: Invalid index for leaf\n");
+		return false;
+	}
+
 	leaf = &func->leafs[s];
 
 	leaf->index = f;
@@ -210,8 +220,10 @@ struct cpuid_range *setup_cpuid_range(u32 input_eax)
 	idx_func = (max_func & 0xffff) + 1;
 
 	range = malloc(sizeof(struct cpuid_range));
-	if (!range)
+	if (!range) {
 		perror("malloc range");
+		return NULL; // On malloc failure
+	}
 
 	if (input_eax & 0x80000000)
 		range->is_ext = true;
@@ -219,8 +231,11 @@ struct cpuid_range *setup_cpuid_range(u32 input_eax)
 		range->is_ext = false;
 
 	range->funcs = malloc(sizeof(struct cpuid_func) * idx_func);
-	if (!range->funcs)
+	if (!range->funcs) {
 		perror("malloc range->funcs");
+		free(range);
+		return NULL; // On malloc failure
+	}
 
 	range->nr = idx_func;
 	memset(range->funcs, 0, sizeof(struct cpuid_func) * idx_func);
-- 
2.34.1

Re: [PATCH] kcpuid: Fix potential dereferencing of null pointers

[Thomas Gleixner]

On Thu, Sep 26 2024 at 22:35, Remington Brasga wrote:
>  	if (!func->leafs) {
>  		func->leafs = malloc(sizeof(struct subleaf));
> -		if (!func->leafs)
> +		if (!func->leafs) {
>  			perror("malloc func leaf");
> +			return false; // On malloc failure

Please get rid of these horrible and pointless tail comments.

Returning false here does not make sense. This simply should terminate
the program.

> +		}
>  
>  		func->nr = 1;
>  	} else {
>  		s = func->nr;
>  		func->leafs = realloc(func->leafs, (s + 1) * sizeof(*leaf));
> -		if (!func->leafs)
> +		if (!func->leafs) {
>  			perror("realloc f->leafs");
> +			return false; // On realloc failure
> +		}
>  
>  		func->nr++;
>  	}
>  
> +	// Check for valid index
> +	if (s >= func->nr) {

What's the point of this? s is guaranteed to be < func->nr, no?

Thanks,

        tglx