Re: [PATCH v9 2/3] usb: host: tegra: avoid direct dereference of phy->dev.of_node

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Reusing a single `struct notifier_block` (`tegra->id_nb`) across multiple independent USB PHYs corrupts their atomic notifier chains.
- [High] Delayed work can be scheduled after driver removal due to devres-based notifier unregistration.
--

--- Patch [2]: [PATCH v9 2/3] usb: host: tegra: avoid direct dereference of phy->dev.of_node ---
commit 5072858d37310d2ff0794e119a072f8de6f28872
Author: Vladimir Oltean <[email protected]>

usb: host: tegra: avoid direct dereference of phy->dev.of_node

This commit removes direct access to phy->dev.of_node from the Tegra USB
host driver. It implements a parallel lookup path using the 'phys' phandle
to retrieve the OF node. The remote wake debug print is also updated to use
the PHY index instead.

> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1515,23 +1516,34 @@ static int tegra_xusb_init_usb_phy(struct tegra_xusb *tegra)
>  	tegra->otg_usb3_port = -EINVAL;
>  
>  	for (i = 0; i < tegra->num_usb_phys; i++) {
> -		struct phy *phy = tegra_xusb_get_phy(tegra, "usb2", i);
[ ... ]
> +		err = of_parse_phandle_with_args(np, "phys", "#phy-cells",
> +						 index, &args);
> +		if (err)
> +			continue;
> +
> +		usbphy = devm_usb_get_phy_by_node(tegra->dev, args.np,
> +						  &tegra->id_nb);

[Severity: High]
This is a pre-existing issue, but does reusing a single struct notifier_block
(tegra->id_nb) across multiple independent USB PHYs corrupt their atomic
notifier chains?

Since tegra_xusb_init_usb_phy() loops over all available usb2 PHYs, it passes
the exact same &tegra->id_nb notifier block to devm_usb_get_phy_by_node().
usb_register_notifier() then links this same block into each PHY's independent
atomic_notifier_head, which overwrites its next pointer.

Could this lead to missed events or infinite loops when the lists are
traversed?

[Severity: High]
This isn't a bug introduced by this patch, but does the devres lifecycle
for this notifier create a use-after-free risk during driver removal?

In tegra_xusb_remove(), cancel_work_sync(&tegra->id_work) is called to
stop pending work, but the notifier remains registered until the devres
release phase via devm_usb_phy_release2().

If a USB PHY event fires in this window, tegra_xhci_id_notify() calls
schedule_work(). Devres then unregisters the notifier and frees the
tegra_xusb structure, causing the workqueue to dereference the freed
structure when it executes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2