Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Double-free and Use-After-Free of OF Node Reference - [Medium] Initialization Ordering and Error Masking --
commit 0733fab98743300c9bca39a083f1a901d4ff8cda Author: Chaoyi Chen <[email protected]> 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 cause a double-free of the OF node reference? Since child_np is the loop iterator in for_each_available_child_of_node(), its reference is automatically dropped when advancing to the next iteration. However, drm_aux_bridge_register_from_node() assigns the OF node to the auxiliary device without incrementing the reference count, and drops a reference in its error paths and release function. Will this lead to a use-after-free when the loop iterator automatically drops the reference, leaving the registered bridge pointing to a freed device node? [Severity: Medium] Also, does this sequence mask errors from devm_phy_create()? The result of devm_phy_create() is not checked before calling drm_aux_bridge_register_from_node(). If devm_phy_create() returns an error pointer (such as -EPROBE_DEFER) and drm_aux_bridge_register_from_node() fails, the original error will be permanently lost when returning 'ret' below. Additionally, if devm_phy_create() fails but bridge registration succeeds, an auxiliary device is registered only to be immediately unregistered via devres when returning PTR_ERR(phy) shortly after. Could this be avoided by checking IS_ERR(phy) before registering the bridge? > + } 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); [Severity: High] Since drm_aux_bridge_register_from_node() already dropped the OF node reference on its internal failure path, will calling of_node_put() here cause an immediate double-free? > + return ret; > + } > + > if (IS_ERR(phy)) { > dev_err(dev, "failed to create phy: %pOFn\n", > child_np); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
