Skip to contentSkip to conversation

pinctrl: pinctrl-microchip-sgpio: out-of-bounds bug in sgpio_clrsetbits()

REPORT Gustavo A. R. Silva 3 participants 3 messages Feb 2, 2021 – Feb 3, 2021 GPIO

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

Hi,

While addressing some out-of-bounds warnings, I found the following bug:

drivers/pinctrl/pinctrl-microchip-sgpio.c:154:57: warning: array subscript 10 is above array bounds of ‘const u8[10]’ {aka ‘const unsigned char[10]’} [-Warray-bounds]

The bug was introduced by commit be2dc859abd4 ("pinctrl: pinctrl-microchip-sgpio: Add irq support (for sparx5)"):

575 sgpio_clrsetbits(bank->priv, REG_INT_TRIGGER + SGPIO_MAX_BITS, addr.bit,
576 BIT(addr.port), (!!(type & 0x2)) << addr.port);

REG_INT_TRIGGER + SGPIO_MAX_BITS turns out to be 10, which is outside the boundaries of priv->properties->regoff[] at line 154:

151 static inline void sgpio_clrsetbits(struct sgpio_priv *priv,
152 u32 rno, u32 off, u32 clear, u32 set)
153 {
154 u32 __iomem *reg = &priv->regs[priv->properties->regoff[rno] + off];
155 u32 val = readl(reg);
156
157 val &= ~clear;
158 val |= set;
159
160 writel(val, reg);
161 }

because priv->properties->regoff[] is an array of MAXREG elements, with MAXREG representing the value of 10 in the following enum:

28 enum {

 29         REG_INPUT_DATA,                                                                             
 30         REG_PORT_CONFIG,                                                                            
 31         REG_PORT_ENABLE,                                                                            
 32         REG_SIO_CONFIG,                                                                             
 33         REG_SIO_CLOCK,                                                                              
 34         REG_INT_POLARITY,                                                                           
 35         REG_INT_TRIGGER,                                                                            
 36         REG_INT_ACK,                                                                                
 37         REG_INT_ENABLE,                                                                             
 38         REG_INT_IDENT,                                                                              
 39         MAXREG

40 };

52 struct sgpio_properties {

 53         int arch;                                                                                   
 54         int flags;                                                                                  
 55         u8 regoff[MAXREG];

56 };

Thanks
--
Gustavo

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#1 lore raw
Quoted 6 lines · Gustavo A. R. Silva

Hi,

While addressing some out-of-bounds warnings, I found the following bug:

drivers/pinctrl/pinctrl-microchip-sgpio.c:154:57: warning: array subscript 10 is above array bounds of ‘const u8[10]’ {aka ‘const unsigned char[10]’} [-Warray-bounds]

Quoted 7 lines

The bug was introduced by commit be2dc859abd4 ("pinctrl: pinctrl-microchip-sgpio: Add irq support (for sparx5)"):

575 sgpio_clrsetbits(bank->priv, REG_INT_TRIGGER + SGPIO_MAX_BITS, addr.bit,
576 BIT(addr.port), (!!(type & 0x2)) << addr.port);

REG_INT_TRIGGER + SGPIO_MAX_BITS turns out to be 10, which is outside the boundaries of priv->properties->regoff[] at line 154:

Hi Gustavo!

Thanks for spotting this - the "+" is misplaced. I will submit a patch asap.

Cheers,

---Lars

Quoted 37 lines

151 static inline void sgpio_clrsetbits(struct sgpio_priv *priv,
152 u32 rno, u32 off, u32 clear, u32 set)
153 {
154 u32 __iomem *reg = &priv->regs[priv->properties->regoff[rno] + off];
155 u32 val = readl(reg);
156
157 val &= ~clear;
158 val |= set;
159
160 writel(val, reg);
161 }

because priv->properties->regoff[] is an array of MAXREG elements, with MAXREG representing the value of 10 in the following enum:

28 enum {

 29         REG_INPUT_DATA,
 30         REG_PORT_CONFIG,
 31         REG_PORT_ENABLE,
 32         REG_SIO_CONFIG,
 33         REG_SIO_CLOCK,
 34         REG_INT_POLARITY,
 35         REG_INT_TRIGGER,
 36         REG_INT_ACK,
 37         REG_INT_ENABLE,
 38         REG_INT_IDENT,
 39         MAXREG

40 };

52 struct sgpio_properties {

 53         int arch;
 54         int flags;
 55         u8 regoff[MAXREG];

56 };

Thanks

Signature · 7 lines
Lars Povlsen,
Microchip

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#2 lore raw
Quoted 14 lines · Lars Povlsen
Quoted 11 lines · Gustavo A. R. Silva

Hi,

While addressing some out-of-bounds warnings, I found the following bug:

drivers/pinctrl/pinctrl-microchip-sgpio.c:154:57: warning: array subscript 10 is above array bounds of ‘const u8[10]’ {aka ‘const unsigned char[10]’} [-Warray-bounds]

The bug was introduced by commit be2dc859abd4 ("pinctrl: pinctrl-microchip-sgpio: Add irq support (for sparx5)"):

575 sgpio_clrsetbits(bank->priv, REG_INT_TRIGGER + SGPIO_MAX_BITS, addr.bit,
576 BIT(addr.port), (!!(type & 0x2)) << addr.port);

Quoted 7 lines
Quoted 2 lines

REG_INT_TRIGGER + SGPIO_MAX_BITS turns out to be 10, which is outside the boundaries of priv->properties->regoff[] at line 154:

Hi Gustavo!

Thanks for spotting this - the "+" is misplaced. I will submit a patch asap.

Awesome. :)

Thanks
--
Gustavo

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel