Re: [PATCH v3] usb: gadget: udc: Fix use-after-free in gadget_match_driver

[Alan Stern]

On Thu, Jun 25, 2026 at 03:37:04PM +0800, Jimmy Hu wrote:
> The udc structure acts as the management structure for the gadget,
> but their lifecycles are decoupled. A race condition exists where
> usb_del_gadget() frees the udc memory (e.g., via mode-switch work)
> while gadget_match_driver() concurrently accesses the freed udc memory
> (e.g., via configfs), causing a Use-After-Free (UAF) that triggers a
> NULL pointer dereference when the freed memory is zeroed:
> 
> [39430.908615][ T1171] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [39430.911397][ T1171] pc : __pi_strcmp+0x20/0x140
> [39430.911441][ T1171] lr : gadget_match_driver+0x34/0x60
> ...
> [39430.911890][ T1171]  usb_gadget_register_driver_owner+0x50/0xf8
> [39430.911910][ T1171]  gadget_dev_desc_UDC_store+0xf4/0x140
> [39430.931308][ T1171]  configfs_write_iter+0xec/0x134
> 
> [39430.957058][ T1171] Workqueue: events_freezable __dwc3_set_mode
> [39430.957287][ T1171]  dwc3_gadget_exit+0x34/0x8c
> [39430.957304][ T1171]  __dwc3_set_mode+0xc0/0x664
> 
> Fix this by ensuring the udc structure remains allocated until the
> gadget is released. To achieve this, introduce a new
> usb_gadget_release() routine to the core. When the gadget is added,
> usb_add_gadget() stores the gadget's release routine in the udc
> structure and takes a reference to the udc. When the gadget is
> released, usb_gadget_release() drops the reference to the udc and
> then calls the gadget's release routine.
> 
> Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Jimmy Hu <hhhuuu@xxxxxxxxxx>
> ---

Looks good to me.

Reviewed-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

Alan Stern

> V2 -> V3:
> - Fix column alignment in struct usb_udc.
> - Remove redundant NULL check in usb_gadget_release().
> - Add comments explaining the lifecycle override and error path cleanup.
> 
> V1 -> V2: Rework the fix using a new release routine in the core.
> 
> v2: https://lore.kernel.org/all/20260624030154.393004-1-hhhuuu@xxxxxxxxxx/
> v1: https://lore.kernel.org/all/20260526070635.839701-1-hhhuuu@xxxxxxxxxx/
> 
>  drivers/usb/gadget/udc/core.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 60340ff9edbf..f6da12b553a0 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -31,8 +31,9 @@ static const struct bus_type gadget_bus_type;
>  /**
>   * struct usb_udc - describes one usb device controller
>   * @driver: the gadget driver pointer. For use by the class code
> - * @dev: the child device to the actual controller
>   * @gadget: the gadget. For use by the class code
> + * @gadget_release: the gadget's release routine
> + * @dev: the child device to the actual controller
>   * @list: for use by the udc class driver
>   * @vbus: for udcs who care about vbus status, this value is real vbus status;
>   * for udcs who do not care about vbus status, this value is always true
> @@ -53,6 +54,7 @@ static const struct bus_type gadget_bus_type;
>  struct usb_udc {
>  	struct usb_gadget_driver	*driver;
>  	struct usb_gadget		*gadget;
> +	void				(*gadget_release)(struct device *dev);
>  	struct device			dev;
>  	struct list_head		list;
>  	bool				vbus;
> @@ -1362,6 +1364,17 @@ static void usb_udc_nop_release(struct device *dev)
>  	dev_vdbg(dev, "%s\n", __func__);
>  }
>  
> +static void usb_gadget_release(struct device *dev)
> +{
> +	struct usb_gadget *gadget = dev_to_usb_gadget(dev);
> +	struct usb_udc *udc = gadget->udc;
> +	/* Cache the gadget's release routine to prevent UAF */
> +	void (*release)(struct device *dev) = udc->gadget_release;
> +
> +	put_device(&udc->dev);
> +	release(dev);
> +}
> +
>  /**
>   * usb_initialize_gadget - initialize a gadget and its embedded struct device
>   * @parent: the parent device to this udc. Usually the controller driver's
> @@ -1418,6 +1431,14 @@ int usb_add_gadget(struct usb_gadget *gadget)
>  	mutex_init(&udc->connect_lock);
>  
>  	udc->started = false;
> +	/*
> +	 * Align decoupled lifecycles: take a UDC reference to ensure it
> +	 * remains allocated until the gadget is released, requiring an
> +	 * override of the gadget's release routine to drop it.
> +	 */
> +	udc->gadget_release = gadget->dev.release;
> +	gadget->dev.release = usb_gadget_release;
> +	get_device(&udc->dev);
>  
>  	mutex_lock(&udc_lock);
>  	list_add_tail(&udc->list, &udc_list);
> @@ -1462,6 +1483,12 @@ int usb_add_gadget(struct usb_gadget *gadget)
>  	mutex_lock(&udc_lock);
>  	list_del(&udc->list);
>  	mutex_unlock(&udc_lock);
> +	/*
> +	 * Revert the override and drop the UDC reference to prevent
> +	 * leaking the UDC if the gadget was statically allocated.
> +	 */
> +	gadget->dev.release = udc->gadget_release;
> +	put_device(&udc->dev);
>  
>   err_put_udc:
>  	put_device(&udc->dev);
> 
> base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
> -- 
> 2.55.0.rc0.799.gd6f94ed593-goog
>