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

[Arend Van Spriel <arend.vanspriel@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]

On January 18, 2024 9:19:52 PM Kees Cook <keescook@chromium.org> wrote:

> 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@gmail.com>
>>> ---
>>> 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@chromium.org>

Thanks for the confirmation.

Regards,
Arend

> --
> Kees Cook




[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]