[v1] drm/bridge: dw-mipi-dsi: Fix bridge leak when host attach fails

[PATCH] drm/bridge: dw-mipi-dsi: Fix bridge leak when host attach fails

Posted by Osama Abdelkader 2 months, 1 week ago

dw_mipi_dsi_host_attach() and dw_mipi_dsi2_host_attach() call
drm_bridge_add() before pdata->host_ops->attach(). If attach fails,
the bridge stayed registered without drm_bridge_remove(), leaking the
bridge reference and leaving the device on the global bridge list.

On failure, undo in the same order as host_detach(): for dw-mipi-dsi,
drm_of_panel_bridge_remove() then drm_bridge_remove(); for dw-mipi-dsi2,
drm_bridge_remove() then drm_of_panel_bridge_remove().

Fixes: 90910a651123 ("drm/bridge/synopsys: dsi: add ability to have glue-specific attach and detach")
Fixes: 0d6d86253fef ("drm/bridge/synopsys: Add MIPI DSI2 host controller bridge")
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c  | 7 ++++++-
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index ca4dea226f4b..ffa169c2de73 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -345,10 +345,15 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 	if (pdata->host_ops && pdata->host_ops->attach) {
 		ret = pdata->host_ops->attach(pdata->priv_data, device);
 		if (ret < 0)
-			return ret;
+			goto err_remove_bridge;
 	}
 
 	return 0;
+
+err_remove_bridge:
+	drm_of_panel_bridge_remove(host->dev->of_node, 1, 0);
+	drm_bridge_remove(&dsi->bridge);
+	return ret;
 }
 
 static int dw_mipi_dsi_host_detach(struct mipi_dsi_host *host,
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c
index e6eaf9fd0251..d7c922d8841c 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c
@@ -540,10 +540,15 @@ static int dw_mipi_dsi2_host_attach(struct mipi_dsi_host *host,
 	if (pdata->host_ops && pdata->host_ops->attach) {
 		ret = pdata->host_ops->attach(pdata->priv_data, device);
 		if (ret < 0)
-			return ret;
+			goto err_remove_bridge;
 	}
 
 	return 0;
+
+err_remove_bridge:
+	drm_bridge_remove(&dsi2->bridge);
+	drm_of_panel_bridge_remove(host->dev->of_node, 1, 0);
+	return ret;
 }
 
 static int dw_mipi_dsi2_host_detach(struct mipi_dsi_host *host,
-- 
2.43.0

Re: [PATCH] drm/bridge: dw-mipi-dsi: Fix bridge leak when host attach fails

Posted by Luca Ceresoli 2 months, 1 week ago

Hello Osama,

On Thu Apr 2, 2026 at 11:00 PM CEST, Osama Abdelkader wrote:
> dw_mipi_dsi_host_attach() and dw_mipi_dsi2_host_attach() call
> drm_bridge_add() before pdata->host_ops->attach(). If attach fails,
> the bridge stayed registered without drm_bridge_remove(), leaking the
> bridge reference and leaving the device on the global bridge list.
>
> On failure, undo in the same order as host_detach(): for dw-mipi-dsi,
> drm_of_panel_bridge_remove() then drm_bridge_remove(); for dw-mipi-dsi2,
> drm_bridge_remove() then drm_of_panel_bridge_remove().
>
> Fixes: 90910a651123 ("drm/bridge/synopsys: dsi: add ability to have glue-specific attach and detach")
> Fixes: 0d6d86253fef ("drm/bridge/synopsys: Add MIPI DSI2 host controller bridge")
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c  | 7 ++++++-
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index ca4dea226f4b..ffa169c2de73 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -345,10 +345,15 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>  	if (pdata->host_ops && pdata->host_ops->attach) {
>  		ret = pdata->host_ops->attach(pdata->priv_data, device);
>  		if (ret < 0)
> -			return ret;
> +			goto err_remove_bridge;
>  	}
>
>  	return 0;
> +
> +err_remove_bridge:
> +	drm_of_panel_bridge_remove(host->dev->of_node, 1, 0);

It was added by a devm_action, you shouldn't remove it here. Besides,
devm_drm_of_get_bridge() might return A) a panel_bridge or B) a pointer to
another, pre-existing bridge (and the called can't know which one got
returned). drm_of_panel_bridge_remove() unconditionally removes a
panel_bridge so it's wrong in case B.

Which means the drm_of_panel_bridge_remove() in _host_detach is probably
wrong and you should look at it.

> +	drm_bridge_remove(&dsi->bridge);

This is correct, even though using devm_drm_bridge_add() instead of
drm_bridge_add() above is probably the simplest solution.

Same for the other driver in this patch.

Luca

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

Re: [PATCH] drm/bridge: dw-mipi-dsi: Fix bridge leak when host attach fails

Posted by Osama Abdelkader 2 months, 1 week ago

Hello Luca,

On Fri, Apr 03, 2026 at 08:50:29AM +0200, Luca Ceresoli wrote:
> Hello Osama,
> 
> On Thu Apr 2, 2026 at 11:00 PM CEST, Osama Abdelkader wrote:
> > dw_mipi_dsi_host_attach() and dw_mipi_dsi2_host_attach() call
> > drm_bridge_add() before pdata->host_ops->attach(). If attach fails,
> > the bridge stayed registered without drm_bridge_remove(), leaking the
> > bridge reference and leaving the device on the global bridge list.
> >
> > On failure, undo in the same order as host_detach(): for dw-mipi-dsi,
> > drm_of_panel_bridge_remove() then drm_bridge_remove(); for dw-mipi-dsi2,
> > drm_bridge_remove() then drm_of_panel_bridge_remove().
> >
> > Fixes: 90910a651123 ("drm/bridge/synopsys: dsi: add ability to have glue-specific attach and detach")
> > Fixes: 0d6d86253fef ("drm/bridge/synopsys: Add MIPI DSI2 host controller bridge")
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> > ---
> >  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c  | 7 ++++++-
> >  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c | 7 ++++++-
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > index ca4dea226f4b..ffa169c2de73 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -345,10 +345,15 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >  	if (pdata->host_ops && pdata->host_ops->attach) {
> >  		ret = pdata->host_ops->attach(pdata->priv_data, device);
> >  		if (ret < 0)
> > -			return ret;
> > +			goto err_remove_bridge;
> >  	}
> >
> >  	return 0;
> > +
> > +err_remove_bridge:
> > +	drm_of_panel_bridge_remove(host->dev->of_node, 1, 0);
> 
> It was added by a devm_action, you shouldn't remove it here. Besides,
> devm_drm_of_get_bridge() might return A) a panel_bridge or B) a pointer to
> another, pre-existing bridge (and the called can't know which one got
> returned). drm_of_panel_bridge_remove() unconditionally removes a
> panel_bridge so it's wrong in case B.
> 
> Which means the drm_of_panel_bridge_remove() in _host_detach is probably
> wrong and you should look at it.
> 
> > +	drm_bridge_remove(&dsi->bridge);

Thanks for the information, I'm going to look at it in another patch.

> 
> This is correct, even though using devm_drm_bridge_add() instead of
> drm_bridge_add() above is probably the simplest solution.
> 
> Same for the other driver in this patch.

Thank you, I just did that in v2. 
Thanks,
Osama

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