The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path
@ 2026-02-07 18:37 Osama Abdelkader
  2026-02-09 12:41 ` Luca Ceresoli
  0 siblings, 1 reply; 6+ messages in thread
From: Osama Abdelkader @ 2026-02-07 18:37 UTC (permalink / raw)
  To: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel
  Cc: Osama Abdelkader

In samsung_dsim_host_attach(), drm_bridge_add() is called to add the
bridge. However, if samsung_dsim_register_te_irq() or
pdata->host_ops->attach() fails afterwards, the function returns
without removing the bridge, causing a memory leak.

Fix this by adding proper error handling with goto labels to ensure
drm_bridge_remove() is called in all error paths. Also ensure that
samsung_dsim_unregister_te_irq() is called if the attach operation
fails after the TE IRQ has been registered.

Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index eabc4c32f6ab..4712637749f8 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1881,6 +1881,8 @@ static int samsung_dsim_register_te_irq(struct samsung_dsim *dsi, struct device
 	return 0;
 }
 
+static void samsung_dsim_unregister_te_irq(struct samsung_dsim *dsi);
+
 static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 				    struct mipi_dsi_device *device)
 {
@@ -1955,13 +1957,13 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
 		ret = samsung_dsim_register_te_irq(dsi, &device->dev);
 		if (ret)
-			return ret;
+			goto err_remove_bridge;
 	}
 
 	if (pdata->host_ops && pdata->host_ops->attach) {
 		ret = pdata->host_ops->attach(dsi, device);
 		if (ret)
-			return ret;
+			goto err_unregister_te_irq;
 	}
 
 	dsi->lanes = device->lanes;
@@ -1969,6 +1971,13 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 	dsi->mode_flags = device->mode_flags;
 
 	return 0;
+
+err_unregister_te_irq:
+	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))
+		samsung_dsim_unregister_te_irq(dsi);
+err_remove_bridge:
+	drm_bridge_remove(&dsi->bridge);
+	return ret;
 }
 
 static void samsung_dsim_unregister_te_irq(struct samsung_dsim *dsi)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path
  2026-02-07 18:37 [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path Osama Abdelkader
@ 2026-02-09 12:41 ` Luca Ceresoli
  2026-02-09 18:47   ` Osama Abdelkader
  0 siblings, 1 reply; 6+ messages in thread
From: Luca Ceresoli @ 2026-02-09 12:41 UTC (permalink / raw)
  To: Osama Abdelkader, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

Hello Osama,

On Sat Feb 7, 2026 at 7:37 PM CET, Osama Abdelkader wrote:
> In samsung_dsim_host_attach(), drm_bridge_add() is called to add the
> bridge. However, if samsung_dsim_register_te_irq() or
> pdata->host_ops->attach() fails afterwards, the function returns
> without removing the bridge, causing a memory leak.
>
> Fix this by adding proper error handling with goto labels to ensure
> drm_bridge_remove() is called in all error paths. Also ensure that
> samsung_dsim_unregister_te_irq() is called if the attach operation
> fails after the TE IRQ has been registered.
>
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>

Good catch!

However being a fix you need a 'Fixes:' line pointing to the first commit
where hte bug exists. At a quick search it looks like f9bfd326f57e, but
please double check that. And with a Fixes tag you need Cc: stable, see the
docs [0].

> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1881,6 +1881,8 @@ static int samsung_dsim_register_te_irq(struct samsung_dsim *dsi, struct device
>  	return 0;
>  }
>
> +static void samsung_dsim_unregister_te_irq(struct samsung_dsim *dsi);

Please don't add a forward declaration. Just move
samsung_dsim_unregister_te_irq() function earlier, perhaps right here. Also
mention in the commit message that you have just moved it without changes,
to help reviewers: unfortunately code being moved it not very well
visualized in a diff.

Otherwise looks good!

It would be great if you please add me in Cc for future patches to this
driver. I'm using and touching it [1][2] so I'd like to review patches
touching it. Thanks!

[0] https://docs.kernel.org/process/stable-kernel-rules.html
[1] https://lore.kernel.org/lkml/20260109-drm-bridge-alloc-getput-drm_of_find_bridge-2-v2-0-8bad3ef90b9f@bootlin.com/
[2] https://lore.kernel.org/lkml/20260206-drm-bridge-atomic-vs-remove-clear_and_put-v1-2-6f1a7d03c45f@bootlin.com/

Luca

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path
@ 2026-02-09 18:41 Osama Abdelkader
  2026-02-10 10:44 ` Luca Ceresoli
  2026-02-20 14:31 ` Luca Ceresoli
  0 siblings, 2 replies; 6+ messages in thread
From: Osama Abdelkader @ 2026-02-09 18:41 UTC (permalink / raw)
  To: luca.ceresoli, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Marek Vasut,
	dri-devel, linux-kernel
  Cc: Osama Abdelkader, stable

In samsung_dsim_host_attach(), drm_bridge_add() is called to add the
bridge. However, if samsung_dsim_register_te_irq() or
pdata->host_ops->attach() fails afterwards, the function returns
without removing the bridge, causing a memory leak.

Fix this by adding proper error handling with goto labels to ensure
drm_bridge_remove() is called in all error paths. Also ensure that
samsung_dsim_unregister_te_irq() is called if the attach operation
fails after the TE IRQ has been registered.

samsung_dsim_unregister_te_irq() function is moved without changes
to be before samsung_dsim_host_attach() to avoid forward declaration.

Fixes: e7447128ca4a ("drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge")
Cc: stable@vger.kernel.org
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
v2: 
- Move samsung_dsim_unregister_te_irq() function
- Add Fixes tag
- Add Cc tag
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index eabc4c32f6ab..ad8c6aa49d48 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1881,6 +1881,14 @@ static int samsung_dsim_register_te_irq(struct samsung_dsim *dsi, struct device
 	return 0;
 }
 
+static void samsung_dsim_unregister_te_irq(struct samsung_dsim *dsi)
+{
+	if (dsi->te_gpio) {
+		free_irq(gpiod_to_irq(dsi->te_gpio), dsi);
+		gpiod_put(dsi->te_gpio);
+	}
+}
+
 static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 				    struct mipi_dsi_device *device)
 {
@@ -1955,13 +1963,13 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
 		ret = samsung_dsim_register_te_irq(dsi, &device->dev);
 		if (ret)
-			return ret;
+			goto err_remove_bridge;
 	}
 
 	if (pdata->host_ops && pdata->host_ops->attach) {
 		ret = pdata->host_ops->attach(dsi, device);
 		if (ret)
-			return ret;
+			goto err_unregister_te_irq;
 	}
 
 	dsi->lanes = device->lanes;
@@ -1969,14 +1977,13 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 	dsi->mode_flags = device->mode_flags;
 
 	return 0;
-}
 
-static void samsung_dsim_unregister_te_irq(struct samsung_dsim *dsi)
-{
-	if (dsi->te_gpio) {
-		free_irq(gpiod_to_irq(dsi->te_gpio), dsi);
-		gpiod_put(dsi->te_gpio);
-	}
+err_unregister_te_irq:
+	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))
+		samsung_dsim_unregister_te_irq(dsi);
+err_remove_bridge:
+	drm_bridge_remove(&dsi->bridge);
+	return ret;
 }
 
 static int samsung_dsim_host_detach(struct mipi_dsi_host *host,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path
  2026-02-09 12:41 ` Luca Ceresoli
