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

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);
}

Gr. AvS