linux-kernel - Re: [PATCH] brcmfmac: fwsignal: Use struct_size() to simplify brcmf_fws

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0360afd2-8749-4be6-a451-78345dc44b12@broadcom.com>
Date: Sun, 14 Jul 2024 20:54:05 +0200
From: Arend van Spriel <arend.vanspriel@...adcom.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>,
 Kalle Valo <kvalo@...nel.org>
Cc: linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
 linux-wireless@...r.kernel.org, brcm80211@...ts.linux.dev,
 brcm80211-dev-list.pdl@...adcom.com
Subject: Re: [PATCH] brcmfmac: fwsignal: Use struct_size() to simplify
 brcmf_fws_rxreorder()

On 7/14/2024 12:10 PM, Christophe JAILLET wrote:
> In the "struct brcmf_ampdu_rx_reorder", change the 'pktslots' field into
> flexible array.
> 
> It saves the size of a pointer when the memory is allocated and avoids
> an indirection when the array is used.
> It also removes the usage of a pointer arithmetic and saves a few lines of
> code.
> 
> Finally, struct_size() can be used. It is not a must have here, because
> it is easy to see that buf_size can not overflow, but still, it is a good
> practice.

Looks good to me with a minor nit (see below)...

Acked-by: Arend van Spriel <arend.vanspriel@...adcom.com>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> Compile tested only
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h   | 4 ++--
>   .../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c   | 8 ++------
>   2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> index ea76b8d33401..6ea2b677f047 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> @@ -48,20 +48,20 @@
>   /**
>    * struct brcmf_ampdu_rx_reorder - AMPDU receive reorder info
>    *
> - * @pktslots: dynamic allocated array for ordering AMPDU packets.
>    * @flow_id: AMPDU flow identifier.
>    * @cur_idx: last AMPDU index from firmware.
>    * @exp_idx: expected next AMPDU index.
>    * @max_idx: maximum amount of packets per AMPDU.
>    * @pend_pkts: number of packets currently in @pktslots.
> + * @pktslots: dynamic allocated array for ordering AMPDU packets.

pktslots is not dynamic allocated, but just part of the structure. That 
was already so before this patch through. Just drop the "dynamic 
allocated" part.

>    */
>   struct brcmf_ampdu_rx_reorder {
> -	struct sk_buff **pktslots;
>   	u8 flow_id;
>   	u8 cur_idx;
>   	u8 exp_idx;
>   	u8 max_idx;
>   	u8 pend_pkts;
> +	struct sk_buff *pktslots[];
>   };
>   
>   /* Forward decls for struct brcmf_pub (see below) */

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4219 bytes)