[PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak

[PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak

[Laurent Pinchart]

If the probe function returns an error after getting the I2C adapter for
DDC, the reference to the adapter is never released. Fix it by releasing
it in the bridge .destroy() handler.

There is no need to test the ddc pointer with !IS_ERR(), as
of_get_i2c_adapter_by_node() returns NULL on error.

Fixes: 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio support")
Cc: stable@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/display-connector.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index d1e74dd2e051..eb3412ec30a0 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -43,6 +43,13 @@ static int display_connector_attach(struct drm_bridge *bridge,
 	return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
 }
 
+static void display_connector_destroy(struct drm_bridge *bridge)
+{
+	struct display_connector *conn = to_display_connector(bridge);
+
+	i2c_put_adapter(conn->bridge.ddc);
+}
+
 static enum drm_connector_status display_connector_detect(struct drm_bridge *bridge)
 {
 	struct display_connector *conn = to_display_connector(bridge);
@@ -208,6 +215,7 @@ static u32 *display_connector_get_input_bus_fmts(struct drm_bridge *bridge,
 
 static const struct drm_bridge_funcs display_connector_bridge_funcs = {
 	.attach = display_connector_attach,
+	.destroy = display_connector_destroy,
 	.detect = display_connector_bridge_detect,
 	.hpd_enable = display_connector_hpd_enable,
 	.hpd_disable = display_connector_hpd_disable,
@@ -439,9 +447,6 @@ static void display_connector_remove(struct platform_device *pdev)
 		regulator_disable(conn->supply);
 
 	drm_bridge_remove(&conn->bridge);
-
-	if (!IS_ERR(conn->bridge.ddc))
-		i2c_put_adapter(conn->bridge.ddc);
 }
 
 static const struct of_device_id display_connector_match[] = {
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak

[Johan Hovold]

On Mon, Jul 06, 2026 at 12:35:40AM +0300, Laurent Pinchart wrote:
> If the probe function returns an error after getting the I2C adapter for
> DDC, the reference to the adapter is never released. Fix it by releasing
> it in the bridge .destroy() handler.
> 
> There is no need to test the ddc pointer with !IS_ERR(), as
> of_get_i2c_adapter_by_node() returns NULL on error.

I stumbled over this this morning as well and posted a fix here (which
releases the adapter on driver unbind as is currently done):

	https://lore.kernel.org/lkml/20260717085716.1619275-1-johan@kernel.org/

> Fixes: 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio support")

I believe this issue was first introduced by commit 2e2bf3a5584d
("drm/bridge: display-connector: add DP support") a few releases
earlier.

> Cc: stable@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Johan Hovold <johan@kernel.org>

Johan

Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak

[Laurent Pinchart]

On Fri, Jul 17, 2026 at 05:21:11PM +0200, Luca Ceresoli wrote:
> On Fri Jul 17, 2026 at 5:03 PM CEST, Johan Hovold wrote:
> > On Mon, Jul 06, 2026 at 12:35:40AM +0300, Laurent Pinchart wrote:
> >> If the probe function returns an error after getting the I2C adapter for
> >> DDC, the reference to the adapter is never released. Fix it by releasing
> >> it in the bridge .destroy() handler.
> >>
> >> There is no need to test the ddc pointer with !IS_ERR(), as
> >> of_get_i2c_adapter_by_node() returns NULL on error.
> >
> > I stumbled over this this morning as well and posted a fix here (which
> > releases the adapter on driver unbind as is currently done):
> >
> > 	https://lore.kernel.org/lkml/20260717085716.1619275-1-johan@kernel.org/
> >
> >> Fixes: 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio support")
> >
> > I believe this issue was first introduced by commit 2e2bf3a5584d
> > ("drm/bridge: display-connector: add DP support") a few releases
> > earlier.
> >
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >
> > Reviewed-by: Johan Hovold <johan@kernel.org>
> 
> While Johan's patch has the benefit of putting the i2c adapter at remove
> time (as opposed to destroy time, which is potentially a long time later),
> it is also more complex. So I'd say this patch is fine.

In this case putting the I2C adapter early is probably fine, but in
general I really prefer reference-counting and releasing references at
destroy time. This ensures that the resources stay available if they
need to be accessed between .remove() and destruction (for instance in a
.release() handler following a close() or munmap() from userspace). I
think late release should be the default, as it's safer, and early
release should be carefully reviewed when it's required.

> Maybe we should have a devm version of of_get_i2c_adapter_by_node() to make
> it all simpler.
> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak

[Luca Ceresoli]

Hi Laurent,

On Fri Jul 17, 2026 at 8:28 PM CEST, Laurent Pinchart wrote:
> On Fri, Jul 17, 2026 at 05:21:11PM +0200, Luca Ceresoli wrote:
>> On Fri Jul 17, 2026 at 5:03 PM CEST, Johan Hovold wrote:
>> > On Mon, Jul 06, 2026 at 12:35:40AM +0300, Laurent Pinchart wrote:
>> >> If the probe function returns an error after getting the I2C adapter for
>> >> DDC, the reference to the adapter is never released. Fix it by releasing
>> >> it in the bridge .destroy() handler.
>> >>
>> >> There is no need to test the ddc pointer with !IS_ERR(), as
>> >> of_get_i2c_adapter_by_node() returns NULL on error.
>> >
>> > I stumbled over this this morning as well and posted a fix here (which
>> > releases the adapter on driver unbind as is currently done):
>> >
>> > 	https://lore.kernel.org/lkml/20260717085716.1619275-1-johan@kernel.org/
>> >
>> >> Fixes: 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio support")
>> >
>> > I believe this issue was first introduced by commit 2e2bf3a5584d
>> > ("drm/bridge: display-connector: add DP support") a few releases
>> > earlier.
>> >
>> >> Cc: stable@vger.kernel.org
>> >> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> >
>> > Reviewed-by: Johan Hovold <johan@kernel.org>
>>
>> While Johan's patch has the benefit of putting the i2c adapter at remove
>> time (as opposed to destroy time, which is potentially a long time later),
>> it is also more complex. So I'd say this patch is fine.
>
> In this case putting the I2C adapter early is probably fine, but in
> general I really prefer reference-counting and releasing references at
> destroy time. This ensures that the resources stay available if they
> need to be accessed between .remove() and destruction (for instance in a
> .release() handler following a close() or munmap() from userspace). I
> think late release should be the default, as it's safer, and early
> release should be carefully reviewed when it's required.

Makes sense indeed.

Thanks for the clarification.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak

[Laurent Pinchart]

On Fri, Jul 17, 2026 at 05:03:49PM +0200, Johan Hovold wrote:
> On Mon, Jul 06, 2026 at 12:35:40AM +0300, Laurent Pinchart wrote:
> > If the probe function returns an error after getting the I2C adapter for
> > DDC, the reference to the adapter is never released. Fix it by releasing
> > it in the bridge .destroy() handler.
> > 
> > There is no need to test the ddc pointer with !IS_ERR(), as
> > of_get_i2c_adapter_by_node() returns NULL on error.
> 
> I stumbled over this this morning as well and posted a fix here (which
> releases the adapter on driver unbind as is currently done):
> 
> 	https://lore.kernel.org/lkml/20260717085716.1619275-1-johan@kernel.org/
> 
> > Fixes: 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio support")
> 
> I believe this issue was first introduced by commit 2e2bf3a5584d
> ("drm/bridge: display-connector: add DP support") a few releases
> earlier.

You're right. I'll update the Fixes: tag.

> > Cc: stable@vger.kernel.org
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Reviewed-by: Johan Hovold <johan@kernel.org>

-- 
Regards,

Laurent Pinchart