Re: [PATCH] wifi: brcmfmac: Fix memory leak in brcmf_sdio_read_control()

On 03/08/2026 07:44, Arend van Spriel wrote:
> Op 2 augustus 2026 18:44:40 schreef Johannes Berg 
> <[email protected]>:
> 
>> On Sun, 2026-08-02 at 18:41 +0200, Johannes Berg wrote:
>>> On Sun, 2026-08-02 at 10:38 +0200, Arend van Spriel wrote:
>>>> On Wed, 22 Jul 2026 16:37:12 +0530, Abdun Nihaal 
>>>> <[email protected]> wrote:
>>>>> The memory allocated for buf is not freed in some of the error 
>>>>> paths in
>>>>> brcmf_sdio_read_control(). Fix that by adding vfree() calls.
>>>>>
>>>>> Fixes: dd43a01c5cdb ("brcmfmac: use dynamically allocated control 
>>>>> frame buffer")
>>>>> Cc: [email protected]
>>>>> Signed-off-by: Abdun Nihaal <[email protected]>
>>>>> ---
>>>>> Compile tested only. Issue found using static analysis.
>>>>>
>>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 3 +++
>>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> To be applied to wireless tree.
>>>
>>> I'm going to put this into wireless-next, at this point (-rc6) this just
>>> doesn't cut it, the commit message makes it sound purely theoretical.
>>
>> Actually, I'm not even going to do that - sprinkling vfree()s over the
>> code when moving the one that exists would be sufficient is, and I'm
>> being generous here, not a good change.
> 
> Sorry, Johannes
> 
> I looked at moving the vfree() to the done: label, but the code also 
> gets there in the success path to wakeup waiters. In the success path 
> the buf is stored for further processing so it should not be freed. 
> Maybe better to do:
> 
>         spin_unlock_bh(&bus->rxctl_lock);
>         brcmf_sdio_dcmd_resp_wake(bus);
>         return;
> 
> fail:
>         vfree(buf);
>         /* Awake any waiters */
>         brcmf_sdio_dcmd_resp_wake(bus);
> }
Actually it turns out brcmf_sdio_read_control() is only called once so 
the brcmf_sdio_dcmd_resp_wake() can be taken outside the function.

Gr. AvS