Re: [PATCH V2 4/8] phy: tegra: xusb: Use devm_clk_get_optional to fetch USB2 tracking clock

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Double free of the pad structure on the `tegra_xusb_pad_init()` error path.
- [High] Improper use of `device_unregister()` for an un-added device causes a NULL pointer dereference.
--

commit 0aeed6fcdb21e8c89c88a750c7002793b60d83d1
Author: Sing-Han Chen <singhanc@nvidia.com>

phy: tegra: xusb: Use devm_clk_get_optional to fetch USB2 tracking clock

This commit updates the USB2 PAD tracking clock fetching to use
devm_clk_get_optional, allowing the probe to continue if the optional
tracking clock is missing in the devicetree.

> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
> --- a/drivers/phy/tegra/xusb-tegra186.c
> +++ b/drivers/phy/tegra/xusb-tegra186.c
> @@ -1045,7 +1045,7 @@ tegra186_usb2_pad_probe(struct tegra_xusb_padctl *padctl,
>  		goto out;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the error
handling around this initialization, I noticed a couple of pre-existing
issues in the failure path of tegra_xusb_pad_init() just above this change.

If dev_set_name() or device_add() fails in tegra_xusb_pad_init(), it jumps
to the unregister label and calls device_unregister(&pad->dev).

First, does calling device_unregister() on a device that was never fully 
added cause a NULL pointer dereference? Since device_add() failed or wasn't
called, dev->p would be NULL. When device_del() is called internally, it
attempts to access dev->p->knode_parent, which seems like it would crash.
Should this strictly use put_device(&pad->dev) instead?

Second, if tegra_xusb_pad_init() fails, calling device_unregister() drops
the reference count to 0, which triggers tegra_xusb_pad_release() and its
associated pad-specific remove callback, executing the first free. However,
when the error is returned here to tegra186_usb2_pad_probe(), the code
explicitly calls kfree(usb2). Could this result in a double-free of the
usb2 structure?

> -	priv->usb2_trk_clk = devm_clk_get(&pad->dev, "trk");
> +	priv->usb2_trk_clk = devm_clk_get_optional(&pad->dev, "trk");
>  	if (IS_ERR(priv->usb2_trk_clk)) {
>  		err = PTR_ERR(priv->usb2_trk_clk);
>  		dev_dbg(&pad->dev, "failed to get usb2 trk clock: %d\n", err);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831185835.3732707-1-waynec@nvidia.com?part=4