From: Carlos Bilbao <[email protected]>
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 ?
> 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 <[email protected]>
> ---
> 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.
Where does tp->irq_max go over TG3_IRQ_MAX_VECS ?
> struct tg3_napi *tnapi = &tp->napi[i];
>
> tnapi->tp = tp;
> --
> 2.41.0
On 7/17/23 14:24, Kuniyuki Iwashima wrote:
> From: Carlos Bilbao <[email protected]>
> 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 <[email protected]>
>> ---
>> 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
On Tue, 18 Jul 2023 10:52:39 -0500 Carlos Bilbao wrote:
> >> 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’:
What compiler are you using? Any extra flags?
I remember seeing this warning too, but I can't repro it now (gcc 13.1;
clang 16).
> >> 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 <[email protected]>
We need a sign-off tag
--
pw-bot: cr
On Wed, Jul 19, 2023 at 4:54 AM Jakub Kicinski <[email protected]> wrote:
>
> On Tue, 18 Jul 2023 10:52:39 -0500 Carlos Bilbao wrote:
> > >> 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’:
>
> What compiler are you using? Any extra flags?
>
> I remember seeing this warning too, but I can't repro it now (gcc 13.1;
> clang 16).
Same here, I think I was seeing this 4 or 5 years ago.
I ignored the warning at that time because we were using an old compiler.
>
> > >> 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 <[email protected]>
>
> We need a sign-off tag
> --
> pw-bot: cr
On 7/18/23 9:54 PM, Jakub Kicinski wrote:
> On Tue, 18 Jul 2023 10:52:39 -0500 Carlos Bilbao wrote:
>>>> 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’:
>
> What compiler are you using? Any extra flags?
GCC version 13.1.1. No special flags.
>
> I remember seeing this warning too, but I can't repro it now (gcc 13.1;
> clang 16).
>
>>>> 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 <[email protected]>
>
> We need a sign-off tag
Apologies, Signed-off-By: Carlos Bilbao <[email protected]>
Thanks,
Carlos
On Fri, Jul 21, 2023 at 3:45 PM Carlos Bilbao <[email protected]> wrote:
>
> On 7/18/23 9:54 PM, Jakub Kicinski wrote:
> > On Tue, 18 Jul 2023 10:52:39 -0500 Carlos Bilbao wrote:
> >>>> 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’:
> >
> > What compiler are you using? Any extra flags?
>
> GCC version 13.1.1. No special flags.
>
I've reviewed the driver code. tp->irq_max is always properly
initialized to 1 or TG3_IRQ_MAX_VECS. So this gcc warning seems
bogus.