Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Critical] Refcount underflow and use-after-free for bridges acquired in DPI mode. - [High] Resource leak of the bridge reference on probe error paths. Pre-existing issues: - [High] Undefined behavior due to a negative bitshift when evaluating `tc->hpd_pin`. -- --- Patch [6]: [PATCH 06/11] drm/bridge: tc358767: don't create a panel_bridge --- commit ae8aef2f483ffa927af25a2f1ac42b0420349ca8 Author: Luca Ceresoli <[email protected]> drm/bridge: tc358767: don't create a panel_bridge This commit prevents creating a second panel_bridge by retrieving the already-existing one using of_drm_get_bridge_by_endpoint() for eDP endpoints. It updates tc_remove() to release the reference. > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index e3a57f8228dae..2fef5d9fa361d 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -2347,24 +2347,15 @@ static int tc_probe_dpi_bridge_endpoint(struct > tc_data *tc) > static int tc_probe_edp_bridge_endpoint(struct tc_data *tc) > { > struct device *dev = tc->dev; > - struct drm_panel *panel; > - int ret; > + struct drm_bridge *bridge; > > /* port@2 is the output port */ > - ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &panel, NULL); > - if (ret && ret != -ENODEV) > - return dev_err_probe(dev, ret, > - "Could not find DSI panel or bridge\n"); > + bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 2, 0); > + if (IS_ERR(bridge) && PTR_ERR(bridge) != -ENODEV) > + return dev_err_probe(dev, PTR_ERR(bridge), "Could not find DSI > bridge\n"); > > - if (panel) { > - struct drm_bridge *panel_bridge; > - > - panel_bridge = devm_drm_panel_bridge_add(dev, panel); > - drm_panel_put(panel); > - if (IS_ERR(panel_bridge)) > - return PTR_ERR(panel_bridge); > - > - tc->panel_bridge = panel_bridge; > + if (!IS_ERR(bridge)) { > + tc->panel_bridge = bridge; > tc->bridge.type = DRM_MODE_CONNECTOR_eDP; > } else { > tc->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; > } [Severity: High] If of_drm_get_bridge_by_endpoint() successfully returns a refcounted bridge pointer, it is stored in tc->panel_bridge. If any subsequent initialization step in tc_probe() fails, it appears to return directly without cleaning up the bridge reference: tc_probe() { ... tc_probe_edp_bridge_endpoint(tc); ... tc->refclk = devm_clk_get_enabled(dev, "ref"); if (IS_ERR(tc->refclk)) return dev_err_probe(dev, PTR_ERR(tc->refclk), "Failed to get and enable the ref clk\n"); ... } Could this cause a resource leak on probe error paths? [Severity: High] This is a pre-existing issue, but does evaluating tc->hpd_pin here lead to a negative bitshift later? Due to initialization order, tc_data is zero-allocated, meaning tc->hpd_pin is 0 when checked in tc_probe_edp_bridge_endpoint(): if (tc->hpd_pin >= 0) tc->bridge.ops |= DRM_BRIDGE_OP_DETECT; Later in tc_probe(), if the DT property is absent, tc->hpd_pin is assigned -ENODEV (-19). When the DRM core periodically polls the connector, it calls tc_bridge_detect(), which executes: conn = val & BIT(tc->hpd_pin); Does this lead to undefined behavior by evaluating BIT(-19)? [ ... ] > @@ -2609,6 +2600,7 @@ static void tc_remove(struct i2c_client *client) > { > struct tc_data *tc = i2c_get_clientdata(client); > > + drm_bridge_put(tc->panel_bridge); > drm_bridge_remove(&tc->bridge); > } [Severity: Critical] Does this unconditional drm_bridge_put() cause a double-free for bridges acquired in DPI mode? When operating in DPI mode, tc_probe_dpi_bridge_endpoint() is called instead, which assigns tc->panel_bridge to a devm-managed bridge: tc_probe_dpi_bridge_endpoint() { ... bridge = devm_drm_panel_bridge_add(dev, panel); ... tc->panel_bridge = bridge; } When the device unbinds, tc_remove() drops a reference it does not own. For the devm-managed bridge, this prematurely drops its refcount to 0, freeing the bridge immediately. Later, the devres framework will attempt to clean up the same bridge, causing a double-free. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
