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

On Mon, 2026-08-03 at 10:21 +0200, Arend van Spriel wrote:
> >=20
> > I looked at moving the vfree() to the done: label, but the code also=
=20
> > gets there in the success path to wakeup waiters. In the success path=
=20
> > the buf is stored for further processing so it should not be freed.=20

Hah, my bad, sorry!

> > Maybe better to do:
> >=20
> >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 spin_unlock_bh(&bus->rxctl_lock);
> >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 brcmf_sdio_dcmd_resp_wake(bus);
> >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return;
> >=20
> > fail:
> >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 vfree(buf);
> >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Awake any waiters */
> >  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 brcmf_sdio_dcmd_resp_wake(bus);
> > }
> Actually it turns out brcmf_sdio_read_control() is only called once so=
=20
> the brcmf_sdio_dcmd_resp_wake() can be taken outside the function.

Or just something like this?

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1827,15 +1827,17 @@ brcmf_sdio_read_control(struct brcmf_sdio *bus, u8 =
*hdr, uint len, uint doff)
        if (bus->rxctl) {
                brcmf_err("last control frame is being processed.\n");
                spin_unlock_bh(&bus->rxctl_lock);
-               vfree(buf);
                goto done;
        }
        bus->rxctl =3D buf + doff;
        bus->rxctl_orig =3D buf;
        bus->rxlen =3D len - doff;
        spin_unlock_bh(&bus->rxctl_lock);
+       /* buffer is queued */
+       buf =3D NULL;
=20
 done:
+       vfree(buf);
        /* Awake any waiters */
        brcmf_sdio_dcmd_resp_wake(bus);
 }


It's already using vmalloc() so seems it wouldn't care that much about
calling vfree(NULL) on the success path?

johannes