@ 2026-02-09 18:47   ` Osama Abdelkader
  0 siblings, 0 replies; 6+ messages in thread
From: Osama Abdelkader @ 2026-02-09 18:47 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

On Mon, Feb 09, 2026 at 01:41:21PM +0100, Luca Ceresoli wrote:
> Hello Osama,
> 
> On Sat Feb 7, 2026 at 7:37 PM CET, Osama Abdelkader wrote:
> > In samsung_dsim_host_attach(), drm_bridge_add() is called to add the
> > bridge. However, if samsung_dsim_register_te_irq() or
> > pdata->host_ops->attach() fails afterwards, the function returns
> > without removing the bridge, causing a memory leak.
> >
> > Fix this by adding proper error handling with goto labels to ensure
> > drm_bridge_remove() is called in all error paths. Also ensure that
> > samsung_dsim_unregister_te_irq() is called if the attach operation
> > fails after the TE IRQ has been registered.
> >
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> 
> Good catch!
> 
> However being a fix you need a 'Fixes:' line pointing to the first commit
> where hte bug exists. At a quick search it looks like f9bfd326f57e, but
> please double check that. And with a Fixes tag you need Cc: stable, see the
> docs [0].
> 
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1881,6 +1881,8 @@ static int samsung_dsim_register_te_irq(struct samsung_dsim *dsi, struct device
> >  	return 0;
> >  }
> >
> > +static void samsung_dsim_unregister_te_irq(struct samsung_dsim *dsi);
> 
> Please don't add a forward declaration. Just move
> samsung_dsim_unregister_te_irq() function earlier, perhaps right here. Also
> mention in the commit message that you have just moved it without changes,
> to help reviewers: unfortunately code being moved it not very well
> visualized in a diff.
> 
> Otherwise looks good!
> 
> It would be great if you please add me in Cc for future patches to this
> driver. I'm using and touching it [1][2] so I'd like to review patches
> touching it. Thanks!

Sure, I will.
Thanks for letting me know.

> 
> [0] https://docs.kernel.org/process/stable-kernel-rules.html
> [1] https://lore.kernel.org/lkml/20260109-drm-bridge-alloc-getput-drm_of_find_bridge-2-v2-0-8bad3ef90b9f@bootlin.com/
> [2] https://lore.kernel.org/lkml/20260206-drm-bridge-atomic-vs-remove-clear_and_put-v1-2-6f1a7d03c45f@bootlin.com/
> 
> Luca
> 
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Hello Luca,

Thank you for the review, I just did that in v2.

Thanks,
Osama

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path
  2026-02-09 18:41 Osama Abdelkader
@ 2026-02-10 10:44 ` Luca Ceresoli
  2026-02-20 14:31 ` Luca Ceresoli
  1 sibling, 0 replies; 6+ messages in thread
From: Luca Ceresoli @ 2026-02-10 10:44 UTC (permalink / raw)
  To: Osama Abdelkader, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Marek Vasut,
	dri-devel, linux-kernel
  Cc: stable

Hello Osama,

On Mon Feb 9, 2026 at 7:41 PM CET, Osama Abdelkader wrote:
> In samsung_dsim_host_attach(), drm_bridge_add() is called to add the
> bridge. However, if samsung_dsim_register_te_irq() or
> pdata->host_ops->attach() fails afterwards, the function returns
> without removing the bridge, causing a memory leak.
>
> Fix this by adding proper error handling with goto labels to ensure
> drm_bridge_remove() is called in all error paths. Also ensure that
> samsung_dsim_unregister_te_irq() is called if the attach operation
> fails after the TE IRQ has been registered.
>
> samsung_dsim_unregister_te_irq() function is moved without changes
> to be before samsung_dsim_host_attach() to avoid forward declaration.
>
> Fixes: e7447128ca4a ("drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge")
> Cc: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> v2:

Please add the version number to the e-mail Subject, it should be:
  [PATCH v2] drm/bridge: ...
         ^^

No need to resend just for this, but please keep it in mind for the future.

Using b4 automates all of this very nicely, you can consider using it.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path
  2026-02-09 18:41 Osama Abdelkader
  2026-02-10 10:44 ` Luca Ceresoli
