Re: [PATCH] mmc: vub300: fix use-after-free in vub300 teardown

[Johan Hovold <johan@kernel.org>]

On Tue, Sep 08, 2026 at 10:28:06AM -0300, Adriano Córdova wrote:
> El mar, 8 sept 2026 a las 4:26, Johan Hovold (<johan@kernel.org>) escribió:
> >
> > On Mon, Sep 07, 2026 at 12:14:28PM -0300, Adriano Cordova wrote:
> > > Do not dereference mmc/vub300/udev after the final kref_put, because
> > > it can release them via mmc_free_host()/usb_put_dev().
> > >
> > > Fixes: 8f4d20a71022 ("mmc: vub300: fix use-after-free on disconnect")
> >
> > This commit reverted a buggy change so if anything is broken here, this
> > isn't the commit to blame.
> >
> > > Reported-by: syzbot+f312381a95cc080992fd@syzkaller.appspotmail.com
> > > Link: https://syzkaller.appspot.com/bug?extid=f312381a95cc080992fd
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Adriano Cordova <adrianox@gmail.com>
> >
> > Are you missing an Assisted-by tag?

Again, are you using an LLM without disclosing it?

> > > ---
> > >  drivers/mmc/host/vub300.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
> > > index 2dae474dcd06..a1a6aa1aafdb 100644
> > > --- a/drivers/mmc/host/vub300.c
> > > +++ b/drivers/mmc/host/vub300.c
> > > @@ -370,13 +370,14 @@ static void vub300_delete(struct kref *kref)
> > >  {                            /* kref callback - softirq */
> > >       struct vub300_mmc_host *vub300 = kref_to_vub300_mmc_host(kref);
> > >       struct mmc_host *mmc = vub300->mmc;
> > > +     struct usb_device *udev = vub300->udev;
> > >
> > >       usb_free_urb(vub300->command_out_urb);
> > >       vub300->command_out_urb = NULL;
> > >       usb_free_urb(vub300->command_res_urb);
> > >       vub300->command_res_urb = NULL;
> > > -     usb_put_dev(vub300->udev);
> > >       mmc_free_host(mmc);
> > > +     usb_put_dev(udev);
> >
> > This makes no sense at all as the driver data is freed by
> > mmc_free_host().
> 
> mmc_free_host() only frees mmc (struct mmc_host) and vub300
> (struct vub300_mmc_host), but the udev (struct usb_device, linked
> as mmc->parent) is not freed by mmc_free_host() and has to be
> freed separately.
> 
> But it cannot be freed before mmc_free_host(), because the kobject
> cleanup code of mmc_host accesses it in mmc_host_classdev_release()
> via host->parent->of_node.

That's a bug in MMC core. It needs to hold a reference to the parent, or
just store the id directly. 

And your LLM didn't put any of this in the commit message.

> > >       /*
> > >        * and hence also frees vub300
> > >        * which is contained at the end of struct mmc
> > > @@ -1794,8 +1795,8 @@ static void vub300_cmndwork_thread(struct work_struct *work)
> > >                       construct_request_response(vub300, cmd);
> > >                       vub300->resp_len = 0;
> > >                       mutex_unlock(&vub300->cmd_mutex);
> > > -                     kref_put(&vub300->kref, vub300_delete);
> > >                       mmc_request_done(vub300->mmc, req);
> > > +                     kref_put(&vub300->kref, vub300_delete);
> >
> > This order has been here since the driver was merged.
> >
> > >                       return;
> > >               }
> > >       }
> > > @@ -1946,8 +1947,8 @@ static void vub300_mmc_request(struct mmc_host *mmc, struct mmc_request *req)
> > >                   satisfy_request_from_offloaded_data(vub300, cmd)) {
> > >                       cmd->error = 0;
> > >                       mutex_unlock(&vub300->cmd_mutex);
> > > -                     kref_put(&vub300->kref, vub300_delete);
> > >                       mmc_request_done(mmc, req);
> > > +                     kref_put(&vub300->kref, vub300_delete);
> >
> > Same here.
> >
> > So if this is wrong (it does look suspicious, but this driver is just a
> > mess) then that's the commit to blame.
> 
> Hope is more clear now, it makes sense to me, and I think the Fixes:
> tag is right.

No, these issues were there since the driver was merged, that is, they
were introduced by commit 88095e7b473a ("mmc: Add new VUB300
USB-to-SD/SDIO/MMC driver") in 2011. (And the of_node issue was
introduced in 2021.)

The broken devres change may or may not have masked them for a bit, but
the revert is not the culprit here.

> >
> > >                       return;
> > >               } else {
> > >                       vub300->cmd = cmd;

Johan