diff mbox series

[next] wifi: mt76: mt7921: Replace fake flex-arrays with flexible-array members

Message ID ZBTUB/kJYQxq/6Cj@work
State New
Headers
Series [next] wifi: mt76: mt7921: Replace fake flex-arrays with flexible-array members |

Commit Message

Gustavo A. R. Silva March 17, 2023, 8:56 p.m. UTC 
  Zero-length arrays as fake flexible arrays are deprecated and we are
moving towards adopting C99 flexible-array members instead.

Address the following warnings found with GCC-13 and
-fstrict-flex-arrays=3 enabled:
drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:266:25: warning: array subscript 0 is outside array bounds of ‘struct mt7921_asar_dyn_limit_v2[0]’ [-Warray-bounds=]
drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:263:25: warning: array subscript 0 is outside array bounds of ‘struct mt7921_asar_dyn_limit[0]’ [-Warray-bounds=]
drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:223:28: warning: array subscript <unknown> is outside array bounds of ‘struct mt7921_asar_geo_limit_v2[0]’ [-Warray-bounds=]
drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:220:28: warning: array subscript <unknown> is outside array bounds of ‘struct mt7921_asar_geo_limit[0]’ [-Warray-bounds=]
drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:334:37: warning: array subscript i is outside array bounds of ‘u8[0]’ {aka ‘unsigned char[]’} [-Warray-bounds=]

Notice that the DECLARE_FLEX_ARRAY() helper allows for flexible-array
members in unions.

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].

Link: https://github.com/KSPP/linux/issues/21
Link: https://github.com/KSPP/linux/issues/272
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Simon Horman March 20, 2023, 2:56 p.m. UTC | #1 
On Fri, Mar 17, 2023 at 02:56:39PM -0600, Gustavo A. R. Silva wrote:
> Zero-length arrays as fake flexible arrays are deprecated and we are
> moving towards adopting C99 flexible-array members instead.
> 
> Address the following warnings found with GCC-13 and
> -fstrict-flex-arrays=3 enabled:
> drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:266:25: warning: array subscript 0 is outside array bounds of ‘struct mt7921_asar_dyn_limit_v2[0]’ [-Warray-bounds=]
> drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:263:25: warning: array subscript 0 is outside array bounds of ‘struct mt7921_asar_dyn_limit[0]’ [-Warray-bounds=]
> drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:223:28: warning: array subscript <unknown> is outside array bounds of ‘struct mt7921_asar_geo_limit_v2[0]’ [-Warray-bounds=]
> drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:220:28: warning: array subscript <unknown> is outside array bounds of ‘struct mt7921_asar_geo_limit[0]’ [-Warray-bounds=]
> drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.c:334:37: warning: array subscript i is outside array bounds of ‘u8[0]’ {aka ‘unsigned char[]’} [-Warray-bounds=]
> 
> Notice that the DECLARE_FLEX_ARRAY() helper allows for flexible-array
> members in unions.
> 
> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> routines on memcpy() and help us make progress towards globally
> enabling -fstrict-flex-arrays=3 [1].
> 
> Link: https://github.com/KSPP/linux/issues/21
> Link: https://github.com/KSPP/linux/issues/272
> Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h b/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h
> index 35268b0890ad..6f2c4a572572 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h

...

