Re: [PATCH v2] brcmfmac: replace one-element array with flexible-array member

[Kees Cook <keescook@xxxxxxxxxxxx>]

On Thu, Jan 18, 2024 at 02:53:28PM +0100, Arend van Spriel wrote:
> + Kees
> 
> On 8/3/2023 7:22 AM, Atul Raut wrote:
> > One-element arrays are obsolete, and flexible
> > array members have taken their place. So, in
> > struct cca_stats_n_flags, replace the one-element
> > array with a flexible-array member.
> > 
> > This fixes warnings such as:
> > ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> 
> I think this correct, but maybe Kees can give definitive answer here.
> 
> > Signed-off-by: Atul Raut <rauji.raut@xxxxxxxxx>
> > ---
> >   drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > index de8a2e27f49c..fff32e54833d 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > @@ -116,7 +116,7 @@ struct brcmf_dump_survey {
> >   struct cca_stats_n_flags {
> >   	u32 msrmnt_time; /* Time for Measurement (msec) */
> >   	u32 msrmnt_done; /* flag set when measurement complete */
> > -	char buf[1];
> > +	char buf[];
> >   };
> >   struct cca_msrmnt_query {

Normally a [1] -> [] conversion needs some details in the commit log
about why this is safe (since it runs the risk of changing sizeof(struct
cca_stats_n_flags). In this case, there's only a single user of the
struct, and nothing about its use depends on its size:

#define BRCMF_DCMD_MEDLEN       1536
...
        struct cca_stats_n_flags *results;
        char *buf;
	...
        buf = kzalloc(sizeof(char) * BRCMF_DCMD_MEDLEN, GFP_KERNEL);
	...
        results = (struct cca_stats_n_flags *)(buf);
	...
                brcmf_parse_dump_obss(results->buf, survey);

So, the allocation size is big enough for the struct, and nothing
depends on the struct size.

Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>

-- 
Kees Cook