diff mbox series

[3/4] net/ice/base: fix build with gcc11

Message ID 20210510150319.1496105-3-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers show
Series [1/4] net/bnx2x: fix build with gcc11 | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ferruh Yigit May 10, 2021, 3:03 p.m. UTC
Reproduced with '--buildtype=debugoptimized' config,
compiler version: gcc (GCC) 12.0.0 20210509 (experimental)

There are multiple build errors, like:
../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
../drivers/net/ice/base/ice_switch.c:3727:15:
	warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
	is partly outside array bounds of ‘unsigned char[52]’
	[-Warray-bounds]
 3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
      |               ^~
In file included from ../drivers/net/ice/base/ice_type.h:52,
                 from ../drivers/net/ice/base/ice_common.h:8,
                 from ../drivers/net/ice/base/ice_switch.h:8,
                 from ../drivers/net/ice/base/ice_switch.c:5:
../drivers/net/ice/base/ice_osdep.h:209:29:
	note: referencing an object of size 52 allocated by ‘rte_zmalloc’
  209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
      |                             ^~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/ice/base/ice_switch.c:3720:50:
	note: in expansion of macro ‘ice_malloc’
  lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);

These errors are mainly because allocated memory is cast to
"struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
of "struct ice_aqc_sw_rules_elem".

"struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
based on which one is used allocated memory being less than the size of
"struct ice_aqc_sw_rules_elem" is logically correct but compiler is
complaining about it.

As a solution making sure allocated memory size is at least size of
"struct ice_aqc_sw_rules_elem".
The function to use the struct is 'ice_aq_sw_rules()', and it already has
parameter for size of the rule, allocating more than needed shouldn't
cause any problem.

Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: paul.m.stillwell.jr@intel.com
Cc: qi.z.zhang@intel.com
Cc: leyi.rong@intel.com
Cc: Kevin Traynor <ktraynor@redhat.com>
Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

Comments

Wang, Haiyue May 10, 2021, 5:04 p.m. UTC | #1
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> Sent: Monday, May 10, 2021 23:03
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
> <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>;
> Shukla, Shivanshu <shivanshu.shukla@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>
> Reproduced with '--buildtype=debugoptimized' config,
> compiler version: gcc (GCC) 12.0.0 20210509 (experimental)
>
> There are multiple build errors, like:
> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
> ../drivers/net/ice/base/ice_switch.c:3727:15:
>       warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
>       is partly outside array bounds of ‘unsigned char[52]’
>       [-Warray-bounds]
>  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
>       |               ^~
> In file included from ../drivers/net/ice/base/ice_type.h:52,
>                  from ../drivers/net/ice/base/ice_common.h:8,
>                  from ../drivers/net/ice/base/ice_switch.h:8,
>                  from ../drivers/net/ice/base/ice_switch.c:5:
> ../drivers/net/ice/base/ice_osdep.h:209:29:
>       note: referencing an object of size 52 allocated by ‘rte_zmalloc’
>   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
>       |                             ^~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/net/ice/base/ice_switch.c:3720:50:
>       note: in expansion of macro ‘ice_malloc’
>   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
>
> These errors are mainly because allocated memory is cast to
> "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
> of "struct ice_aqc_sw_rules_elem".
>
> "struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
> based on which one is used allocated memory being less than the size of
> "struct ice_aqc_sw_rules_elem" is logically correct but compiler is
> complaining about it.
>
> As a solution making sure allocated memory size is at least size of
> "struct ice_aqc_sw_rules_elem".
> The function to use the struct is 'ice_aq_sw_rules()', and it already has
> parameter for size of the rule, allocating more than needed shouldn't
> cause any problem.
>
> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: paul.m.stillwell.jr@intel.com
> Cc: qi.z.zhang@intel.com
> Cc: leyi.rong@intel.com
> Cc: Kevin Traynor <ktraynor@redhat.com>
> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)

GCC bug ?

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266

Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual inheritance