@ 2026-02-20 14:31 ` Luca Ceresoli
  1 sibling, 0 replies; 6+ messages in thread
From: Luca Ceresoli @ 2026-02-20 14:31 UTC (permalink / raw)
  To: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Marek Vasut,
	dri-devel, linux-kernel, Osama Abdelkader
  Cc: stable


On Mon, 09 Feb 2026 19:41:14 +0100, Osama Abdelkader wrote:
> In samsung_dsim_host_attach(), drm_bridge_add() is called to add the
> bridge. However, if samsung_dsim_register_te_irq() or
> pdata->host_ops->attach() fails afterwards, the function returns
> without removing the bridge, causing a memory leak.
> 
> Fix this by adding proper error handling with goto labels to ensure
> drm_bridge_remove() is called in all error paths. Also ensure that
> samsung_dsim_unregister_te_irq() is called if the attach operation
> fails after the TE IRQ has been registered.
> 
> [...]

Applied, thanks!

[1/1] drm/bridge: samsung-dsim: Fix memory leak in error path
      commit: 803ec1faf7c1823e6e3b1f2aaa81be18528c9436

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-20 14:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-07 18:37 [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path Osama Abdelkader
2026-02-09 12:41 ` Luca Ceresoli
2026-02-09 18:47   ` Osama Abdelkader
  -- strict thread matches above, loose matches on Subject: below --
2026-02-09 18:41 Osama Abdelkader
2026-02-10 10:44 ` Luca Ceresoli
2026-02-20 14:31 ` Luca Ceresoli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox