Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/bridge: display-connector: double-put of I2C adapter on probe failure --
commit 6664228340b6b3ba7c429f5b94a033d13e1f4e21 Author: Wentao Liang <[email protected]> drm/bridge: display-connector: Fix DDC adapter leak in display_connector_probe() This commit explicitly drops the DDC adapter reference on error paths in display_connector_probe() to prevent a resource leak when probe fails after obtaining the adapter. > diff --git a/drivers/gpu/drm/bridge/display-connector.c > b/drivers/gpu/drm/bridge/display-connector.c > index eb3412ec30a07..e7e9c40dbdd97 100644 > --- a/drivers/gpu/drm/bridge/display-connector.c > +++ b/drivers/gpu/drm/bridge/display-connector.c > @@ -394,18 +394,22 @@ static int display_connector_probe(struct > platform_device *pdev) > > if (IS_ERR(conn->ddc_en)) { > dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n"); > + i2c_put_adapter(conn->bridge.ddc); [Severity: High] Could this result in a double-put of the I2C adapter when returning an error? Since the display_connector structure is allocated earlier using devm_drm_bridge_alloc(), returning an error will trigger the devres cleanup action. This cleanup calls drm_bridge_put(), which invokes the bridge's destroy callback, display_connector_destroy(). Looking at display_connector_destroy(), it already releases the adapter: static void display_connector_destroy(struct drm_bridge *bridge) { struct display_connector *conn = bridge_to_display_connector(bridge); ... i2c_put_adapter(conn->bridge.ddc); ... } If we explicitly call i2c_put_adapter() here without also setting conn->bridge.ddc to NULL, will the devres cleanup path put the adapter a second time and cause a refcount underflow? > return PTR_ERR(conn->ddc_en); > } > > ret = display_connector_get_supply(pdev, conn, "hdmi-pwr"); > - if (ret < 0) > + if (ret < 0) { > + i2c_put_adapter(conn->bridge.ddc); [Severity: High] Does this also suffer from the same devres double-put regression during cleanup? > return dev_err_probe(&pdev->dev, ret, "failed to get > HDMI +5V Power regulator\n"); > + } > } > > if (conn->supply) { > ret = regulator_enable(conn->supply); > if (ret) { > dev_err(&pdev->dev, "failed to enable PWR regulator: > %d\n", ret); > + i2c_put_adapter(conn->bridge.ddc); [Severity: High] Does this path similarly trigger a double-put via display_connector_destroy() when returning the error? > return ret; > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