> --
> 2.31.1
Wang, Haiyue May 10, 2021, 5:13 p.m. UTC | #2
> -----Original Message-----
> From: Wang, Haiyue
> Sent: Tuesday, May 11, 2021 01:05
> To: Ferruh Yigit <ferruh.yigit@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Shukla, Shivanshu
> <Shivanshu.Shukla@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
> Subject: RE: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> > Sent: Monday, May 10, 2021 23:03
> > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
> > <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Rong, Leyi
> <leyi.rong@intel.com>;
> > Shukla, Shivanshu <shivanshu.shukla@intel.com>
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
> > <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
> >
> > Reproduced with '--buildtype=debugoptimized' config,
> > compiler version: gcc (GCC) 12.0.0 20210509 (experimental)
> >
> > There are multiple build errors, like:
> > ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
> > ../drivers/net/ice/base/ice_switch.c:3727:15:
> >     warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
> >     is partly outside array bounds of ‘unsigned char[52]’
> >     [-Warray-bounds]
> >  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
> >       |               ^~
> > In file included from ../drivers/net/ice/base/ice_type.h:52,
> >                  from ../drivers/net/ice/base/ice_common.h:8,
> >                  from ../drivers/net/ice/base/ice_switch.h:8,
> >                  from ../drivers/net/ice/base/ice_switch.c:5:
> > ../drivers/net/ice/base/ice_osdep.h:209:29:
> >     note: referencing an object of size 52 allocated by ‘rte_zmalloc’
> >   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
> >       |                             ^~~~~~~~~~~~~~~~~~~~~~~
> > ../drivers/net/ice/base/ice_switch.c:3720:50:
> >     note: in expansion of macro ‘ice_malloc’
> >   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
> >
> > These errors are mainly because allocated memory is cast to
> > "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
> > of "struct ice_aqc_sw_rules_elem".
> >
> > "struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
> > based on which one is used allocated memory being less than the size of
> > "struct ice_aqc_sw_rules_elem" is logically correct but compiler is
> > complaining about it.
> >
> > As a solution making sure allocated memory size is at least size of
> > "struct ice_aqc_sw_rules_elem".
> > The function to use the struct is 'ice_aq_sw_rules()', and it already has
> > parameter for size of the rule, allocating more than needed shouldn't
> > cause any problem.
> >
> > Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
> > Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
> > Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> > Cc: paul.m.stillwell.jr@intel.com
> > Cc: qi.z.zhang@intel.com
> > Cc: leyi.rong@intel.com
> > Cc: Kevin Traynor <ktraynor@redhat.com>
> > Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >  drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
> >  1 file changed, 23 insertions(+), 7 deletions(-)
>
> GCC bug ?
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266
>
> Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual
> inheritance
>

Disable this gcc warning ?  ;-)

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=44720996e2d79e47d508b0abe99b931a726a3197

gcc-10: disable 'array-bounds' warning for now

>
>
> > --
> > 2.31.1
Ferruh Yigit May 10, 2021, 5:28 p.m. UTC | #3
On 5/10/2021 6:04 PM, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>> Sent: Monday, May 10, 2021 23:03
>> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
>> <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>;
>> Shukla, Shivanshu <shivanshu.shukla@intel.com>
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
>> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>>
>> Reproduced with '--buildtype=debugoptimized' config,
>> compiler version: gcc (GCC) 12.0.0 20210509 (experimental)
>>
>> There are multiple build errors, like:
>> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
>> ../drivers/net/ice/base/ice_switch.c:3727:15:
>>       warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
>>       is partly outside array bounds of ‘unsigned char[52]’
>>       [-Warray-bounds]
>>  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
>>       |               ^~
>> In file included from ../drivers/net/ice/base/ice_type.h:52,
>>                  from ../drivers/net/ice/base/ice_common.h:8,
>>                  from ../drivers/net/ice/base/ice_switch.h:8,
>>                  from ../drivers/net/ice/base/ice_switch.c:5:
>> ../drivers/net/ice/base/ice_osdep.h:209:29:
>>       note: referencing an object of size 52 allocated by ‘rte_zmalloc’
>>   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
>>       |                             ^~~~~~~~~~~~~~~~~~~~~~~
>> ../drivers/net/ice/base/ice_switch.c:3720:50:
>>       note: in expansion of macro ‘ice_malloc’
>>   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
>>
>> These errors are mainly because allocated memory is cast to
>> "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
>> of "struct ice_aqc_sw_rules_elem".
>>
>> "struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
>> based on which one is used allocated memory being less than the size of
>> "struct ice_aqc_sw_rules_elem" is logically correct but compiler is
>> complaining about it.
>>
>> As a solution making sure allocated memory size is at least size of
>> "struct ice_aqc_sw_rules_elem".
>> The function to use the struct is 'ice_aq_sw_rules()', and it already has
>> parameter for size of the rule, allocating more than needed shouldn't
>> cause any problem.
>>
>> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
>> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
>> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: paul.m.stillwell.jr@intel.com
>> Cc: qi.z.zhang@intel.com
>> Cc: leyi.rong@intel.com
>> Cc: Kevin Traynor <ktraynor@redhat.com>
>> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> ---
>>  drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
>>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> GCC bug ?
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266
> 
> Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual inheritance
> 

I am not sure if this is a gcc defect.

Here there is a memory allocated and assigned to "struct ice_aqc_sw_rules_elem
*", but allocated memory size is less than the struct size. As far as I
understand this is the reason of compiler warning.

