[PATCH] riscv: cpuinfo: Fix NULL dereference and inconsistent error code

[PATCH] riscv: cpuinfo: Fix NULL dereference and inconsistent error code

[Mohamed Ayman]

Fix a potential NULL pointer dereference in c_show() by ensuring
the device tree node returned by of_get_cpu_node() is checked before use.

Also replace a non-standard return value (-1) in
riscv_of_parent_hartid() with -ENODEV to follow Linux kernel
conventions for "not found" error returns.

Both issues improve robustness and consistency with existing RISC-V CPU info handling code.

v2: Add missing Signed-off-by

Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
---
 arch/riscv/kernel/cpu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 3dbc8cc557dd..e0bc7526861f 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -135,7 +135,7 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
 		}
 	}
 
-	return -1;
+	return -ENODEV;
 }
 
 unsigned long __init riscv_get_marchid(void)
@@ -348,12 +348,13 @@ static int c_show(struct seq_file *m, void *v)
 
 	if (acpi_disabled) {
 		node = of_get_cpu_node(cpu_id, NULL);
+		if (node) {
+			if (!of_property_read_string(node, "compatible", &compat) &&
+				strcmp(compat, "riscv"))
+				seq_printf(m, "uarch\t\t: %s\n", compat);
 
-		if (!of_property_read_string(node, "compatible", &compat) &&
-		    strcmp(compat, "riscv"))
-			seq_printf(m, "uarch\t\t: %s\n", compat);
-
-		of_node_put(node);
+			of_node_put(node);
+		}
 	}
 
 	seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
-- 
2.34.1

Re: [PATCH] riscv: cpuinfo: Fix NULL dereference and inconsistent error code

[Michael Ellerman]

On 3/5/2026 02:17, Mohamed Ayman wrote:
> Fix a potential NULL pointer dereference in c_show() by ensuring
> the device tree node returned by of_get_cpu_node() is checked before use.

Where is the NULL pointer deref?

of_property_read_string() can cope with a NULL np, it just returns an error.

of_node_put() also handles a NULL np.

> Also replace a non-standard return value (-1) in
> riscv_of_parent_hartid() with -ENODEV to follow Linux kernel
> conventions for "not found" error returns.

That should be a separate patch. It does look like a good change. At 
least some of the callers of riscv_of_parent_hartid() pass the return 
value up to their callers, so -1 could be misinterpreted somewhere as EPERM.

cheers

Re: [PATCH] riscv: cpuinfo: Fix NULL dereference and inconsistent error code

[MOHAMED AYMAN]

Thanks for the review Michael.

You're right, both of_property_read_string() and of_node_put() handle
NULL np internally, so the c_show() hunk is unnecessary. I'll drop it
in v3.

I'll also send the -ENODEV fix as a standalone patch as you suggested. :)

On Sun, May 3, 2026 at 3:26 PM Michael Ellerman <mpe@kernel.org> wrote:
>
> On 3/5/2026 02:17, Mohamed Ayman wrote:
> > Fix a potential NULL pointer dereference in c_show() by ensuring
> > the device tree node returned by of_get_cpu_node() is checked before use.
>
> Where is the NULL pointer deref?
>
> of_property_read_string() can cope with a NULL np, it just returns an error.
>
> of_node_put() also handles a NULL np.
>
> > Also replace a non-standard return value (-1) in
> > riscv_of_parent_hartid() with -ENODEV to follow Linux kernel
> > conventions for "not found" error returns.
>
> That should be a separate patch. It does look like a good change. At
> least some of the callers of riscv_of_parent_hartid() pass the return
> value up to their callers, so -1 could be misinterpreted somewhere as EPERM.
>
> cheers
>

[PATCH v3] riscv: Replace non-standard -1 return with -ENODEV in riscv_of_parent_hartid()

[Mohamed Ayman]

