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 org.kernel.vger.stable,dev.linux.lists.brcm80211,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless
Message-ID <[email protected]>
On Mon, 2026-08-03 at 10:54 +0200, Arend van Spriel wrote:
> > 
> > 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 = buf + doff;
> >          bus->rxctl_orig = buf;
> >          bus->rxlen = len - doff;
> >          spin_unlock_bh(&bus->rxctl_lock);
> > +       /* buffer is queued */
> > +       buf = NULL;
> >   
> >   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?
> True. And yes, the existing vfree() should go, but I figured that goes 
> without saying so I did not mention that. I am fine with your proposal, 
> 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