Skip to contentSkip to conversation

pinctrl: stmfx: Fix hazardous u8[] to unsigned long cast

PATCH In review Marc Zyngier 2 participants 2 messages Jul 25, 2021 – Aug 10, 2021

Subsystems (3)
Stable branch Arm STM32 Pin control
lore
Share & tools
lore

Marc Zyngier

author
#0 lore raw

PATCH 1/1 pinctrl: stmfx: Fix hazardous u8[] to unsigned long cast

Casting a small array of u8 to an unsigned long is *never* OK:

- it does funny thing when the array size is less than that of a long, as it accesses random places in the stack
- it makes everything even more fun with a BE kernel

Fix this by building the unsigned long used as a bitmap byte by byte, in a way that works across endianess and has no undefined behaviours.

An extra BUILD_BUG_ON() catches the unlikely case where the array would be larger than a single unsigned long.

Fixes 1490d9f841b1 ("pinctrl: Add STMFX GPIO expander Pinctrl/GPIO driver")
Signed-off-by Marc Zyngier <[email protected]>
Cc [email protected]
Cc Amelie Delaunay <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Maxime Coquelin <[email protected]>
Cc Alexandre Torgue <[email protected]>
---
 drivers/pinctrl/pinctrl-stmfx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/pinctrl/pinctrl-stmfx.c +4 -2
index 008c83107a3c..5fa2488fae87 100644
@@ -566,7 +566,7 @@ static irqreturn_t stmfx_pinctrl_irq_thread_fn(int irq, void *dev_id)
 	u8 pending[NR_GPIO_REGS];
 	u8 src[NR_GPIO_REGS] = {0, 0, 0};
 	unsigned long n, status;
-	int ret;
+	int i, ret;
 
 	ret = regmap_bulk_read(pctl->stmfx->map, STMFX_REG_IRQ_GPI_PENDING,
 			       &pending, NR_GPIO_REGS);

@@ -576,7 +576,9 @@ static irqreturn_t stmfx_pinctrl_irq_thread_fn(int irq, void *dev_id)
 	regmap_bulk_write(pctl->stmfx->map, STMFX_REG_IRQ_GPI_SRC,
 			  src, NR_GPIO_REGS);
 
-	status = *(unsigned long *)pending;
+	BUILD_BUG_ON(NR_GPIO_REGS > sizeof(status));
+	for (i = 0, status = 0; i < NR_GPIO_REGS; i++)
+		status |= (unsigned long)pending[i] << (i * 8);
 	for_each_set_bit(n, &status, gc->ngpio) {
 		handle_nested_irq(irq_find_mapping(gc->irq.domain, n));
 		stmfx_pinctrl_irq_toggle_trigger(pctl, n);
Signature · 1 line
2.30.2
#1 lore raw
Quoted 12 lines · Marc Zyngier

Casting a small array of u8 to an unsigned long is *never* OK:

- it does funny thing when the array size is less than that of a long, as it accesses random places in the stack
- it makes everything even more fun with a BE kernel

Fix this by building the unsigned long used as a bitmap byte by byte, in a way that works across endianess and has no undefined behaviours.

An extra BUILD_BUG_ON() catches the unlikely case where the array would be larger than a single unsigned long.

Quoted 7 lines
Fixes 1490d9f841b1 ("pinctrl: Add STMFX GPIO expander Pinctrl/GPIO driver")
Signed-off-by Marc Zyngier <[email protected]>
Cc [email protected]
Cc Amelie Delaunay <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Maxime Coquelin <[email protected]>
Cc Alexandre Torgue <[email protected]>

Patch applied!

Yours,
Linus Walleij