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

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

[Adriano Cordova]

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")
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>
---
 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);
 	/*
 	 * 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);
 			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);
 			return;
 		} else {
 			vub300->cmd = cmd;
-- 
2.51.0

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

[Johan Hovold]

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?

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

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

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

Johan

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

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

[Johan Hovold]

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

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@kernel.org>) 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@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?
>

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