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

Johannes Berg <[email protected]> Mon, 03 Aug 2026 10:57:08 +0200
Newsgroups dev.linux.lists.brcm80211,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless,org.kernel.vger.stable
Message-ID <[email protected]>
On Mon, 2026-08-03 at 10:54 +0200, Arend van Spriel wrote:
> >=20
> > Or just something like this?
> >=20
> > --- 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);
> >   }
> >=20
> >=20
> > It's already using vmalloc() so seems it wouldn't care that much about
> > calling vfree(NULL) on the success path?
> True. And yes, the existing vfree() should go, but I figured that goes=
=20
> without saying so I did not mention that. I am fine with your proposal,=
=20
> but I guess the author will have to submit a v2 patch, right?

I wasn't going to do it myself, but I guess I don't care who does :)

johannes