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

[Adriano Córdova]

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

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

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

Best,
Adriano