Re: [PATCH v6 10/16] usb: hub: Power on connected M.2 E-key connectors with power sequencing API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition during sysfs unbind leads to Use-After-Free of the `hub->ports` array in `usb_hub_set_port_power()`.
--

--- Patch [10]: [PATCH v6 10/16] usb: hub: Power on connected M.2 E-key connectors with power sequencing API ---
commit 11e3aac46e37825563690ac3ff867ec8a68c9006
Author: Chen-Yu Tsai <wenst@chromium.org>

usb: hub: Power on connected M.2 E-key connectors with power sequencing API

This commit adds support for powering on M.2 E-key connectors over the power
sequencing API. It modifies the port power control functions to retrieve and
toggle power sequencing descriptors. Port devices now manage power sequences
for connected connectors that require it.

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 10f15b40fcc55..298fb68c6afc7 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
[ ... ]
> @@ -887,15 +898,22 @@ static void hub_tt_work(struct work_struct *work)
>  int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
>  			   int port1, bool set)
>  {
> +	struct usb_port *pwrseq_port = hub->ports[port1 - 1];
                                       ^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Can this result in a Use-After-Free of the hub->ports array?

The commit message notes that sysfs callbacks acquire the lock and check for
disconnect, preventing a use-after-free. Is this assumption accurate?

Looking at the disable_store() sysfs callback, it uses
sysfs_break_active_protection(). This breaks active sysfs protection,
causing device_unregister() in the disconnect path to return immediately
without waiting for disable_store() to finish.

If a concurrent hub_disconnect() runs, it does not acquire the hdev lock.
It sets hub->disconnected = 1, unregisters the port devices, and calls
kfree(hub->ports).

If disable_store() has already passed the hub->disconnected check while
holding the lock, could it proceed to call usb_hub_set_port_power()
and dereference the newly freed hub->ports array here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721065413.2306137-1-wenst@chromium.org?part=10