2021-04-15 08:58:58

by Anup Patel

Subject: [PATCH] RISC-V: Fix error code returned by riscv_hartid_to_cpuid()

We should return a negative error code upon failure in
riscv_hartid_to_cpuid() instead of NR_CPUS. This is also
aligned with all uses of riscv_hartid_to_cpuid() which
expect negative error code upon failure.

Fixes: 6825c7a80f18 ("RISC-V: Add logical CPU indexing
for RISC-V")
Cc: [email protected]
Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index ea028d9e0d24..d44567490d91 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -54,7 +54,7 @@ int riscv_hartid_to_cpuid(int hartid)
return i;

pr_err("Couldn't find cpu id for hartid [%d]\n", hartid);
- return i;
+ return -ENOENT;
}

void riscv_cpuid_to_hartid_mask(const struct cpumask *in, struct cpumask *out)
--
2.25.1

2021-04-23 07:16:42

by Palmer Dabbelt

Subject: Re: [PATCH] RISC-V: Fix error code returned by riscv_hartid_to_cpuid()

On Thu, 15 Apr 2021 01:55:22 PDT (-0700), Anup Patel wrote:
> We should return a negative error code upon failure in

Not just should: the PLIC driver assumed this was the return flavor and
will overflow a per_cpu because it doesn't detect the error. IIUC this
will manifest if we have more harts that NR_CPUS, which is a possible
configuration.

That said, it's gone unnoticed for ~3 years so I'm inclined to leave it
out of fixes because I have nothing else and this late in the cycle is
always a bit touchy. It's kind of splitting hairs as it'll just get
backported, but post-rc8 is an odd spot to be in.

Is this actually manifesting as a crash for you?

Sorry I missed this last week.

> riscv_hartid_to_cpuid() instead of NR_CPUS. This is also
> aligned with all uses of riscv_hartid_to_cpuid() which
> expect negative error code upon failure.
>
> Fixes: 6825c7a80f18 ("RISC-V: Add logical CPU indexing for RISC-V")

Also

Fixes: f99fb607fb2b ("RISC-V: Use Linux logical CPU number instead of hartid")

which is what actually manifests the bug -- again, kind of splitting
hairs as it's the patch after.

> Cc: [email protected]
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/kernel/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index ea028d9e0d24..d44567490d91 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -54,7 +54,7 @@ int riscv_hartid_to_cpuid(int hartid)
> return i;
>
> pr_err("Couldn't find cpu id for hartid [%d]\n", hartid);
> - return i;
> + return -ENOENT;
> }
>
> void riscv_cpuid_to_hartid_mask(const struct cpumask *in, struct cpumask *out)