| Message ID | 20240103121220.26506-1-palmer@rivosinc.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | RISC-V: hwprobe: Check for the reserved flags | expand |
Hi Palmer, On Wed, Jan 03, 2024 at 04:12:20AM -0800, Palmer Dabbelt wrote: > This check got dropped when RISCV_HWPROBE_WHICH_CPUS was added, but > we've still got a bunch of other reserved flags. So check for them, > just to make sure userspace doesn't get confused later. > > Fixes: e178bf146e4b ("RISC-V: hwprobe: Introduce which-cpus flag") I'm having trouble finding the tree/branch for this commit hash. > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > --- > arch/riscv/kernel/sys_hwprobe.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c > index ccf61b040536..10d72893b04f 100644 > --- a/arch/riscv/kernel/sys_hwprobe.c > +++ b/arch/riscv/kernel/sys_hwprobe.c > @@ -347,6 +347,10 @@ static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, > unsigned long __user *cpus_user, > unsigned int flags) > { > + /* The rest of the flags are still reserved. */ > + if (flags & ~RISCV_HWPROBE_WICHH_CPUS) ^ WHICH We shouldn't need this check since the way v3 of the series has do_riscv_hwprobe() is static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, size_t pair_count, size_t cpusetsize, unsigned long __user *cpus_user, unsigned int flags) { if (flags & RISCV_HWPROBE_WHICH_CPUS) return hwprobe_get_cpus(pairs, pair_count, cpusetsize, cpus_user, flags); return hwprobe_get_values(pairs, pair_count, cpusetsize, cpus_user, flags); } And hwprobe_get_cpus() starts with this check if (flags != RISCV_HWPROBE_WHICH_CPUS) return -EINVAL; And hwprobe_get_values() starts with the original check /* Check the reserved flags. */ if (flags != 0) return -EINVAL; IOW, we don't check flags at the top level, but each branch checks for its flags and hwprobe_get_values() doesn't have any. Thanks, drew > + return -EINVAL; > + > if (flags & RISCV_HWPROBE_WHICH_CPUS) > return hwprobe_get_cpus(pairs, pair_count, cpusetsize, > cpus_user, flags); > -- > 2.43.0 >
On Wed, 03 Jan 2024 05:23:57 PST (-0800), ajones@ventanamicro.com wrote: > Hi Palmer, > > On Wed, Jan 03, 2024 at 04:12:20AM -0800, Palmer Dabbelt wrote: >> This check got dropped when RISCV_HWPROBE_WHICH_CPUS was added, but >> we've still got a bunch of other reserved flags. So check for them, >> just to make sure userspace doesn't get confused later. >> >> Fixes: e178bf146e4b ("RISC-V: hwprobe: Introduce which-cpus flag") > > I'm having trouble finding the tree/branch for this commit hash. It's just over here <https://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git/log/?h=for-next>, that's where I push stuff before it goes to my testing box. Something rebooted, though, so it ended up tied up for a bit... > >> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> >> --- >> arch/riscv/kernel/sys_hwprobe.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c >> index ccf61b040536..10d72893b04f 100644 >> --- a/arch/riscv/kernel/sys_hwprobe.c >> +++ b/arch/riscv/kernel/sys_hwprobe.c >> @@ -347,6 +347,10 @@ static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, >> unsigned long __user *cpus_user, >> unsigned int flags) >> { >> + /* The rest of the flags are still reserved. */ >> + if (flags & ~RISCV_HWPROBE_WICHH_CPUS) > ^ WHICH > > > We shouldn't need this check since the way v3 of the series has > do_riscv_hwprobe() is > > static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, > size_t pair_count, size_t cpusetsize, > unsigned long __user *cpus_user, > unsigned int flags) > { > if (flags & RISCV_HWPROBE_WHICH_CPUS) > return hwprobe_get_cpus(pairs, pair_count, cpusetsize, > cpus_user, flags); > > return hwprobe_get_values(pairs, pair_count, cpusetsize, > cpus_user, flags); > } > > And hwprobe_get_cpus() starts with this check > > if (flags != RISCV_HWPROBE_WHICH_CPUS) > return -EINVAL; > > And hwprobe_get_values() starts with the original check > > /* Check the reserved flags. */ > if (flags != 0) > return -EINVAL; > > > IOW, we don't check flags at the top level, but each branch checks for its > flags and hwprobe_get_values() doesn't have any. Ya, sorry, I think I was just still asleep. > > Thanks, > drew > >> + return -EINVAL; >> + >> if (flags & RISCV_HWPROBE_WHICH_CPUS) >> return hwprobe_get_cpus(pairs, pair_count, cpusetsize, >> cpus_user, flags); >> -- >> 2.43.0 >>
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c index ccf61b040536..10d72893b04f 100644 --- a/arch/riscv/kernel/sys_hwprobe.c +++ b/arch/riscv/kernel/sys_hwprobe.c @@ -347,6 +347,10 @@ static int do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, unsigned long __user *cpus_user, unsigned int flags) { + /* The rest of the flags are still reserved. */ + if (flags & ~RISCV_HWPROBE_WICHH_CPUS) + return -EINVAL; + if (flags & RISCV_HWPROBE_WHICH_CPUS) return hwprobe_get_cpus(pairs, pair_count, cpusetsize, cpus_user, flags);
This check got dropped when RISCV_HWPROBE_WHICH_CPUS was added, but we've still got a bunch of other reserved flags. So check for them, just to make sure userspace doesn't get confused later. Fixes: e178bf146e4b ("RISC-V: hwprobe: Introduce which-cpus flag") Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> --- arch/riscv/kernel/sys_hwprobe.c | 4 ++++ 1 file changed, 4 insertions(+)