| Message ID | ZC7X7KCb+JEkPe5D@work (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | [next] wifi: mt76: Replace zero-length array with flexible-array member | expand |
On Thu, Apr 06, 2023 at 08:32:12AM -0600, Gustavo A. R. Silva wrote: > Zero-length arrays are deprecated [1] and have to be replaced by C99 > flexible-array members. > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines > on memcpy() and help to make progress towards globally enabling > -fstrict-flex-arrays=3 [2] > > Link: https://github.com/KSPP/linux/issues/78 [1] > Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2] > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Simon Horman <simon.horman@corigine.com> > --- > drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h > index a5e6ee4daf92..9bf4b4199ee3 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h > @@ -127,7 +127,7 @@ struct mt76_connac2_mcu_rxd { > u8 rsv1[2]; > u8 s2d_index; > > - u8 tlv[0]; > + u8 tlv[]; > }; > > struct mt76_connac2_patch_hdr { Curiously -fstrict-flex-arrays=3 didn't flag this one in my environment, Ubuntu Lunar. gcc-13 --version gcc-13 (Ubuntu 13-20230320-1ubuntu1) 13.0.1 20230320 (experimental) [master r13-6759-g5194ad1958c] Copyright (C) 2023 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. I did, however, notice some other problems reported by gcc-13 with -fstrict-flex-arrays=3 in drivers/net/wireless/mediatek/mt76 when run against net-next wireless. I've listed them in diff format below. Are these on your radar too? diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h b/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h index 35268b0890ad..d09bb4eed1ec 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]; + 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]; + 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]; + 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]; + 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 {
On 4/7/23 01:23, Simon Horman wrote: > On Thu, Apr 06, 2023 at 08:32:12AM -0600, Gustavo A. R. Silva wrote: >> Zero-length arrays are deprecated [1] and have to be replaced by C99 >> flexible-array members. >> >> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines >> on memcpy() and help to make progress towards globally enabling >> -fstrict-flex-arrays=3 [2] >> >> Link: https://github.com/KSPP/linux/issues/78 [1] >> Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2] >> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > Reviewed-by: Simon Horman <simon.horman@corigine.com> Thanks :) > >> --- >> drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h >> index a5e6ee4daf92..9bf4b4199ee3 100644 >> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h >> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h >> @@ -127,7 +127,7 @@ struct mt76_connac2_mcu_rxd { >> u8 rsv1[2]; >> u8 s2d_index; >> >> - u8 tlv[0]; >> + u8 tlv[]; >> }; >> >> struct mt76_connac2_patch_hdr { > > Curiously -fstrict-flex-arrays=3 didn't flag this one in my environment, > Ubuntu Lunar. Yep; that's why I didn't include any warning message in the changelog text this time. And the reason for that is that tlv is not being indexed anywhere in the code. However, it's being used in the pointer arithmetic operation below: drivers/net/wireless/mediatek/mt76/mt7921/mcu.c: 164 rxd = (struct mt76_connac2_mcu_rxd *)skb->data; 165 grant = (struct mt7921_roc_grant_tlv *)(rxd->tlv + 4); which means that it can be considered as an array of size greater than zero at some point. Hence, it should be transformed into a C99 flexible array. > > gcc-13 --version > gcc-13 (Ubuntu 13-20230320-1ubuntu1) 13.0.1 20230320 (experimental) [master r13-6759-g5194ad1958c] > Copyright (C) 2023 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > I did, however, notice some other problems reported by gcc-13 with > -fstrict-flex-arrays=3 in drivers/net/wireless/mediatek/mt76 > when run against net-next wireless. I've listed them in diff format below. > > Are these on your radar too? Yep; those are being addressed here: https://lore.kernel.org/linux-hardening/ZBTUB%2FkJYQxq%2F6Cj@work/ -- Gustavo > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h b/drivers/net/wireless/mediatek/mt76/mt7921/acpi_sar.h > index 35268b0890ad..d09bb4eed1ec 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]; > + 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]; > + 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]; > + 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]; > + 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 {
On Tue, Apr 11, 2023 at 12:39:06PM -0600, Gustavo A. R. Silva wrote: > [You don't often get email from gustavo@embeddedor.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > On 4/7/23 01:23, Simon Horman wrote: > > On Thu, Apr 06, 2023 at 08:32:12AM -0600, Gustavo A. R. Silva wrote: > > > Zero-length arrays are deprecated [1] and have to be replaced by C99 > > > flexible-array members. > > > > > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines > > > on memcpy() and help to make progress towards globally enabling > > > -fstrict-flex-arrays=3 [2] > > > > > > Link: https://github.com/KSPP/linux/issues/78 [1] > > > Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2] > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > > > Reviewed-by: Simon Horman <simon.horman@corigine.com> > > Thanks :) > > > > > > --- > > > drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h > > > index a5e6ee4daf92..9bf4b4199ee3 100644 > > > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h > > > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h > > > @@ -127,7 +127,7 @@ struct mt76_connac2_mcu_rxd { > > > u8 rsv1[2]; > > > u8 s2d_index; > > > > > > - u8 tlv[0]; > > > + u8 tlv[]; > > > }; > > > > > > struct mt76_connac2_patch_hdr { > > > > Curiously -fstrict-flex-arrays=3 didn't flag this one in my environment, > > Ubuntu Lunar. > > Yep; that's why I didn't include any warning message in the changelog text > this time. > > And the reason for that is that tlv is not being indexed anywhere in the > code. However, it's being used in the pointer arithmetic operation below: > > drivers/net/wireless/mediatek/mt76/mt7921/mcu.c: > 164 rxd = (struct mt76_connac2_mcu_rxd *)skb->data; > 165 grant = (struct mt7921_roc_grant_tlv *)(rxd->tlv + 4); > > > which means that it can be considered as an array of size greater than zero > at some point. Hence, it should be transformed into a C99 flexible array. Understood, thanks for the explanation. > > gcc-13 --version > > gcc-13 (Ubuntu 13-20230320-1ubuntu1) 13.0.1 20230320 (experimental) [master r13-6759-g5194ad1958c] > > Copyright (C) 2023 Free Software Foundation, Inc. > > This is free software; see the source for copying conditions. There is NO > > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > > > I did, however, notice some other problems reported by gcc-13 with > > -fstrict-flex-arrays=3 in drivers/net/wireless/mediatek/mt76 > > when run against net-next wireless. I've listed them in diff format below. > > > > Are these on your radar too? > > Yep; those are being addressed here: > > https://lore.kernel.org/linux-hardening/ZBTUB%2FkJYQxq%2F6Cj@work/ Thanks, I had forgotten about that.
diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h index a5e6ee4daf92..9bf4b4199ee3 100644 --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h @@ -127,7 +127,7 @@ struct mt76_connac2_mcu_rxd { u8 rsv1[2]; u8 s2d_index; - u8 tlv[0]; + u8 tlv[]; }; struct mt76_connac2_patch_hdr {
Zero-length arrays are deprecated [1] and have to be replaced by C99 flexible-array members. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help to make progress towards globally enabling -fstrict-flex-arrays=3 [2] Link: https://github.com/KSPP/linux/issues/78 [1] Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2] Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)