"Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx> 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].
>
> Refactor the code a bit according to the use of a flexible-array member in
> struct scan_chan_list_params instead of a one-element array, and use the
> struct_size() helper.
>
> Also, save 25 (too many) bytes that were being allocated:
>
> $ pahole -C channel_param drivers/net/wireless/ath/ath11k/reg.o
> struct channel_param {
> u8 chan_id; /* 0 1 */
> u8 pwr; /* 1 1 */
> u32 mhz; /* 2 4 */
>
> /* Bitfield combined with next fields */
>
> u32 half_rate:1; /* 4:16 4 */
> u32 quarter_rate:1; /* 4:17 4 */
> u32 dfs_set:1; /* 4:18 4 */
> u32 dfs_set_cfreq2:1; /* 4:19 4 */
> u32 is_chan_passive:1; /* 4:20 4 */
> u32 allow_ht:1; /* 4:21 4 */
> u32 allow_vht:1; /* 4:22 4 */
> u32 allow_he:1; /* 4:23 4 */
> u32 set_agile:1; /* 4:24 4 */
> u32 psc_channel:1; /* 4:25 4 */
>
> /* XXX 6 bits hole, try to pack */
>
> u32 phy_mode; /* 8 4 */
> u32 cfreq1; /* 12 4 */
> u32 cfreq2; /* 16 4 */
> char maxpower; /* 20 1 */
> char minpower; /* 21 1 */
> char maxregpower; /* 22 1 */
> u8 antennamax; /* 23 1 */
> u8 reg_class_id; /* 24 1 */
>
> /* size: 25, cachelines: 1, members: 21 */
> /* sum members: 23 */
> /* sum bitfield members: 10 bits, bit holes: 1, sum bit holes: 6 bits */
> /* last cacheline: 25 bytes */
> } __attribute__((__packed__));
>
> as previously, sizeof(struct scan_chan_list_params) was 32 bytes:
>
> $ pahole -C scan_chan_list_params drivers/net/wireless/ath/ath11k/reg.o
> struct scan_chan_list_params {
> u32 pdev_id; /* 0 4 */
> u16 nallchans; /* 4 2 */
> struct channel_param ch_param[1]; /* 6 25 */
>
> /* size: 32, cachelines: 1, members: 3 */
> /* padding: 1 */
> /* last cacheline: 32 bytes */
> };
>
> and now with the flexible array transformation it is just 8 bytes:
>
> $ pahole -C scan_chan_list_params drivers/net/wireless/ath/ath11k/reg.o
> struct scan_chan_list_params {
> u32 pdev_id; /* 0 4 */
> u16 nallchans; /* 4 2 */
> struct channel_param ch_param[]; /* 6 0 */
>
> /* size: 8, cachelines: 1, members: 3 */
> /* padding: 2 */
> /* last cacheline: 8 bytes */
> };
>
> 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@xxxxxxxxxx>
> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
Patch applied to ath-next branch of ath.git, thanks.
b2549465cdea ath11k: Replace one-element array with flexible-array member
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210823172159.GA25800@embeddedor/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
