[PATCH] usb: gadget: fsl_qe_udc: fix gadget lifetime on registration failure

[PATCH] usb: gadget: fsl_qe_udc: fix gadget lifetime on registration failure

[Haoxiang Li]

usb_add_gadget_udc_release() drops the gadget device reference when
registration fails. This invokes qe_udc_release() while qe_udc_probe()
is still unwinding. Since udc->done is not initialized during probe,
the release callback dereferences NULL in complete(). It also frees
the qe_udc object before the remaining probe cleanup accesses it,
resulting in use-after-free and double-free risks.

Initialize the gadget device explicitly and register it with
usb_add_gadget(), which leaves the gadget reference owned by the
driver on failure. Unwind the IRQ, DMA mappings, endpoint resources
and registers before dropping that reference with usb_put_gadget().
Make the completion notification conditional because it is only
installed by the remove path.

Similarly, use usb_del_gadget() during removal so the final gadget
reference remains held while the controller resources are released.
Set the completion pointer immediately before dropping the reference,
then wait for the release callback to finish.

Fixes: d77c1198666d ("usb: gadget: fsl_qe_udc: convert to new style start/stop")
Cc: stable@kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
 drivers/usb/gadget/udc/fsl_qe_udc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index bf87285ad13c..f9a59b32e272 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -2459,7 +2459,9 @@ static void qe_udc_release(struct device *dev)
 	struct qe_udc *udc = container_of(dev, struct qe_udc, gadget.dev);
 	int i;
 
-	complete(udc->done);
+	if (udc->done)
+		complete(udc->done);
+
 	cpm_muram_free(cpm_muram_offset(udc->ep_param[0]));
 	for (i = 0; i < USB_MAX_ENDPOINTS; i++)
 		udc->ep_param[i] = NULL;
@@ -2489,6 +2491,9 @@ static int qe_udc_probe(struct platform_device *ofdev)
 		return -ENOMEM;
 	}
 
+	usb_initialize_gadget(&ofdev->dev, &udc->gadget,
+						qe_udc_release);
+
 	udc->soc_type = (unsigned long)device_get_match_data(&ofdev->dev);
 	udc->usb_regs = of_iomap(np, 0);
 	if (!udc->usb_regs) {
@@ -2575,8 +2580,7 @@ static int qe_udc_probe(struct platform_device *ofdev)
 		goto err4;
 	}
 
-	ret = usb_add_gadget_udc_release(&ofdev->dev, &udc->gadget,
-			qe_udc_release);
+	ret = usb_add_gadget(&udc->gadget);
 	if (ret)
 		goto err5;
 
@@ -2610,7 +2614,7 @@ static int qe_udc_probe(struct platform_device *ofdev)
 err2:
 	iounmap(udc->usb_regs);
 err1:
-	kfree(udc);
+	usb_put_gadget(&udc->gadget);
 	return ret;
 }
 
@@ -2633,9 +2637,8 @@ static void qe_udc_remove(struct platform_device *ofdev)
 	unsigned int size;
 	DECLARE_COMPLETION_ONSTACK(done);
 
-	usb_del_gadget_udc(&udc->gadget);
+	usb_del_gadget(&udc->gadget);
 
-	udc->done = &done;
 	tasklet_disable(&udc->rx_tasklet);
 
 	if (udc->nullmap) {
@@ -2675,6 +2678,8 @@ static void qe_udc_remove(struct platform_device *ofdev)
 
 	iounmap(udc->usb_regs);
 
+	udc->done = &done;
+	usb_put_gadget(&udc->gadget);
 	/* wait for release() of gadget.dev to free udc */
 	wait_for_completion(&done);
 }
-- 
2.25.1

Re: [PATCH] usb: gadget: fsl_qe_udc: fix gadget lifetime on registration failure

[Greg KH]

On Mon, Jun 22, 2026 at 10:06:10PM +0800, Haoxiang Li wrote:
> usb_add_gadget_udc_release() drops the gadget device reference when
> registration fails. This invokes qe_udc_release() while qe_udc_probe()
> is still unwinding. Since udc->done is not initialized during probe,
> the release callback dereferences NULL in complete(). It also frees
> the qe_udc object before the remaining probe cleanup accesses it,
> resulting in use-after-free and double-free risks.
> 
> Initialize the gadget device explicitly and register it with
> usb_add_gadget(), which leaves the gadget reference owned by the
> driver on failure. Unwind the IRQ, DMA mappings, endpoint resources
> and registers before dropping that reference with usb_put_gadget().
> Make the completion notification conditional because it is only
> installed by the remove path.
> 
> Similarly, use usb_del_gadget() during removal so the final gadget
> reference remains held while the controller resources are released.
> Set the completion pointer immediately before dropping the reference,
> then wait for the release callback to finish.
> 
> Fixes: d77c1198666d ("usb: gadget: fsl_qe_udc: convert to new style start/stop")
> Cc: stable@kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
>  drivers/usb/gadget/udc/fsl_qe_udc.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
> index bf87285ad13c..f9a59b32e272 100644
> --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
> +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
> @@ -2459,7 +2459,9 @@ static void qe_udc_release(struct device *dev)
>  	struct qe_udc *udc = container_of(dev, struct qe_udc, gadget.dev);
>  	int i;
>  
> -	complete(udc->done);
> +	if (udc->done)
> +		complete(udc->done);
> +
>  	cpm_muram_free(cpm_muram_offset(udc->ep_param[0]));
>  	for (i = 0; i < USB_MAX_ENDPOINTS; i++)
>  		udc->ep_param[i] = NULL;
> @@ -2489,6 +2491,9 @@ static int qe_udc_probe(struct platform_device *ofdev)
>  		return -ENOMEM;
>  	}
>  
> +	usb_initialize_gadget(&ofdev->dev, &udc->gadget,
> +						qe_udc_release);

Odd coding style, why not just one line?

thanks,

greg k-h