For this case it may not be problem logically since both who allocates memory
and who uses the memory follows a contract, but there is a mismatch between
pointer type and object. If some other function wants to access all fields of
the struct, it will be out of bound access.
Ferruh Yigit May 10, 2021, 5:31 p.m. UTC | #4
On 5/10/2021 6:13 PM, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Wang, Haiyue
>> Sent: Tuesday, May 11, 2021 01:05
>> To: Ferruh Yigit <ferruh.yigit@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Shukla, Shivanshu
>> <Shivanshu.Shukla@intel.com>
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
>> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Subject: RE: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>>
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>>> Sent: Monday, May 10, 2021 23:03
>>> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
>>> <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Rong, Leyi
>> <leyi.rong@intel.com>;
>>> Shukla, Shivanshu <shivanshu.shukla@intel.com>
>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
>>> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>>>
>>> Reproduced with '--buildtype=debugoptimized' config,
>>> compiler version: gcc (GCC) 12.0.0 20210509 (experimental)
>>>
>>> There are multiple build errors, like:
>>> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
>>> ../drivers/net/ice/base/ice_switch.c:3727:15:
>>>     warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
>>>     is partly outside array bounds of ‘unsigned char[52]’
>>>     [-Warray-bounds]
>>>  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
>>>       |               ^~
>>> In file included from ../drivers/net/ice/base/ice_type.h:52,
>>>                  from ../drivers/net/ice/base/ice_common.h:8,
>>>                  from ../drivers/net/ice/base/ice_switch.h:8,
>>>                  from ../drivers/net/ice/base/ice_switch.c:5:
>>> ../drivers/net/ice/base/ice_osdep.h:209:29:
>>>     note: referencing an object of size 52 allocated by ‘rte_zmalloc’
>>>   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
>>>       |                             ^~~~~~~~~~~~~~~~~~~~~~~
>>> ../drivers/net/ice/base/ice_switch.c:3720:50:
>>>     note: in expansion of macro ‘ice_malloc’
>>>   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
>>>
>>> These errors are mainly because allocated memory is cast to
>>> "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
>>> of "struct ice_aqc_sw_rules_elem".
>>>
>>> "struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
>>> based on which one is used allocated memory being less than the size of
>>> "struct ice_aqc_sw_rules_elem" is logically correct but compiler is
>>> complaining about it.
>>>
>>> As a solution making sure allocated memory size is at least size of
>>> "struct ice_aqc_sw_rules_elem".
>>> The function to use the struct is 'ice_aq_sw_rules()', and it already has
>>> parameter for size of the rule, allocating more than needed shouldn't
>>> cause any problem.
>>>
>>> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
>>> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
>>> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>>> Cc: paul.m.stillwell.jr@intel.com
>>> Cc: qi.z.zhang@intel.com
>>> Cc: leyi.rong@intel.com
>>> Cc: Kevin Traynor <ktraynor@redhat.com>
>>> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> ---
>>>  drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
>>>  1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> GCC bug ?
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266
>>
>> Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual
>> inheritance
>>
> 
> Disable this gcc warning ?  ;-)
> 

That is an option as well but I prefer to fix it warning as much as possible
(unless false positive warnings).

