Re: [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double-free of the `child_np` device node reference leading to a Use-After-Free.
- [Medium] Incorrect error check ordering masks `devm_phy_create` failures and attempts bridge registration on invalid PHY pointers.
--

commit 83430d0b883e6ab3238d19fd9d398fef26aa6ab7
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge

Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge
device.

> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index d9701b6106d55..48070b50416e5 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
[ ... ]
> @@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
>  
>  	for_each_available_child_of_node(np, child_np) {
>  		struct phy *phy;
> +		ret = 0;
>  
> -		if (of_node_name_eq(child_np, "dp-port"))
> +		if (of_node_name_eq(child_np, "dp-port")) {
>  			phy = devm_phy_create(dev, child_np,
>  					      &rockchip_dp_phy_ops);
> -		else if (of_node_name_eq(child_np, "usb3-port"))
> +			ret = drm_aux_bridge_register_from_node(dev, child_np);

[Severity: High]
Does this code result in a double-free of the child_np device node
reference?

The macro for_each_available_child_of_node() automatically manages the
reference count for child_np, acquiring it for the current iteration and
dropping it when advancing.

drm_aux_bridge_register_from_node() assigns the node to the auxiliary bridge
device, and the auxiliary bridge assumes ownership of a reference (calling
of_node_put() in its error paths and release callback).

Because an additional reference is not acquired via of_node_get()
before passing it, the reference is decremented twice, which can lead to a
use-after-free.

> +		} else if (of_node_name_eq(child_np, "usb3-port"))
>  			phy = devm_phy_create(dev, child_np,
>  					      &rockchip_usb3_phy_ops);
>  		else
>  			continue;
>  
> +		if (ret) {
> +			pm_runtime_disable(dev);
> +			of_node_put(child_np);
> +			return ret;
> +		}
> +
>  		if (IS_ERR(phy)) {

[Severity: Medium]
Will this error check ordering mask devm_phy_create() failures?

The check for bridge registration failure (if (ret)) happens before the
check for PHY creation failure (if (IS_ERR(phy))). If devm_phy_create()
fails (e.g., returning -EPROBE_DEFER) and the subsequent bridge registration
also fails, the function will return the bridge error and mask the PHY error.

Additionally, this attempts to register the auxiliary bridge even after the
underlying PHY creation has already failed. Should IS_ERR(phy) be checked
before calling drm_aux_bridge_register_from_node(), or at least before
checking ret?

>  			dev_err(dev, "failed to create phy: %pOFn\n",
>  				child_np);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521032854.103-1-kernel@airkyi.com?part=3

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