Hi,
On Mon, 11 Feb 2019 at 05:39, Michael Clark <michaeljclark@xxxxxxx> wrote:
>
> __cmpxchg_small erroneously uses u8 for load comparison which can
> be either char or short. This patch changes the local varialble to
varialble => variable
> u32 which is sufficiently sized, as the loaded value is already
> masked and shifted appropriately. Using an integer size avoids
> any unnecessary canonicalization from use of non native widths.
>
> This patch is part of a series that adapts the MIPS small word
> atomics code for xchg and cmpxchg on short and char to RISC-V.
>
> Cc: RISC-V Patches <patches@xxxxxxxxxxxxxxxx>
> Cc: Linux RISC-V <linux-riscv@xxxxxxxxxxxxxxxxxxx>
> Cc: Linux MIPS <linux-mips@xxxxxxxxxxxxxx>
> Signed-off-by: Michael Clark <michaeljclark@xxxxxxx>
> ---
> arch/mips/kernel/cmpxchg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/cmpxchg.c b/arch/mips/kernel/cmpxchg.c
> index 0b9535bc2c53..1169958fd748 100644
> --- a/arch/mips/kernel/cmpxchg.c
> +++ b/arch/mips/kernel/cmpxchg.c
> @@ -57,7 +57,7 @@ unsigned long __cmpxchg_small(volatile void *ptr, unsigned long old,
> u32 mask, old32, new32, load32;
> volatile u32 *ptr32;
> unsigned int shift;
> - u8 load;
> + u32 load;
There already is a u32 line above, so maybe move it there.
Also reading the code to understand this, isn't the old code broken
for cmpxchg_small calls for 16-bit variables, where old is > 0xff?
because it does later
/*
* Ensure the byte we want to exchange matches the expected
* old value, and if not then bail.
*/
load = (load32 & mask) >> shift;
if (load != old)
return load;
and if load is a u8, it can never be old if old contains a larger
value than what can fit in a u8.
After re-reading your commit log, it seems you say something similar,
but it wasn't quite obvious for me that this means the function is
basically broken for short values where the old value does not fit in
u8.
So this should have an appropriate "Fixes:" tag. And Cc: stable. Seems
like quite a serious issue to me.
Regards
Jonas
Jonas