What do you think about the patch? Do you see any problem with it?

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=44720996e2d79e47d508b0abe99b931a726a3197
> 
> gcc-10: disable 'array-bounds' warning for now
> 
>>
>>
>>> --
>>> 2.31.1
>
Stillwell Jr, Paul M May 10, 2021, 5:55 p.m. UTC | #5
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Monday, May 10, 2021 10:32 AM
> To: Wang, Haiyue <haiyue.wang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul
> M <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Rong, Leyi <leyi.rong@intel.com>; Shukla, Shivanshu
> <shivanshu.shukla@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Kevin Traynor <ktraynor@redhat.com>;
> Ajit Khaparde <ajit.khaparde@broadcom.com>
> Subject: Re: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
> 
> On 5/10/2021 6:13 PM, Wang, Haiyue wrote:
> >> -----Original Message-----
> >> From: Wang, Haiyue
> >> Sent: Tuesday, May 11, 2021 01:05
> >> To: Ferruh Yigit <ferruh.yigit@intel.com>; Yang, Qiming
> >> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >> Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo
> >> <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Shukla,
> >> Shivanshu <Shivanshu.Shukla@intel.com>
> >> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org;
> >> stable@dpdk.org; Kevin Traynor <ktraynor@redhat.com>; Ajit Khaparde
> >> <ajit.khaparde@broadcom.com>
> >> Subject: RE: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with
> >> gcc11
> >>
> >>> -----Original Message-----
> >>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> >>> Sent: Monday, May 10, 2021 23:03
> >>> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> >>> <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
> >>> <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> >>> Rong, Leyi
> >> <leyi.rong@intel.com>;
> >>> Shukla, Shivanshu <shivanshu.shukla@intel.com>
> >>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org;
> >>> stable@dpdk.org; Kevin Traynor <ktraynor@redhat.com>; Ajit Khaparde
> >>> <ajit.khaparde@broadcom.com>
> >>> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
> >>>
> >>> Reproduced with '--buildtype=debugoptimized' config, compiler
> >>> version: gcc (GCC) 12.0.0 20210509 (experimental)
> >>>
> >>> There are multiple build errors, like:
> >>> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
> >>> ../drivers/net/ice/base/ice_switch.c:3727:15:
> >>>     warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
> >>>     is partly outside array bounds of ‘unsigned char[52]’
> >>>     [-Warray-bounds]
> >>>  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
> >>>       |               ^~
> >>> In file included from ../drivers/net/ice/base/ice_type.h:52,
> >>>                  from ../drivers/net/ice/base/ice_common.h:8,
> >>>                  from ../drivers/net/ice/base/ice_switch.h:8,
> >>>                  from ../drivers/net/ice/base/ice_switch.c:5:
> >>> ../drivers/net/ice/base/ice_osdep.h:209:29:
> >>>     note: referencing an object of size 52 allocated by ‘rte_zmalloc’
> >>>   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
> >>>       |                             ^~~~~~~~~~~~~~~~~~~~~~~
> >>> ../drivers/net/ice/base/ice_switch.c:3720:50:
> >>>     note: in expansion of macro ‘ice_malloc’
> >>>   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw,
> >>> rules_size);
> >>>
> >>> These errors are mainly because allocated memory is cast to "struct
> >>> ice_aqc_sw_rules_elem *" but allocated size is less than the size of
> >>> "struct ice_aqc_sw_rules_elem".
> >>>
> >>> "struct ice_aqc_sw_rules_elem" has multiple other structs has
> >>> unions, based on which one is used allocated memory being less than
> >>> the size of "struct ice_aqc_sw_rules_elem" is logically correct but
> >>> compiler is complaining about it.
> >>>
> >>> As a solution making sure allocated memory size is at least size of
> >>> "struct ice_aqc_sw_rules_elem".
> >>> The function to use the struct is 'ice_aq_sw_rules()', and it
> >>> already has parameter for size of the rule, allocating more than
> >>> needed shouldn't cause any problem.
> >>>
> >>> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
> >>> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and
> >>> counter")
> >>> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> ---
> >>> Cc: paul.m.stillwell.jr@intel.com
> >>> Cc: qi.z.zhang@intel.com
> >>> Cc: leyi.rong@intel.com
> >>> Cc: Kevin Traynor <ktraynor@redhat.com>
> >>> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >>> ---
> >>>  drivers/net/ice/base/ice_switch.c | 30
> >>> +++++++++++++++++++++++-------
> >>>  1 file changed, 23 insertions(+), 7 deletions(-)
> >>
> >> GCC bug ?
> >>
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266
> >>
> >> Bug 98266 - [11 Regression] bogus array subscript is partly outside
> >> array bounds on virtual inheritance
> >>
> >
> > Disable this gcc warning ?  ;-)
> >
> 
> That is an option as well but I prefer to fix it warning as much as possible (unless
> false positive warnings).
> 
> What do you think about the patch? Do you see any problem with it?
> 

The correct way to handle this is to submit a patch to our internal shared code repo where the patch will get reviewed and merged. Once it is merged, then it can go upstream in DPDK.

Paul

> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
> > mit/?id=44720996e2d79e47d508b0abe99b931a726a3197
> >
> > gcc-10: disable 'array-bounds' warning for now
> >
> >>
> >>
> >>> --
> >>> 2.31.1
> >
Wang, Haiyue May 11, 2021, 1:59 a.m. UTC | #6
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, May 11, 2021 01:28
> To: Wang, Haiyue <haiyue.wang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Shukla, Shivanshu
> <shivanshu.shukla@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Kevin Traynor <ktraynor@redhat.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>
> Subject: Re: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
> 
> On 5/10/2021 6:04 PM, Wang, Haiyue wrote:
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> >> Sent: Monday, May 10, 2021 23:03
> >> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
> >> <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Rong, Leyi
> <leyi.rong@intel.com>;
> >> Shukla, Shivanshu <shivanshu.shukla@intel.com>
> >> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
> >> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
> >> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
> >>
> >> Reproduced with '--buildtype=debugoptimized' config,
> >> compiler version: gcc (GCC) 12.0.0 20210509 (experimental)
> >>
> >> There are multiple build errors, like:
> >> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
> >> ../drivers/net/ice/base/ice_switch.c:3727:15:
> >>       warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
> >>       is partly outside array bounds of ‘unsigned char[52]’
> >>       [-Warray-bounds]
> >>  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
> >>       |               ^~
> >> In file included from ../drivers/net/ice/base/ice_type.h:52,
> >>                  from ../drivers/net/ice/base/ice_common.h:8,
> >>                  from ../drivers/net/ice/base/ice_switch.h:8,
> >>                  from ../drivers/net/ice/base/ice_switch.c:5:
> >> ../drivers/net/ice/base/ice_osdep.h:209:29:
> >>       note: referencing an object of size 52 allocated by ‘rte_zmalloc’
> >>   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
> >>       |                             ^~~~~~~~~~~~~~~~~~~~~~~
> >> ../drivers/net/ice/base/ice_switch.c:3720:50:
> >>       note: in expansion of macro ‘ice_malloc’
> >>   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
> >>
> >> These errors are mainly because allocated memory is cast to
> >> "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
> >> of "struct ice_aqc_sw_rules_elem".
> >>
> >> "struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
> >> based on which one is used allocated memory being less than the size of
> >> "struct ice_aqc_sw_rules_elem" is logically correct but compiler is
> >> complaining about it.
> >>
> >> As a solution making sure allocated memory size is at least size of
> >> "struct ice_aqc_sw_rules_elem".
> >> The function to use the struct is 'ice_aq_sw_rules()', and it already has
> >> parameter for size of the rule, allocating more than needed shouldn't
> >> cause any problem.
> >>
> >> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
> >> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
> >> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >> Cc: paul.m.stillwell.jr@intel.com
> >> Cc: qi.z.zhang@intel.com
> >> Cc: leyi.rong@intel.com
> >> Cc: Kevin Traynor <ktraynor@redhat.com>
> >> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >> ---
> >>  drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
> >>  1 file changed, 23 insertions(+), 7 deletions(-)
> >
> > GCC bug ?
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266
> >
> > Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual
> inheritance
> >
> 
> I am not sure if this is a gcc defect.
> 
> Here there is a memory allocated and assigned to "struct ice_aqc_sw_rules_elem
> *", but allocated memory size is less than the struct size. As far as I
> understand this is the reason of compiler warning.
> 
> For this case it may not be problem logically since both who allocates memory
> and who uses the memory follows a contract, but there is a mismatch between
> pointer type and object. If some other function wants to access all fields of
> the struct, it will be out of bound access.
> 

That's why they are static function, not API. These functions are designed for
internally using to handle the compact memory layout with different types.

These XXX_SIZE is a helper to get the different memory type size. Of course,
this should be very careful, in other words, as you said: follows a contract. ;-)


#define DUMMY_ETH_HDR_LEN		16
#define ICE_SW_RULE_RX_TX_ETH_HDR_SIZE \
	(offsetof(struct ice_aqc_sw_rules_elem, pdata.lkup_tx_rx.hdr) + \
	 (DUMMY_ETH_HDR_LEN * \
	  sizeof(((struct ice_sw_rule_lkup_rx_tx *)0)->hdr[0])))
#define ICE_SW_RULE_RX_TX_NO_HDR_SIZE \
	(offsetof(struct ice_aqc_sw_rules_elem, pdata.lkup_tx_rx.hdr))
#define ICE_SW_RULE_LG_ACT_SIZE(n) \
	(offsetof(struct ice_aqc_sw_rules_elem, pdata.lg_act.act) + \
	 ((n) * sizeof(((struct ice_sw_rule_lg_act *)0)->act[0])))
#define ICE_SW_RULE_VSI_LIST_SIZE(n) \
	(offsetof(struct ice_aqc_sw_rules_elem, pdata.vsi_list.vsi) + \
	 ((n) * sizeof(((struct ice_sw_rule_vsi_list *)0)->vsi[0])))



> 
>
Kevin Traynor May 11, 2021, 9:08 a.m. UTC | #7
On 10/05/2021 18:28, Ferruh Yigit wrote:
> On 5/10/2021 6:04 PM, Wang, Haiyue wrote:
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>>> Sent: Monday, May 10, 2021 23:03
>>> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
>>> <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>;
>>> Shukla, Shivanshu <shivanshu.shukla@intel.com>
>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
>>> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>>>
>>> Reproduced with '--buildtype=debugoptimized' config,
>>> compiler version: gcc (GCC) 12.0.0 20210509 (experimental)
>>>
>>> There are multiple build errors, like:
>>> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
>>> ../drivers/net/ice/base/ice_switch.c:3727:15:
>>>       warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
>>>       is partly outside array bounds of ‘unsigned char[52]’
>>>       [-Warray-bounds]
>>>  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
>>>       |               ^~
>>> In file included from ../drivers/net/ice/base/ice_type.h:52,
>>>                  from ../drivers/net/ice/base/ice_common.h:8,
>>>                  from ../drivers/net/ice/base/ice_switch.h:8,
>>>                  from ../drivers/net/ice/base/ice_switch.c:5:
>>> ../drivers/net/ice/base/ice_osdep.h:209:29:
>>>       note: referencing an object of size 52 allocated by ‘rte_zmalloc’
>>>   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
>>>       |                             ^~~~~~~~~~~~~~~~~~~~~~~
>>> ../drivers/net/ice/base/ice_switch.c:3720:50:
>>>       note: in expansion of macro ‘ice_malloc’
>>>   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
>>>
>>> These errors are mainly because allocated memory is cast to
>>> "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
>>> of "struct ice_aqc_sw_rules_elem".
>>>
>>> "struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
>>> based on which one is used allocated memory being less than the size of
>>> "struct ice_aqc_sw_rules_elem" is logically correct but compiler is
>>> complaining about it.
>>>
>>> As a solution making sure allocated memory size is at least size of
>>> "struct ice_aqc_sw_rules_elem".
>>> The function to use the struct is 'ice_aq_sw_rules()', and it already has
>>> parameter for size of the rule, allocating more than needed shouldn't
>>> cause any problem.
>>>

