* [PATCH][next] wifi: mt76: Replace zero-length array with flexible-array member @ 2023-04-06 14:32 Gustavo A. R. Silva 2023-04-07 7:23 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Gustavo A. R. Silva @ 2023-04-06 14:32 UTC (permalink / raw) To: Felix Fietkau, Lorenzo Bianconi, Ryder Lee, Shayne Chen, Sean Wang, Kalle Valo, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno Cc: linux-wireless, netdev, linux-kernel, linux-arm-kernel, linux-mediatek, Gustavo A. R. Silva, linux-hardening 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(-) 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 { -- 2.34.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH][next] wifi: mt76: Replace zero-length array with flexible-array member 2023-04-06 14:32 [PATCH][next] wifi: mt76: Replace zero-length array with flexible-array member Gustavo A. R. Silva @ 2023-04-07 7:23 ` Simon Horman 2023-04-11 18:39 ` Gustavo A. R. Silva 0 siblings, 1 reply; 4+ messages in thread From: Simon Horman @ 2023-04-07 7:23 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Felix Fietkau, Lorenzo Bianconi, Ryder Lee, Shayne Chen, Sean Wang, Kalle Valo, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno, linux-wireless, netdev, linux-kernel, linux-arm-kernel, linux-mediatek, linux-hardening 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 { ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH][next] wifi: mt76: Replace zero-length array with flexible-array member 2023-04-07 7:23 ` Simon Horman @ 2023-04-11 18:39 ` Gustavo A. R. Silva 2023-04-17 11:28 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Gustavo A. R. Silva @ 2023-04-11 18:39 UTC (permalink / raw) To: Simon Horman, Gustavo A. R. Silva Cc: Felix Fietkau, Lorenzo Bianconi, Ryder Lee, Shayne Chen, Sean Wang, Kalle Valo, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno, linux-wireless, netdev, linux-kernel, linux-arm-kernel, linux-mediatek, linux-hardening 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 { ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][next] wifi: mt76: Replace zero-length array with flexible-array member 2023-04-11 18:39 ` Gustavo A. R. Silva @ 2023-04-17 11:28 ` Simon Horman 0 siblings, 0 replies; 4+ messages in thread From: Simon Horman @ 2023-04-17 11:28 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Gustavo A. R. Silva, Felix Fietkau, Lorenzo Bianconi, Ryder Lee, Shayne Chen, Sean Wang, Kalle Valo, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno, linux-wireless, netdev, linux-kernel, linux-arm-kernel, linux-mediatek, linux-hardening 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-17 11:30 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-06 14:32 [PATCH][next] wifi: mt76: Replace zero-length array with flexible-array member Gustavo A. R. Silva 2023-04-07 7:23 ` Simon Horman 2023-04-11 18:39 ` Gustavo A. R. Silva 2023-04-17 11:28 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).