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@xxxxxxxxxx
> Signed-off-by: Haoxiang Li <haoxiang_li2024@xxxxxxx>
> ---
>  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