Received-SPF: pass (google.com: domain of linux-wireless+bounces-2595-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <fa71e57c-e50f-4538-afa8-9779f3aebde0@gmail.com>
Date: Fri, 26 Jan 2024 21:19:28 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] brcmfmac: replace one-element array with
 flexible-array member
Content-Language: en-US
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>, Kees Cook <keescook@chromium.org>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>, aspriel@gmail.com,
 franky.lin@broadcom.com, kvalo@kernel.org, linux-wireless@vger.kernel.org,
 brcm80211-dev-list.pdl@broadcom.com,
 linux-kernel-mentees@lists.linuxfoundation.org
References: <20230803052238.12147-1-rauji.raut@gmail.com>
 <20230803052238.12147-2-rauji.raut@gmail.com>
 <f739cc15-dade-4f69-a17a-30242026c099@broadcom.com>
 <202401181217.1D75C32E@keescook>
 <CAKXUXMzjGbO0U0pko1rg9=q3j6xS5rbzmLnMPeT3g9fMhBr0hw@mail.gmail.com>
From: Atul Raut <rauji.raut@gmail.com>
In-Reply-To: <CAKXUXMzjGbO0U0pko1rg9=q3j6xS5rbzmLnMPeT3g9fMhBr0hw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

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