[PATCH] riscv: hwprobe: Avoid uninitialized read in hwprobe_get_cpus()

[Mark Harris]

Michael Ellerman wrote:
>
> On 12/6/26 2:55 pm, Mark Harris wrote:
> > When cpusetsize < cpumask_size(), hwprobe_get_cpus() did not fully
> > initialize its copy of the cpu mask, which could cause non-deterministic
> > results from the riscv_hwprobe syscall on a system with more than 8 CPUs
> > when the supplied cpu mask is empty.  Address this by fully initializing
> > the cpu mask.
> >
> > Signed-off-by: Mark Harris <mark.hsj at gmail.com>
> > ---
> >   arch/riscv/kernel/sys_hwprobe.c | 1 +
> >   1 file changed, 1 insertion(+)
>
> This should have a fixes tag, I think it's:
>
> Fixes: e178bf146e4b ("RISC-V: hwprobe: Introduce which-cpus flag")

Yes, that looks correct.

>
> > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > index 1659d31fd288..caf6762427c8 100644
> > --- a/arch/riscv/kernel/sys_hwprobe.c
> > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > @@ -450,6 +450,7 @@ static int hwprobe_get_cpus(struct riscv_hwprobe __user *pairs,
> >       if (cpusetsize > cpumask_size())
> >               cpusetsize = cpumask_size();
> >
> > +     cpumask_clear(&cpus);
> >       ret = copy_from_user(&cpus, cpus_user, cpusetsize);
> >       if (ret)
> >               return -EFAULT;
>
> cpus is on the stack, and is copied back out at the end of the function,
> so this looks like it could be a stack info leak.
>
> But the copy back is also bounded by cpusetsize, so in fact there is not
> any leak of uninitialised stack out to userspace:
>
>          ret = copy_to_user(cpus_user, &cpus, cpusetsize);
>          if (ret)
>                  return -EFAULT;

It can leak 1 bit of information from the kernel stack.  For example
with 16 CPUs, all online, if a 1-byte 0x00 mask is supplied, the
caller can determine whether the second (uninitialized) mask byte is
zero or non-zero due to the cpumask_empty(&cpus) check.

 - Mark

>
> Reviewed-by: Michael Ellerman <mpe at kernel.org>
>
> cheers