Re: [PATCH v13 05/35] phy: rockchip: usbdp: Handle missing clock-names DT property gracefully

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing NULL pointer checks for clock ID strings cause kernel panics on non-compliant device trees.
--

--- Patch [5]: [PATCH v13 05/35] phy: rockchip: usbdp: Handle missing clock-names DT property gracefully ---
commit 01abe2c3417df3024104ac116d14a930891d0500
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Handle missing clock-names DT property gracefully

The commit fixes a NULL pointer dereference in rk_udphy_clk_init() that
occurs if a device tree has a clocks property but no clock-names
property. It does this by skipping clocks with a NULL id string.

Closes: https://sashiko.dev/#/message/20260619154349.071321F000E9%40smtp.kernel.org

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 2845a012eafcc..3fc8222fcaec2 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -432,6 +432,9 @@ static int rk_udphy_clk_init(struct rk_udphy *udphy, struct device *dev)
>  
>  	/* used for configure phy reference clock frequency */
>  	for (i = 0; i < udphy->num_clks; i++) {
> +		if (!udphy->clks[i].id)
> +			continue;
> +
>  		if (!strncmp(udphy->clks[i].id, "refclk", 6)) {

[Severity: High]
This is a pre-existing issue, but does this same vulnerability exist in other
drivers that parse devm_clk_bulk_get_all() results?

A malformed device tree without clock-names will result in NULL id pointers,
which cause kernel panics when passed directly to strncmp(). This pattern
appears to still be present in several other places:

drivers/phy/rockchip/phy-rockchip-inno-usb2.c:
rockchip_usb2phy_clk480m_register() {
    ...
    if (!strncmp(rphy->clks[i].id, "phyclk", 6)) {
    ...
}

drivers/phy/rockchip/phy-rockchip-naneng-combphy.c:
rockchip_combphy_parse_dt() {
    ...
    if (!strncmp(priv->clks[i].id, "ref", 3)) {
    ...
}

drivers/pci/controller/dwc/pci-imx6.c:imx_pcie_probe() {
    ...
    if (strncmp(imx_pcie->clks[i].id, "extref", 6) == 0)
    ...
}

drivers/pci/controller/dwc/pci-imx6.c:imx_setup_phy_mpll() {
    ...
    if (strncmp(clks[i].id, "pcie_phy", 8) == 0)
    ...
}

Should these drivers be updated with similar NULL checks to prevent
graceful probe failures from becoming unhandled kernel panics?

>  			udphy->refclk = udphy->clks[i].clk;
>  			break;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-rockchip-usbdp-cleanup-v13-0-6cb3e769d4c5@collabora.com?part=5

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