riscv_of_parent_hartid() returns -1 when no valid hart ID is found among
the parent nodes. This is non-standard; Linux kernel convention for a
"not found" condition is -ENODEV. Additionally, since several callers
propagate this return value up the call chain, a raw -1 could be
misinterpreted as -EPERM by code inspecting errno values.

Replace the -1 with -ENODEV for clarity and consistency with the rest
of the RISC-V CPU handling code.

Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
---
 arch/riscv/kernel/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 3dbc8cc55..414cd55df 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -135,7 +135,7 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
 		}
 	}
 
-	return -1;
+	return -ENODEV;
 }
 
 unsigned long __init riscv_get_marchid(void)
-- 
2.34.1

[PATCH] riscv: cpuinfo: Fix NULL dereference and inconsistent error code

[Mohamed Ayman]

Fix a potential NULL pointer dereference in c_show() by ensuring
the device tree node returned by of_get_cpu_node() is checked before use.

Also replace a non-standard return value (-1) in
riscv_of_parent_hartid() with -ENODEV to follow Linux kernel
conventions for "not found" error returns.

Both issues improve robustness and consistency with existing RISC-V CPU info handling code.
---
 arch/riscv/kernel/cpu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 3dbc8cc557dd..e0bc7526861f 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -135,7 +135,7 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
 		}
 	}
 
-	return -1;
+	return -ENODEV;
 }
 
 unsigned long __init riscv_get_marchid(void)
@@ -348,12 +348,13 @@ static int c_show(struct seq_file *m, void *v)
 
 	if (acpi_disabled) {
 		node = of_get_cpu_node(cpu_id, NULL);
+		if (node) {
+			if (!of_property_read_string(node, "compatible", &compat) &&
+				strcmp(compat, "riscv"))
+				seq_printf(m, "uarch\t\t: %s\n", compat);
 
-		if (!of_property_read_string(node, "compatible", &compat) &&
-		    strcmp(compat, "riscv"))
-			seq_printf(m, "uarch\t\t: %s\n", compat);
-
-		of_node_put(node);
+			of_node_put(node);
+		}
 	}
 
 	seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
-- 
2.34.1

Re: [PATCH] riscv: cpuinfo: Fix NULL dereference and inconsistent error code

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1820 bytes --]

Hey,

On Sat, May 02, 2026 at 07:02:57PM +0300, Mohamed Ayman wrote:
> Fix a potential NULL pointer dereference in c_show() by ensuring
> the device tree node returned by of_get_cpu_node() is checked before use.
> 
> Also replace a non-standard return value (-1) in
> riscv_of_parent_hartid() with -ENODEV to follow Linux kernel
> conventions for "not found" error returns.
> 
> Both issues improve robustness and consistency with existing RISC-V CPU info handling code.


These two things should be two different patches, since they're
completely unrelated.

Additionally, the patch is missing a signoff.

> ---
>  arch/riscv/kernel/cpu.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 3dbc8cc557dd..e0bc7526861f 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -135,7 +135,7 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
>  		}
>  	}
>  
> -	return -1;
> +	return -ENODEV;
>  }
>  
>  unsigned long __init riscv_get_marchid(void)
> @@ -348,12 +348,13 @@ static int c_show(struct seq_file *m, void *v)
>  
>  	if (acpi_disabled) {
>  		node = of_get_cpu_node(cpu_id, NULL);
> +		if (node) {

Is it actually possible for this to fail?

Cheers,
Conor.

> +			if (!of_property_read_string(node, "compatible", &compat) &&
> +				strcmp(compat, "riscv"))
> +				seq_printf(m, "uarch\t\t: %s\n", compat);
>  
> -		if (!of_property_read_string(node, "compatible", &compat) &&
> -		    strcmp(compat, "riscv"))
> -			seq_printf(m, "uarch\t\t: %s\n", compat);
> -
> -		of_node_put(node);
> +			of_node_put(node);
> +		}
>  	}
>  
>  	seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]