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

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

Op 2 augustus 2026 18:44:40 schreef Johannes Berg <johannes@sipsolutions.net>:

> 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 <nihaal@cse.iitm.ac.in> 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: stable@vger.kernel.org
>>>> Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
>>>> ---
>>>> 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