Bugzilla ID: 678

>>> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
>>> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
>>> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
>>> Cc: stable@dpdk.org
>>>

If I apply on head of dpdk-next-net on F34 I still get some of these
warnings in ice. I'm using 'meson --werror -Dtests=false build'.

$ gcc --version
gcc (GCC) 11.1.1 20210428 (Red Hat 11.1.1-1)

>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>>> Cc: paul.m.stillwell.jr@intel.com
>>> Cc: qi.z.zhang@intel.com
>>> Cc: leyi.rong@intel.com
>>> Cc: Kevin Traynor <ktraynor@redhat.com>
>>> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> ---
>>>  drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
>>>  1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> GCC bug ?
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266
>>
>> Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual inheritance

This gcc defect is fixed in PR middle-end/98266 which was first in in
11.0.1-0.2 [1]. Currently Fedora 34 uses 11.1.1-1 and includes this PR.

[1] https://src.fedoraproject.org/rpms/gcc/blob/rawhide/f/gcc.spec#_3246

>>
> > I am not sure if this is a gcc defect.
> 
> Here there is a memory allocated and assigned to "struct ice_aqc_sw_rules_elem
> *", but allocated memory size is less than the struct size. As far as I
> understand this is the reason of compiler warning.
> 
> For this case it may not be problem logically since both who allocates memory
> and who uses the memory follows a contract, but there is a mismatch between
> pointer type and object. If some other function wants to access all fields of
> the struct, it will be out of bound access.
> 
> 
> 
>
Ferruh Yigit May 11, 2021, 9:33 a.m. UTC | #8
On 5/11/2021 2:59 AM, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Tuesday, May 11, 2021 01:28
>> To: Wang, Haiyue <haiyue.wang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Shukla, Shivanshu
>> <shivanshu.shukla@intel.com>
>> Cc: dev@dpdk.org; stable@dpdk.org; Kevin Traynor <ktraynor@redhat.com>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>
>> Subject: Re: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>>
>> On 5/10/2021 6:04 PM, Wang, Haiyue wrote:
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>>>> Sent: Monday, May 10, 2021 23:03
>>>> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stillwell Jr, Paul M
>>>> <paul.m.stillwell.jr@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Rong, Leyi
>> <leyi.rong@intel.com>;
>>>> Shukla, Shivanshu <shivanshu.shukla@intel.com>
>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org; Kevin Traynor
>>>> <ktraynor@redhat.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11
>>>>
>>>> Reproduced with '--buildtype=debugoptimized' config,
>>>> compiler version: gcc (GCC) 12.0.0 20210509 (experimental)
>>>>
>>>> There are multiple build errors, like:
>>>> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’:
>>>> ../drivers/net/ice/base/ice_switch.c:3727:15:
>>>>       warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’
>>>>       is partly outside array bounds of ‘unsigned char[52]’
>>>>       [-Warray-bounds]
>>>>  3727 |         lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT);
>>>>       |               ^~
>>>> In file included from ../drivers/net/ice/base/ice_type.h:52,
>>>>                  from ../drivers/net/ice/base/ice_common.h:8,
>>>>                  from ../drivers/net/ice/base/ice_switch.h:8,
>>>>                  from ../drivers/net/ice/base/ice_switch.c:5:
>>>> ../drivers/net/ice/base/ice_osdep.h:209:29:
>>>>       note: referencing an object of size 52 allocated by ‘rte_zmalloc’
>>>>   209 | #define ice_malloc(h, s)    rte_zmalloc(NULL, s, 0)
>>>>       |                             ^~~~~~~~~~~~~~~~~~~~~~~
>>>> ../drivers/net/ice/base/ice_switch.c:3720:50:
>>>>       note: in expansion of macro ‘ice_malloc’
>>>>   lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
>>>>
>>>> These errors are mainly because allocated memory is cast to
>>>> "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size
>>>> of "struct ice_aqc_sw_rules_elem".
>>>>
>>>> "struct ice_aqc_sw_rules_elem" has multiple other structs has unions,
>>>> based on which one is used allocated memory being less than the size of
>>>> "struct ice_aqc_sw_rules_elem" is logically correct but compiler is
>>>> complaining about it.
>>>>
>>>> As a solution making sure allocated memory size is at least size of
>>>> "struct ice_aqc_sw_rules_elem".
>>>> The function to use the struct is 'ice_aq_sw_rules()', and it already has
>>>> parameter for size of the rule, allocating more than needed shouldn't
>>>> cause any problem.
>>>>
>>>> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code")
>>>> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter")
>>>> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> ---
>>>> Cc: paul.m.stillwell.jr@intel.com
>>>> Cc: qi.z.zhang@intel.com
>>>> Cc: leyi.rong@intel.com
>>>> Cc: Kevin Traynor <ktraynor@redhat.com>
>>>> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>> ---
>>>>  drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++-------
>>>>  1 file changed, 23 insertions(+), 7 deletions(-)
>>>
>>> GCC bug ?
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266
>>>
>>> Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual
>> inheritance
>>>
>>
>> I am not sure if this is a gcc defect.
>>
>> Here there is a memory allocated and assigned to "struct ice_aqc_sw_rules_elem
>> *", but allocated memory size is less than the struct size. As far as I
>> understand this is the reason of compiler warning.
>>
>> For this case it may not be problem logically since both who allocates memory
>> and who uses the memory follows a contract, but there is a mismatch between
>> pointer type and object. If some other function wants to access all fields of
>> the struct, it will be out of bound access.
>>
> 
> That's why they are static function, not API. These functions are designed for
> internally using to handle the compact memory layout with different types.
> 
> These XXX_SIZE is a helper to get the different memory type size. Of course,
> this should be very careful, in other words, as you said: follows a contract. ;-)
> 