> @@ -69,7 +69,7 @@ struct mt7921_asar_geo_v2 {
>  	u8 version;
>  	u8 rsvd;
>  	u8 nr_tbl;
> -	struct mt7921_asar_geo_limit_v2 tbl[0];
> +	DECLARE_FLEX_ARRAY(struct mt7921_asar_geo_limit_v2, tbl);
>  } __packed;
>  
>  struct mt7921_asar_cl {
> @@ -85,7 +85,7 @@ struct mt7921_asar_fg {
>  	u8 rsvd;
>  	u8 nr_flag;
>  	u8 rsvd1;
> -	u8 flag[0];
> +	u8 flag[];

I am curious to know why DECLARE_FLEX_ARRAY isn't used here.


>  } __packed;
>  
>  struct mt7921_acpi_sar {
> -- 
> 2.34.1
>
Gustavo A. R. Silva March 20, 2023, 7:26 p.m. UTC | #2 
On Mon, Mar 20, 2023 at 03:56:54PM +0100, Simon Horman wrote:
> >  struct mt7921_asar_cl {
> > @@ -85,7 +85,7 @@ struct mt7921_asar_fg {
> >  	u8 rsvd;
> >  	u8 nr_flag;
> >  	u8 rsvd1;
> > -	u8 flag[0];
> > +	u8 flag[];
> 
> I am curious to know why DECLARE_FLEX_ARRAY isn't used here.

In contrast to the other structs, there is no object of type struct
mt7921_asar_fg declared in a union:

 91 struct mt7921_acpi_sar {
 92         u8 ver;
 93         union {
 94                 struct mt7921_asar_dyn *dyn;
 95                 struct mt7921_asar_dyn_v2 *dyn_v2;
 96         };
 97         union {
 98                 struct mt7921_asar_geo *geo;
 99                 struct mt7921_asar_geo_v2 *geo_v2;
100         };
101         struct mt7921_asar_cl *countrylist;
102         struct mt7921_asar_fg *fg;
103 };

The DECLARE_FLEX_ARRAY() helper was created to declare flex-array members
in unions or alone in structs[1].

--
Gustavo

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
Simon Horman March 20, 2023, 7:48 p.m. UTC | #3 
On Mon, Mar 20, 2023 at 01:26:49PM -0600, Gustavo A. R. Silva wrote:
> On Mon, Mar 20, 2023 at 03:56:54PM +0100, Simon Horman wrote:
> > >  struct mt7921_asar_cl {
> > > @@ -85,7 +85,7 @@ struct mt7921_asar_fg {
> > >  	u8 rsvd;
> > >  	u8 nr_flag;
> > >  	u8 rsvd1;
> > > -	u8 flag[0];
> > > +	u8 flag[];
> > 
> > I am curious to know why DECLARE_FLEX_ARRAY isn't used here.
> 
> In contrast to the other structs, there is no object of type struct
> mt7921_asar_fg declared in a union:
> 
>  91 struct mt7921_acpi_sar {
>  92         u8 ver;
>  93         union {
>  94                 struct mt7921_asar_dyn *dyn;
>  95                 struct mt7921_asar_dyn_v2 *dyn_v2;
>  96         };
>  97         union {
>  98                 struct mt7921_asar_geo *geo;
>  99                 struct mt7921_asar_geo_v2 *geo_v2;
> 100         };
> 101         struct mt7921_asar_cl *countrylist;
> 102         struct mt7921_asar_fg *fg;
> 103 };
> 
> The DECLARE_FLEX_ARRAY() helper was created to declare flex-array members
> in unions or alone in structs[1].

Thanks for the clarification, much appreciated.

FWIIW, with this information I'm happy with this patch.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Gustavo A. R. Silva March 30, 2023, 8:11 p.m. UTC | #4 
Hi all,

Friendly ping: who can take this, please? 😄

Thanks
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h b/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h
index 35268b0890ad..6f2c4a572572 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h
@@ -24,7 +24,7 @@  struct mt7921_asar_dyn {
 	u8 names[4];
 	u8 enable;
 	u8 nr_tbl;
-	struct mt7921_asar_dyn_limit tbl[0];
+	DECLARE_FLEX_ARRAY(struct mt7921_asar_dyn_limit, tbl);
 } __packed;
 
 struct mt7921_asar_dyn_limit_v2 {
@@ -37,7 +37,7 @@  struct mt7921_asar_dyn_v2 {
 	u8 enable;
 	u8 rsvd;
 	u8 nr_tbl;
-	struct mt7921_asar_dyn_limit_v2 tbl[0];
+	DECLARE_FLEX_ARRAY(struct mt7921_asar_dyn_limit_v2, tbl);
 } __packed;
 
 struct mt7921_asar_geo_band {
@@ -55,7 +55,7 @@  struct mt7921_asar_geo {
 	u8 names[4];
 	u8 version;
 	u8 nr_tbl;
-	struct mt7921_asar_geo_limit tbl[0];
+	DECLARE_FLEX_ARRAY(struct mt7921_asar_geo_limit, tbl);
 } __packed;
 
 struct mt7921_asar_geo_limit_v2 {
@@ -69,7 +69,7 @@  struct mt7921_asar_geo_v2 {
 	u8 version;
 	u8 rsvd;
 	u8 nr_tbl;
-	struct mt7921_asar_geo_limit_v2 tbl[0];
+	DECLARE_FLEX_ARRAY(struct mt7921_asar_geo_limit_v2, tbl);
 } __packed;
 
 struct mt7921_asar_cl {
@@ -85,7 +85,7 @@  struct mt7921_asar_fg {
 	u8 rsvd;
 	u8 nr_flag;
 	u8 rsvd1;
-	u8 flag[0];
+	u8 flag[];
 } __packed;
 
 struct mt7921_acpi_sar {