Re: [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device
From: sashiko-bot@kernel.org
Date: 2026-07-03 11:18:03
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The firmware node reference obtained from `fwnode_graph_get_port_by_id()` is leaked because it is never explicitly released. -- commit 25f9337168757350400d5fa5ab05e87193277473 Author: Chen-Yu Tsai [off-list ref] usb: hub: Associate port@ fwnode with USB port device This commit assigns a firmware node to a USB port device if the hub's firmware node is not an ACPI node. This association allows later M.2 slot power sequencing providers to match against the requesting port.
quoted hunk ↗ jump to hunk
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index b1364f0c384ce..1088776ef750b 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -780,6 +781,13 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1) port_dev->dev.driver = &usb_port_driver; dev_set_name(&port_dev->dev, "%s-port%d", dev_name(&hub->hdev->dev), port1); + /* + * ACPI FW nodes are associated later when device_register() happens. + * Skip assigning one here to avoid potential conflicts. + */ + if (!is_acpi_node(dev_fwnode(&hdev->dev))) + device_set_node(&port_dev->dev, + fwnode_graph_get_port_by_id(dev_fwnode(&hdev->dev), port1));
[Severity: Medium]
Does this code leak the fwnode_handle reference?
fwnode_graph_get_port_by_id() increments the reference count of the returned
firmware node, and the caller is expected to release it.
Since device_set_node() merely assigns dev->fwnode without taking ownership
of the reference, the driver core doesn't automatically drop it when the
device is destroyed.
Looking at usb_port_device_release() in drivers/usb/core/port.c, it
only frees the port structures:
static void usb_port_device_release(struct device *dev)
{
struct usb_port *port_dev = to_usb_port(dev);
kfree(port_dev->req);
kfree(port_dev);
}
Without a corresponding fwnode_handle_put(), will this result in a reference
leak whenever a USB hub is disconnected, or if device_register() fails on the
error path?
mutex_init(&port_dev->status_lock);
retval = device_register(&port_dev->dev);
if (retval) {-- Sashiko AI review · https://sashiko.dev/#/patchset/20260703110317.1283411-1-wenst@chromium.org?part=5