Re: [PATCH] mmc: vub300: fix use-after-free in vub300_delete()

Re: [PATCH] mmc: vub300: fix use-after-free in vub300_delete()

[syzbot]

> #syz test

This crash does not have a reproducer. I cannot test it.

> mmc_free_host() drops the last reference to the mmc_host, which causes
> mmc_host_classdev_release() to run. That release dereferences
> host->parent->of_node, and host->parent points at the parent usb_device.
>
> vub300_delete() currently calls usb_put_dev() before mmc_free_host(), so
> the usb_device is freed while mmc_host_classdev_release() is still
> reading it, leading to a slab use-after-free.
>
> Save the usb_device pointer and release the mmc host before dropping the
> last usb device reference. The probe-error path (err_free_host) already
> does this; apply the same ordering here.
>
> Fixes: 8f4d20a71022 ("mmc: vub300: fix use-after-free on disconnect")
> ---
>  drivers/mmc/host/vub300.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
> index 2dae474dcd06..5000f694cb0e 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
> -- 
> 2.51.0
>

Re: [PATCH] mmc: vub300: fix use-after-free in vub300_delete()

[syzbot]

> #syz test https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing

This crash does not have a reproducer. I cannot test it.

> mmc_free_host() drops the last reference to the mmc_host, which causes
> mmc_host_classdev_release() to run. That release dereferences
> host->parent->of_node, and host->parent points at the parent usb_device.
>
> vub300_delete() currently calls usb_put_dev() before mmc_free_host(), so
> the usb_device is freed while mmc_host_classdev_release() is still
> reading it, leading to a slab use-after-free.
>
> Save the usb_device pointer and release the mmc host before dropping the
> last usb device reference. The probe-error path (err_free_host) already
> does this; apply the same ordering here.
>
> Fixes: 8f4d20a71022 ("mmc: vub300: fix use-after-free on disconnect")
> ---
>  drivers/mmc/host/vub300.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
> index 2dae474dcd06..5000f694cb0e 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
> -- 
> 2.51.0
>