Re: [PATCH v8 phy-next 16/31] usb: host: tegra: avoid direct dereference of phy->dev.of_node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] This problem wasn't introduced by this patch, but a single `struct notifier_block` instance (`tegra->id_nb`) is registered to multiple USB PHY instances in a loop, causing linked list corruption.
- [High] This problem wasn't introduced by this patch, but the devm-managed USB PHY notifier unregistration happens after driver remove, allowing `tegra->id_work` to be scheduled after it is canceled.
--

commit dc45b6840f6afd2c371e491c07e3f59d5e2f3937
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

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

This commit modifies the Tegra XHCI driver to use a parallel lookup path
using the 'phys' phandle to obtain the OF node, avoiding direct
dereferencing of phy->dev.of_node as the generic PHY subsystem moves to
opaque pointers.

> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index d2214d309e965..6724a777ca2ac 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1504,23 +1505,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++) {

[ ... ]

> +		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);

This problem wasn't introduced by this patch, but does passing the same
&tegra->id_nb instance to multiple USB PHYs in this loop cause a linked list
corruption?

A struct notifier_block has a single next pointer used to maintain the linked
list. Registering the exact same block to multiple different PHY notifier
chains simultaneously could overwrite its next pointer, cross-linking and
corrupting the notifier lists.

Additionally, is there a risk of a use-after-free during driver removal due
to how this notifier is managed?

Because devm_usb_get_phy_by_node() uses devres to manage the PHY and its
notifier block, devres cleanup (which unregisters the notifier) runs after
the driver's remove() function returns.

If the notifier callback tegra_xhci_id_notify() schedules tegra->id_work,
could a hardware event fire after cancel_work_sync(&tegra->id_work) is
called in remove(), but before devres unregisters the notifier?

If so, the still-registered callback might execute and re-queue
tegra->id_work on freed memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505100523.1922388-1-vladimir.oltean@nxp.com?part=16