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

[Adriano Córdova]

El mar, 8 sept 2026 a las 10:52, Johan Hovold (<johan@xxxxxxxxxx>) escribió:
>
> 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@xxxxxxxxxx>) 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > Link: https://syzkaller.appspot.com/bug?extid=f312381a95cc080992fd
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Signed-off-by: Adriano Cordova <adrianox@xxxxxxxxx>
> > >
> > > Are you missing an Assisted-by tag?
>
> Again, are you using an LLM without disclosing it?
>

Yes, I will add the tag in the v2.

> > > > ---
> > > >  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.

I understand, you are right. I will send a v2

>
> 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

Adriano