As said above the code is logically correct (at least as far as I can see), but
it can be dangerous from programming perspective.

> 
> #define DUMMY_ETH_HDR_LEN               16
> #define ICE_SW_RULE_RX_TX_ETH_HDR_SIZE \
>         (offsetof(struct ice_aqc_sw_rules_elem, pdata.lkup_tx_rx.hdr) + \
>          (DUMMY_ETH_HDR_LEN * \
>           sizeof(((struct ice_sw_rule_lkup_rx_tx *)0)->hdr[0])))
> #define ICE_SW_RULE_RX_TX_NO_HDR_SIZE \
>         (offsetof(struct ice_aqc_sw_rules_elem, pdata.lkup_tx_rx.hdr))
> #define ICE_SW_RULE_LG_ACT_SIZE(n) \
>         (offsetof(struct ice_aqc_sw_rules_elem, pdata.lg_act.act) + \
>          ((n) * sizeof(((struct ice_sw_rule_lg_act *)0)->act[0])))
> #define ICE_SW_RULE_VSI_LIST_SIZE(n) \
>         (offsetof(struct ice_aqc_sw_rules_elem, pdata.vsi_list.vsi) + \
>          ((n) * sizeof(((struct ice_sw_rule_vsi_list *)0)->vsi[0])))
> 
> 
> 
>>
>>
>
diff mbox series

