[PATCH] drm/bridge: display-connector: Fix DDC adapter leak in display_connector_probe()

[PATCH] drm/bridge: display-connector: Fix DDC adapter leak in display_connector_probe()

[Wentao Liang]

display_connector_probe() takes a reference to the DDC I2C adapter
with of_get_i2c_adapter_by_node() and releases it only in
display_connector_remove().  When the probe continues after obtaining
the adapter and then fails, e.g. because the ddc-en GPIO or the HDMI
power regulator cannot be obtained, or when the regulator cannot be
enabled, the function returns without dropping the adapter reference,
leaking it.

Drop the DDC adapter reference with i2c_put_adapter() on those error
paths before returning.

Fixes: 0c275c30176b ("drm/bridge: Add bridge driver for display connectors")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/bridge/display-connector.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index 16c0631adeb1..926d7638c1a7 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -352,18 +352,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);
 			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);
 			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);
 			return ret;
 		}
 	}
-- 
2.34.1

Re: [PATCH] drm/bridge: display-connector: Fix DDC adapter leak in display_connector_probe()

[sashiko-bot]

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 <vulab@iscas.ac.cn>

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/20260916100547.2012828-1-vulab@iscas.ac.cn?part=1