[PATCH][next] gpio: xilinx: Fix potential integer overflow on shift of a u32 int

David Laight David.Laight at ACULAB.COM
Thu May 13 02:10:07 PDT 2021


From: Colin King
> Sent: 13 May 2021 09:52
> 
> From: Colin Ian King <colin.king at canonical.com>
> 
> The left shift of the u32 integer v is evaluated using 32 bit
> arithmetic and then assigned to a u64 integer. There are cases
> where v will currently overflow on the shift. Avoid this by
> casting it to unsigned long (same type as map[]) before shifting
> it.
> 
> Addresses-Coverity: ("Unintentional integer overflow")
> Fixes: 02b3f84d9080 ("gpio: xilinx: Switch to use bitmap APIs")
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  drivers/gpio/gpio-xilinx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> index 109b32104867..164a3a5a9393 100644
> --- a/drivers/gpio/gpio-xilinx.c
> +++ b/drivers/gpio/gpio-xilinx.c
> @@ -99,7 +99,7 @@ static inline void xgpio_set_value32(unsigned long *map, int bit, u32 v)
>  	const unsigned long offset = (bit % BITS_PER_LONG) & BIT(5);
> 
>  	map[index] &= ~(0xFFFFFFFFul << offset);
> -	map[index] |= v << offset;
> +	map[index] |= (unsigned long)v << offset;
>  }

That code looks dubious on 32bit architectures.

I don't have 02b3f84d9080 in any of my source trees.
But that patch may itself be very dubious.

Since the hardware requires explicit bits be set, relying
on the bitmap functions seems pointless and possibly wrong.
Clearly they cause additional problems because they use long[]
and here the code needs u32[].

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


More information about the linux-arm-kernel mailing list