Patch

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index 229b355c62c5..fa2a623aff8c 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -3704,6 +3704,7 @@  ice_add_marker_act(struct ice_hw *hw, struct ice_fltr_mgmt_list_entry *m_ent,
 	enum ice_status status;
 	u16 lg_act_size;
 	u16 rules_size;
+	u16 max_size;
 	u32 act;
 	u16 id;
 
@@ -3717,7 +3718,9 @@  ice_add_marker_act(struct ice_hw *hw, struct ice_fltr_mgmt_list_entry *m_ent,
 	 */
 	lg_act_size = (u16)ICE_SW_RULE_LG_ACT_SIZE(num_lg_acts);
 	rules_size = lg_act_size + ICE_SW_RULE_RX_TX_ETH_HDR_SIZE;
-	lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
+	max_size = sizeof(struct ice_aqc_sw_rules_elem) * 2;
+	lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw,
+		rules_size < max_size ? max_size : rules_size);
 	if (!lg_act)
 		return ICE_ERR_NO_MEMORY;
 
@@ -3803,6 +3806,7 @@  ice_add_counter_act(struct ice_hw *hw, struct ice_fltr_mgmt_list_entry *m_ent,
 	u16 lg_act_size;
 	u16 rules_size;
 	u16 f_rule_id;
+	u16 max_size;
 	u32 act;
 	u16 id;
 
@@ -3816,7 +3820,9 @@  ice_add_counter_act(struct ice_hw *hw, struct ice_fltr_mgmt_list_entry *m_ent,
 	 */
 	lg_act_size = (u16)ICE_SW_RULE_LG_ACT_SIZE(num_acts);
 	rules_size = lg_act_size + ICE_SW_RULE_RX_TX_ETH_HDR_SIZE;
-	lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size);
+	max_size = sizeof(struct ice_aqc_sw_rules_elem) * 2;
+	lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw,
+		rules_size < max_size ? max_size : rules_size);
 	if (!lg_act)
 		return ICE_ERR_NO_MEMORY;
 
@@ -4009,12 +4015,14 @@  static enum ice_status
 ice_create_pkt_fwd_rule(struct ice_hw *hw, struct ice_sw_recipe *recp_list,
 			struct ice_fltr_list_entry *f_entry)
 {
+	u16 max_size = sizeof(struct ice_aqc_sw_rules_elem);
 	struct ice_fltr_mgmt_list_entry *fm_entry;
 	struct ice_aqc_sw_rules_elem *s_rule;
 	enum ice_status status;
 
 	s_rule = (struct ice_aqc_sw_rules_elem *)
-		ice_malloc(hw, ICE_SW_RULE_RX_TX_ETH_HDR_SIZE);
+		ice_malloc(hw, ICE_SW_RULE_RX_TX_ETH_HDR_SIZE < max_size ?
+				max_size : ICE_SW_RULE_RX_TX_ETH_HDR_SIZE);
 	if (!s_rule)
 		return ICE_ERR_NO_MEMORY;
 	fm_entry = (struct ice_fltr_mgmt_list_entry *)
@@ -4068,11 +4076,13 @@  ice_create_pkt_fwd_rule(struct ice_hw *hw, struct ice_sw_recipe *recp_list,
 static enum ice_status
 ice_update_pkt_fwd_rule(struct ice_hw *hw, struct ice_fltr_info *f_info)
 {
+	u16 max_size = sizeof(struct ice_aqc_sw_rules_elem);
 	struct ice_aqc_sw_rules_elem *s_rule;
 	enum ice_status status;
 
 	s_rule = (struct ice_aqc_sw_rules_elem *)
-		ice_malloc(hw, ICE_SW_RULE_RX_TX_ETH_HDR_SIZE);
+		ice_malloc(hw, ICE_SW_RULE_RX_TX_ETH_HDR_SIZE < max_size ?
+				max_size : ICE_SW_RULE_RX_TX_ETH_HDR_SIZE);
 	if (!s_rule)
 		return ICE_ERR_NO_MEMORY;
 
@@ -4545,11 +4555,13 @@  ice_remove_rule_internal(struct ice_hw *hw, struct ice_sw_recipe *recp_list,
 	}
 
 	if (remove_rule) {
+		u16 max_size = sizeof(struct ice_aqc_sw_rules_elem);
 		/* Remove the lookup rule */
 		struct ice_aqc_sw_rules_elem *s_rule;
 
 		s_rule = (struct ice_aqc_sw_rules_elem *)
-			ice_malloc(hw, ICE_SW_RULE_RX_TX_NO_HDR_SIZE);
+			ice_malloc(hw, ICE_SW_RULE_RX_TX_NO_HDR_SIZE < max_size
+				? max_size : ICE_SW_RULE_RX_TX_NO_HDR_SIZE);
 		if (!s_rule) {
 			status = ICE_ERR_NO_MEMORY;
 			goto exit;
@@ -5264,6 +5276,7 @@  enum ice_status
 ice_cfg_dflt_vsi(struct ice_port_info *pi, u16 vsi_handle, bool set,
 		 u8 direction)
 {
+	u16 max_size = sizeof(struct ice_aqc_sw_rules_elem);
 	struct ice_aqc_sw_rules_elem *s_rule;
 	struct ice_fltr_info f_info;
 	struct ice_hw *hw = pi->hw;
@@ -5279,7 +5292,8 @@  ice_cfg_dflt_vsi(struct ice_port_info *pi, u16 vsi_handle, bool set,
 	s_rule_size = set ? ICE_SW_RULE_RX_TX_ETH_HDR_SIZE :
 		ICE_SW_RULE_RX_TX_NO_HDR_SIZE;
 
-	s_rule = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, s_rule_size);
+	s_rule = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, s_rule_size <
+		max_size ? max_size : s_rule_size);
 	if (!s_rule)
 		return ICE_ERR_NO_MEMORY;
 
@@ -8998,12 +9012,14 @@  ice_rem_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
 	}
 	ice_release_lock(rule_lock);
 	if (remove_rule) {
+		u16 max_size = sizeof(struct ice_aqc_sw_rules_elem);
 		struct ice_aqc_sw_rules_elem *s_rule;
 		u16 rule_buf_sz;
 
 		rule_buf_sz = ICE_SW_RULE_RX_TX_NO_HDR_SIZE;
 		s_rule = (struct ice_aqc_sw_rules_elem *)
-			ice_malloc(hw, rule_buf_sz);
+			ice_malloc(hw, rule_buf_sz < max_size ? max_size :
+				rule_buf_sz);
 		if (!s_rule)
 			return ICE_ERR_NO_MEMORY;
 		s_rule->pdata.lkup_tx_rx.act = 0;