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

[Ulf Hansson]

On Fri, 27 Mar 2026 at 11:52, Johan Hovold <johan@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx      # 6.17
> Cc: Binbin Zhou <zhoubinbin@xxxxxxxxxxx>
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
>  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
>