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

From: sashiko-bot@kernel.org
To: "Wayne Chang" <waynec@nvidia.com>
Cc: vkoul@kernel.org, linux-phy@lists.infradead.org,
	olteanv@gmail.com, conor+dt@kernel.org, robh@kernel.org,
	devicetree@vger.kernel.org, neil.armstrong@linaro.org
Subject: Re: [PATCH V2 4/8] phy: tegra: xusb: Use devm_clk_get_optional to fetch USB2 tracking clock
Date: Mon, 31 Aug 2026 21:38:31 +0000	[thread overview]
Message-ID: <20260831213832.2BA3E1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260831185835.3732707-5-waynec@nvidia.com>

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy