Re: [PATCH] tg3: fix array subscript out of bounds compilation error

[Carlos Bilbao]

On 7/17/23 14:24, Kuniyuki Iwashima wrote:
> From: Carlos Bilbao <carlos.bilbao@xxxxxxx>
> Date: Mon, 17 Jul 2023 09:34:43 -0500
> > Fix encountered compilation error in tg3.c where an array subscript was
>
> What is the error ?

drivers/net/ethernet/broadcom/tg3.c: In function ‘tg3_init_one’:
drivers/net/ethernet/broadcom/tg3.c:17795:51: error: array subscript 5 is 
above array bounds of ‘struct tg3_napi[5]’ [-Werror=array-bounds=]
17795 |                 struct tg3_napi *tnapi = &tp->napi[i];
      |                                           ~~~~~~~~^~~
In file included from drivers/net/ethernet/broadcom/tg3.c:72:
drivers/net/ethernet/broadcom/tg3.h:3203:41: note: while referencing ‘napi’
 3203 |         struct tg3_napi                 napi[TG3_IRQ_MAX_VECS];
      |                                         ^~~~
drivers/net/ethernet/broadcom/tg3.c:17795:51: error: array subscript 5 is 
above array bounds of ‘struct tg3_napi[5]’ [-Werror=array-bounds=]
17795 |                 struct tg3_napi *tnapi = &tp->napi[i];
      |                                           ~~~~~~~~^~~
drivers/net/ethernet/broadcom/tg3.h:3203:41: note: while referencing ‘napi’
 3203 |         struct tg3_napi                 napi[TG3_IRQ_MAX_VECS];

> > above the array bounds of 'struct tg3_napi[5]'. Add an additional check in
> > the for loop to ensure that it does not exceed the bounds of
> > 'struct tg3_napi' (defined by TG3_IRQ_MAX_VECS).
> >
> > Reviewed-By: Carlos Bilbao <carlos.bilbao@xxxxxxx>
> > ---
> >   drivers/net/ethernet/broadcom/tg3.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> > index 4179a12fc881..33ad75b7ed91 100644
> > --- a/drivers/net/ethernet/broadcom/tg3.c
> > +++ b/drivers/net/ethernet/broadcom/tg3.c
> > @@ -17791,7 +17791,7 @@ static int tg3_init_one(struct pci_dev *pdev,
> >   	intmbx = MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW;
> >   	rcvmbx = MAILBOX_RCVRET_CON_IDX_0 + TG3_64BIT_REG_LOW;
> >   	sndmbx = MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW;
> > -	for (i = 0; i < tp->irq_max; i++) {
> > +	for (i = 0; i < tp->irq_max && i < TG3_IRQ_MAX_VECS; i++) {
>
> I'm not familiar with this driver, but it seem tg3_init_one() calls
> tg3_get_invariants() before the loop and initialises irq_max as 1
> or TG3_IRQ_MAX_VECS.

I'm also not familiar with this driver, but what you mention seems to be
the case, and also tp->irq_max is used for loop boundaries elsewhere just
fine. I don't know why was the compiler complaining, maybe a false positive.

> Where does tp->irq_max go over TG3_IRQ_MAX_VECS ?
>
>
> >   		struct tg3_napi *tnapi = &tp->napi[i];
> >   
> >   		tnapi->tp = tp;
> > --
> > 2.41.0

Thanks,
Carlos