Re: [PATCH v6 04/23] phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: conor+dt@kernel.org, vkoul@kernel.org,
	devicetree@vger.kernel.org, olteanv@gmail.com,
	neil.armstrong@linaro.org, linux-phy@lists.infradead.org,
	robh@kernel.org
Subject: Re: [PATCH v6 04/23] phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check
Date: Fri, 19 Jun 2026 15:43:48 +0000	[thread overview]
Message-ID: <20260619154349.071321F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260619-rockchip-usbdp-cleanup-v6-4-3bb1f54b3f35@collabora.com>

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

Pre-existing issues:
- [High] The device tree parsing logic is vulnerable to a NULL pointer dereference if 'clock-names' is missing or incomplete.
--

commit 5e5a7a4f6db0a44a4bac43001ea4d179c3798c6d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check

This commit fixes the return value check for devm_clk_bulk_get_all()
so that deferred probe errors are properly propagated instead of being
masked.

Link: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 12dfdd470a782..2845a012eafcc 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -427,8 +427,8 @@ static int rk_udphy_clk_init(struct rk_udphy *udphy, struct device *dev)
>  	int i;
>  
>  	udphy->num_clks = devm_clk_bulk_get_all(dev, &udphy->clks);
> -	if (udphy->num_clks < 1)
> -		return -ENODEV;
> +	if (udphy->num_clks < 0)
> +		return udphy->num_clks;
>  
>  	/* used for configure phy reference clock frequency */
>  	for (i = 0; i < udphy->num_clks; i++) {

[Severity: High]
This is a pre-existing issue, but could this cause a null pointer dereference
if the device tree is missing the 'clock-names' property?

Looking at devm_clk_bulk_get_all(), it relies on of_clk_bulk_get(), which
initializes clks[i].id to a null pointer. If the device tree specifies
'clocks' but omits 'clock-names' (or has fewer names than clocks), the id
field appears to remain null.

Since strncmp() does not safely handle null pointers, passing it directly
might trigger an oops during driver probe:

rk_udphy_clk_init()
  devm_clk_bulk_get_all()
  // udphy->clks[i].id is potentially null
  ...
  strncmp(udphy->clks[i].id, "refclk", 6)

Would it make sense to verify udphy->clks[i].id is valid before comparing it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-rockchip-usbdp-cleanup-v6-0-3bb1f54b3f35@collabora.com?part=4

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