All of lore.kernel.org
 help / color / mirror / Atom feed
* [REPORT][next] pinctrl: pinctrl-microchip-sgpio: out-of-bounds bug in sgpio_clrsetbits()
@ 2021-02-02 11:34 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2021-02-02 11:34 UTC (permalink / raw)
  To: Lars Povlsen, Steen Hegelund, UNGLinuxDriver, Linus Walleij
  Cc: linux-arm-kernel, linux-gpio, linux-kernel, linux-hardening

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [REPORT][next] pinctrl: pinctrl-microchip-sgpio: out-of-bounds bug in sgpio_clrsetbits()
@ 2021-02-02 11:34 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2021-02-02 11:34 UTC (permalink / raw)
  To: Lars Povlsen, Steen Hegelund, UNGLinuxDriver, Linus Walleij
  Cc: linux-gpio, linux-kernel, linux-arm-kernel, linux-hardening

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
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [REPORT][next] pinctrl: pinctrl-microchip-sgpio: out-of-bounds bug in sgpio_clrsetbits()
  2021-02-02 11:34 ` Gustavo A. R. Silva
@ 2021-02-03 12:00   ` Lars Povlsen
  -1 siblings, 0 replies; 5+ messages in thread
From: Lars Povlsen @ 2021-02-03 12:00 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Lars Povlsen, Steen Hegelund, UNGLinuxDriver, Linus Walleij,
	linux-arm-kernel, linux-gpio, linux-kernel, linux-hardening


Gustavo A. R. Silva writes:

> 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:

Hi Gustavo!

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

Cheers,

---Lars

>
> 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


-- 
Lars Povlsen,
Microchip

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [REPORT][next] pinctrl: pinctrl-microchip-sgpio: out-of-bounds bug in sgpio_clrsetbits()
@ 2021-02-03 12:00   ` Lars Povlsen
  0 siblings, 0 replies; 5+ messages in thread
From: Lars Povlsen @ 2021-02-03 12:00 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Steen Hegelund, linux-kernel, UNGLinuxDriver, linux-gpio,
	Lars Povlsen, Linus Walleij, linux-arm-kernel, linux-hardening


Gustavo A. R. Silva writes:

> 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:

Hi Gustavo!

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

Cheers,

---Lars

>
> 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


-- 
Lars Povlsen,
Microchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [REPORT][next] pinctrl: pinctrl-microchip-sgpio: out-of-bounds bug in sgpio_clrsetbits()
  2021-02-03 12:00   ` Lars Povlsen
  (?)
@ 2021-02-03 12:51   ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2021-02-03 12:51 UTC (permalink / raw)
  To: Lars Povlsen, Gustavo A. R. Silva
  Cc: Steen Hegelund, linux-kernel, UNGLinuxDriver, linux-gpio,
	linux-hardening, Linus Walleij, linux-arm-kernel



On 2/3/21 06:00, Lars Povlsen wrote:
> 
> Gustavo A. R. Silva writes:
> 
>> 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:
> 
> Hi Gustavo!
> 
> Thanks for spotting this - the "+" is misplaced. I will submit a patch
> asap.

Awesome. :)

Thanks
--
Gustavo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-02-03 12:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 11:34 [REPORT][next] pinctrl: pinctrl-microchip-sgpio: out-of-bounds bug in sgpio_clrsetbits() Gustavo A. R. Silva
2021-02-02 11:34 ` Gustavo A. R. Silva
2021-02-03 12:00 ` Lars Povlsen
2021-02-03 12:00   ` Lars Povlsen
2021-02-03 12:51   ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.