Re: [PATCH] wifi: brcmfmac: Fix memory leak in brcmf_sdio_read_control()
From: Arend van Spriel
Date: Mon Aug 03 2026 - 04:21:15 EST
On 03/08/2026 07:44, Arend van Spriel wrote:
Op 2 augustus 2026 18:44:40 schreef Johannes Berg <johannes@xxxxxxxxxxxxxxxx>: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.
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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
Signed-off-by: Abdun Nihaal <nihaal@xxxxxxxxxxxxxx>
---
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