| Message ID | 20210809211134.GA22488@embeddedor (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [next] mwifiex: usb: Replace one-element array with flexible-array member | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Mon, Aug 9, 2021 at 2:08 PM Gustavo A. R. Silva <gustavoars@kernel.org> wrote: > > There is a regular need in the kernel to provide a way to declare having > a dynamically sized set of trailing elements in a structure. Kernel code > should always use “flexible array members”[1] for these cases. The older > style of one-element or zero-length arrays should no longer be used[2]. > > This helps with the ongoing efforts to globally enable -Warray-bounds > and get us closer to being able to tighten the FORTIFY_SOURCE routines > on memcpy(). > > This issue was found with the help of Coccinelle and audited and fixed, > manually. > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > [2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays > > Link: https://github.com/KSPP/linux/issues/79 > Link: https://github.com/KSPP/linux/issues/109 > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> An important part of your patch rationale should include determining that the 1-length wasn't actually important anywhere. I double checked for you, and nobody seemed to be relying on 'sizeof struct fw_data' at all, so this should be OK: Reviewed-by: Brian Norris <briannorris@chromium.org>
On 8/9/21 16:24, Brian Norris wrote: > On Mon, Aug 9, 2021 at 2:08 PM Gustavo A. R. Silva > <gustavoars@kernel.org> wrote: >> >> There is a regular need in the kernel to provide a way to declare having >> a dynamically sized set of trailing elements in a structure. Kernel code >> should always use “flexible array members”[1] for these cases. The older >> style of one-element or zero-length arrays should no longer be used[2]. >> >> This helps with the ongoing efforts to globally enable -Warray-bounds >> and get us closer to being able to tighten the FORTIFY_SOURCE routines >> on memcpy(). >> >> This issue was found with the help of Coccinelle and audited and fixed, >> manually. >> >> [1] https://en.wikipedia.org/wiki/Flexible_array_member >> [2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays >> >> Link: https://github.com/KSPP/linux/issues/79 >> Link: https://github.com/KSPP/linux/issues/109 >> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > An important part of your patch rationale should include determining > that the 1-length wasn't actually important anywhere. I double checked > for you, and nobody seemed to be relying on 'sizeof struct fw_data' at > all, so this should be OK: I always do that. That's the reason why I included this line in the changelog text: "This issue was found with the help of Coccinelle and audited and fixed, manually." Thanks for double-checking, though. :) > Reviewed-by: Brian Norris <briannorris@chromium.org> Thanks -- Gustavo
"Gustavo A. R. Silva" <gustavoars@kernel.org> wrote: > There is a regular need in the kernel to provide a way to declare having > a dynamically sized set of trailing elements in a structure. Kernel code > should always use “flexible array members”[1] for these cases. The older > style of one-element or zero-length arrays should no longer be used[2]. > > This helps with the ongoing efforts to globally enable -Warray-bounds > and get us closer to being able to tighten the FORTIFY_SOURCE routines > on memcpy(). > > This issue was found with the help of Coccinelle and audited and fixed, > manually. > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > [2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays > > Link: https://github.com/KSPP/linux/issues/79 > Link: https://github.com/KSPP/linux/issues/109 > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Reviewed-by: Brian Norris <briannorris@chromium.org> Patch applied to wireless-drivers-next.git, thanks. 090f2c5d3d07 mwifiex: usb: Replace one-element array with flexible-array member
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h b/drivers/net/wireless/marvell/mwifiex/usb.h index d822ec15b7e6..61a96b7fbf21 100644 --- a/drivers/net/wireless/marvell/mwifiex/usb.h +++ b/drivers/net/wireless/marvell/mwifiex/usb.h @@ -134,7 +134,7 @@ struct fw_sync_header { struct fw_data { struct fw_header fw_hdr; __le32 seq_num; - u8 data[1]; + u8 data[]; } __packed; #endif /*_MWIFIEX_USB_H */
There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2]. This helps with the ongoing efforts to globally enable -Warray-bounds and get us closer to being able to tighten the FORTIFY_SOURCE routines on memcpy(). This issue was found with the help of Coccinelle and audited and fixed, manually. [1] https://en.wikipedia.org/wiki/Flexible_array_member [2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/109 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/net/wireless/marvell/mwifiex/usb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)