Re: [PATCH 5/5] usb: dwc3: rockchip: fix USB-C reconnect in gadget mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: dwc3: missing runtime PM resume in role change callback
- [High] usb: dwc3: use-after-free in dwc3_rockchip_vbus_handler due to lockless access
--

commit 58b7c3d4ec9b041b0b40be86929f872c8f0552a7
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

usb: dwc3: rockchip: fix USB-C reconnect in gadget mode

This commit fixes a gadget mode reconnection issue by updating the connection
status and triggering a runtime PM resume when the USB role changes. The
runtime PM reference counter is then immediately dropped so that auto-suspend
can handle keeping the device awake.

> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
> index 7bdd6e2eb22d1..bedec3295bb16 100644
> --- a/drivers/usb/dwc3/dwc3-rockchip.c
> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
> @@ -25,8 +25,17 @@ struct dwc3_rockchip {
>  	struct dwc3		dwc;
>  	struct dwc3_rk_phy_nb	usb3_phy_nb[DWC3_USB3_MAX_PORTS];
>  	u8			phy_reset_active;
> +	enum usb_role		role;
>  };
>  
> +static void dwc3_rockchip_vbus_handler(struct dwc3 *dwc, bool present)
> +{
> +	if (!dwc->gadget || !dwc->gadget_driver)
> +		return;
> +
> +	usb_udc_vbus_handler(dwc->gadget, present);

[Severity: High]
Could this lead to a use-after-free if it races with a gadget unbind?

Because this locklessly checks and dereferences dwc->gadget, and can be
called asynchronously from the PHY notifier (dwc3_usb3_phy_notify), it seems
possible that dwc->gadget could be freed by dwc3_gadget_exit() right after
the if-check passes but before usb_udc_vbus_handler() executes.

[ ... ]
> @@ -130,7 +144,16 @@ static int dwc3_rk_phy_register_notifiers(struct dwc3 *dwc)
>  	return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
>  }
>  
> +static void dwc3_rockchip_set_role(struct dwc3 *dwc, enum usb_role role)
> +{
> +	struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
> +
> +	dwc_rk->role = role;
> +	dwc3_rockchip_vbus_handler(dwc, role == USB_ROLE_DEVICE);
> +}

[Severity: High]
Were the runtime PM calls accidentally omitted here?

The commit message states the fix involves triggering a runtime PM resume
when the role changes, and immediately decreasing the reference counter
afterward. However, there are no calls to pm_runtime_get_sync() or
pm_runtime_put_autosuspend() in this callback. Without them, it seems the
USB controller would incorrectly remain suspended and the original issue
might persist.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-b4-rockchip-dwc3-rockchip-glue-v1-0-763bb546824e@collabora.com?part=5

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy