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

[Atul Raut <rauji.raut@gmail.com>]

Thank You Kees,

Hello Lukas, Arend & Kalle,

Sorry for not getting back to you sooner, am out of county &

hence unable to push the patch right away.

Will share once back, apologize for inconvenience caused.

-Atul


On 1/19/24 05:00, Lukas Bulwahn wrote:
> On Thu, Jan 18, 2024 at 9:20 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>
>>
> As the linux-kernel-mentees mailing list is in CC, I assume that Atul
> is a linux-kernel mentee. In that case, it is certainly reasonable and
> at least of educational value for Atul to take Kees' explanation and
> add this to the patch's commit message in his own words. Then, with
> that, submit a PATCH v3.
>
>
> Lukas