Re: [PATCH 2/4] mmc: vub300: fix use-after-free on disconnect

Ulf Hansson
2026-03-31T10:25:27Z

On Fri, 27 Mar 2026 at 11:52, Johan Hovold <johan@kernel.org> wrote:
>
> The vub300 driver maintains an explicit reference count for the
> controller and its driver data and the last reference can in theory be
> dropped after the driver has been unbound.
>
> This specifically means that the controller allocation must not be
> device managed as that can lead to use-after-free.
>
> Note that the lifetime is currently also incorrectly tied the parent USB
> device rather than interface, which can lead to memory leaks if the
> driver is unbound without its device being physically disconnected (e.g.
> on probe deferral).
>
> Fix both issues by reverting to non-managed allocation of the controller.

Huh, sounds like a real mess, but thanks for the detailed description.

I will defer applying until we finalize the discussion on patch1, but
otherwise this looks good to me.

Kind regards
Uffe

>
> Fixes: dcfdd698dc52 ("mmc: vub300: Use devm_mmc_alloc_host() helper")
> Cc: stable@vger.kernel.org # 6.17
> Cc: Binbin Zhou <zhoubinbin@loongson.cn>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
> drivers/mmc/host/vub300.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
> index f173c7cf4e1a..3c9df27f9fa7 100644
> --- a/drivers/mmc/host/vub300.c
> +++ b/drivers/mmc/host/vub300.c
> @@ -369,11 +369,14 @@ struct vub300_mmc_host {
> 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;
> +
> 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);
> /*
> * and hence also frees vub300
> * which is contained at the end of struct mmc
> @@ -2112,7 +2115,7 @@ static int vub300_probe(struct usb_interface *interface,
> goto error1;
> }
> /* this also allocates memory for our VUB300 mmc host device */
> - mmc = devm_mmc_alloc_host(&udev->dev, sizeof(*vub300));
> + mmc = mmc_alloc_host(sizeof(*vub300), &udev->dev);
> if (!mmc) {
> retval = -ENOMEM;
> dev_err(&udev->dev, "not enough memory for the mmc_host\n");
> @@ -2269,7 +2272,7 @@ static int vub300_probe(struct usb_interface *interface,
> dev_err(&vub300->udev->dev,
> "Could not find two sets of bulk-in/out endpoint pairs\n");
> retval = -EINVAL;
> - goto error4;
> + goto err_free_host;
> }
> retval =
> usb_control_msg(vub300->udev, usb_rcvctrlpipe(vub300->udev, 0),
> @@ -2278,14 +2281,14 @@ static int vub300_probe(struct usb_interface *interface,
> 0x0000, 0x0000, &vub300->hc_info,
> sizeof(vub300->hc_info), 1000);
> if (retval < 0)
> - goto error4;
> + goto err_free_host;
> retval =
> usb_control_msg(vub300->udev, usb_sndctrlpipe(vub300->udev, 0),
> SET_ROM_WAIT_STATES,
> USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> firmware_rom_wait_states, 0x0000, NULL, 0, 1000);
> if (retval < 0)
> - goto error4;
> + goto err_free_host;
> dev_info(&vub300->udev->dev,
> "operating_mode = %s %s %d MHz %s %d byte USB packets\n",
> (mmc->caps & MMC_CAP_SDIO_IRQ) ? "IRQs" : "POLL",
> @@ -2300,7 +2303,7 @@ static int vub300_probe(struct usb_interface *interface,
> 0x0000, 0x0000, &vub300->system_port_status,
> sizeof(vub300->system_port_status), 1000);
> if (retval < 0) {
> - goto error4;
> + goto err_free_host;
> } else if (sizeof(vub300->system_port_status) == retval) {
> vub300->card_present =
> (0x0001 & vub300->system_port_status.port_flags) ? 1 : 0;
> @@ -2308,7 +2311,7 @@ static int vub300_probe(struct usb_interface *interface,
> (0x0010 & vub300->system_port_status.port_flags) ? 1 : 0;
> } else {
> retval = -EINVAL;
> - goto error4;
> + goto err_free_host;
> }
> usb_set_intfdata(interface, vub300);
> INIT_DELAYED_WORK(&vub300->pollwork, vub300_pollwork_thread);
> @@ -2338,6 +2341,8 @@ static int vub300_probe(struct usb_interface *interface,
> return 0;
> error6:
> timer_delete_sync(&vub300->inactivity_timer);
> +err_free_host:
> + mmc_free_host(mmc);
> /*
> * and hence also frees vub300
> * which is contained at the end of struct mmc
> --
> 2.52.0
>