mmc: vub300: fix use-after-free in vub300 teardown

PATCH Superseded Adriano Cordova 2 participants 5 messages Sep 7 – Sep 8

Subsystems (2)

Stable branch MMC, SD and SDIO

lore

Share & tools

lore

Adriano Cordova

author

2026-09-07 15:14 UTC #0 lore raw

PATCH 1/1 mmc: vub300: fix use-after-free in vub300 teardown

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 [email protected]

Link https://syzkaller.appspot.com/bug?extid=f312381a95cc080992fd

Cc [email protected]

Signed-off-by Adriano Cordova <[email protected]>

---
 drivers/mmc/host/vub300.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

drivers/mmc/host/vub300.c +4 -3

index 2dae474dcd06..a1a6aa1aafdb 100644
@@ -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;

Signature · 1 line

2.51.0

Adriano Córdova

author

2026-09-08 13:28 UTC #2 lore raw

Quoted 31 lines · Johan Hovold

Quoted 4 lines · 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")
This commit reverted a buggy change so if anything is broken here, this isn't the commit to blame.
Quoted 4 lines
Reported-by [email protected]
Link https://syzkaller.appspot.com/bug?extid=f312381a95cc080992fd
Cc [email protected]
Signed-off-by Adriano Cordova <[email protected]>
Are you missing an Assisted-by tag?
Quoted 14 lines
---
 drivers/mmc/host/vub300.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/mmc/host/vub300.c +1 -0
index 2dae474dcd06..a1a6aa1aafdb 100644
@@ -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;
 

Quoted 10 lines

drivers/mmc/host/vub300.c static void vub300_delete(struct kref *kref) · line 380
Quoted 7 lines
drivers/mmc/host/vub300.c +1 -1
      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.

Quoted 27 lines

drivers/mmc/host/vub300.c static void vub300_cmndwork_thread(struct work_struct *work) · line 1799

Quoted 11 lines

drivers/mmc/host/vub300.c +1 -1

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

drivers/mmc/host/vub300.c static void vub300_mmc_request(struct mmc_host *mmc, struct mmc_request *req) · line 1951

Quoted 10 lines

drivers/mmc/host/vub300.c +1 -1

                      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.

Quoted 2 lines

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.

Quoted 6 lines

drivers/mmc/host/vub300.c static void vub300_mmc_request(struct mmc_host *mmc, struct mmc_request *req) · line 1954
Quoted 4 lines
drivers/mmc/host/vub300.c +0 -0
                      return;
              } else {
                      vub300->cmd = cmd;
Johan

Best,
Adriano

Johan Hovold

2026-09-08 13:52 UTC #3 lore raw

Quoted 11 lines · Adriano Córdova

Quoted 10 lines · Johan Hovold
Quoted 4 lines · 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")
This commit reverted a buggy change so if anything is broken here, this isn't the commit to blame.

Quoted 6 lines

Quoted 6 lines
Quoted 4 lines
Reported-by [email protected]
Link https://syzkaller.appspot.com/bug?extid=f312381a95cc080992fd
Cc [email protected]
Signed-off-by Adriano Cordova <[email protected]>
Are you missing an Assisted-by tag?

Again, are you using an LLM without disclosing it?

Quoted 25 lines

Quoted 14 lines

---
 drivers/mmc/host/vub300.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

drivers/mmc/host/vub300.c +1 -0

index 2dae474dcd06..a1a6aa1aafdb 100644
@@ -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;

Quoted 10 lines

drivers/mmc/host/vub300.c static void vub300_delete(struct kref *kref) · line 380
Quoted 7 lines
drivers/mmc/host/vub300.c +1 -1
      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().

Quoted 8 lines

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.

Quoted 26 lines

drivers/mmc/host/vub300.c static void vub300_cmndwork_thread(struct work_struct *work) · line 1799

Quoted 10 lines

drivers/mmc/host/vub300.c +1 -1

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

drivers/mmc/host/vub300.c static void vub300_mmc_request(struct mmc_host *mmc, struct mmc_request *req) · line 1951

Quoted 10 lines

drivers/mmc/host/vub300.c +1 -1

                      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.

Quoted 5 lines

Quoted 2 lines
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.

Quoted 4 lines

Quoted 4 lines
drivers/mmc/host/vub300.c static void vub300_mmc_request(struct mmc_host *mmc, struct mmc_request *req) · line 1954
Quoted 4 lines
drivers/mmc/host/vub300.c +0 -0
                      return;
              } else {
                      vub300->cmd = cmd;

Johan

Adriano Córdova

author

2026-09-08 15:08 UTC #4 lore raw

Quoted 13 lines · Johan Hovold

Quoted 11 lines · Adriano Córdova
Quoted 10 lines · Johan Hovold
Quoted 4 lines · 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")
This commit reverted a buggy change so if anything is broken here, this isn't the commit to blame.

Quoted 9 lines

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Reported-by [email protected]
Link https://syzkaller.appspot.com/bug?extid=f312381a95cc080992fd
Cc [email protected]
Signed-off-by Adriano Cordova <[email protected]>
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.

Quoted 30 lines

Quoted 30 lines
Quoted 14 lines
Quoted 14 lines
---
 drivers/mmc/host/vub300.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/mmc/host/vub300.c +1 -0
index 2dae474dcd06..a1a6aa1aafdb 100644
@@ -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;
 
Quoted 10 lines
drivers/mmc/host/vub300.c static void vub300_delete(struct kref *kref) · line 380
Quoted 7 lines
drivers/mmc/host/vub300.c +1 -1
      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.

Quoted 6 lines

Quoted 3 lines
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

Quoted 44 lines

And your LLM didn't put any of this in the commit message.
Quoted 26 lines
Quoted 26 lines
drivers/mmc/host/vub300.c static void vub300_cmndwork_thread(struct work_struct *work) · line 1799
Quoted 10 lines
drivers/mmc/host/vub300.c +1 -1
      /*
       * 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.
drivers/mmc/host/vub300.c static void vub300_mmc_request(struct mmc_host *mmc, struct mmc_request *req) · line 1951
Quoted 10 lines
drivers/mmc/host/vub300.c +1 -1
                      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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 1 line
Quoted 1 line

Quoted 5 lines

Quoted 3 lines
Quoted 3 lines
drivers/mmc/host/vub300.c static void vub300_mmc_request(struct mmc_host *mmc, struct mmc_request *req) · line 1954
Quoted 3 lines
drivers/mmc/host/vub300.c +0 -0
                      return;
              } else {
                      vub300->cmd = cmd;
Johan

Adriano