Thread (9 messages) 9 messages, 5 authors, 2021-05-17

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

From: David Laight <hidden>
Date: 2021-05-13 09:10:23
Also in: linux-arm-kernel, linux-gpio, lkml

From: Colin King
quoted hunk ↗ jump to hunk
Sent: 13 May 2021 09:52

From: Colin Ian King <redacted>

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 <redacted>
---
 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)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help