[PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path

[PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path

[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.

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

Re: [PATCH] drm/bridge: samsung-dsim: Fix memory leak in error path

[Luca Ceresoli]

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

Claude review: drm/bridge: samsung-dsim: Fix memory leak in error path

[Claude Code Review Bot]

Overall Series Review

Subject: drm/bridge: samsung-dsim: Fix memory leak in error path
Author: Osama Abdelkader <osama.abdelkader@gmail.com>
Patches: 2
Reviewed: 2026-02-11T16:53:10.913396

---

This is a single-patch series addressing a memory leak bug in the Samsung DSIM bridge driver. The patch fixes a legitimate resource leak where `drm_bridge_add()` is called early in `samsung_dsim_host_attach()`, but subsequent error paths fail to call the corresponding `drm_bridge_remove()`. The fix implements proper error unwinding using goto labels, following standard kernel patterns.

**Strengths:**
- Correctly identifies and fixes a real resource leak
- Uses standard kernel error handling patterns (goto labels)
- Includes appropriate Fixes tag and Cc: stable
- Handles both bridge removal and TE IRQ cleanup in error paths
- Code movement is minimal and justified (avoids forward declaration)

**Areas for improvement:**
- Patch versioning format (should use `[PATCH v2]` in subject)
- The changelog placement (v2 notes should go after the `---` separator)

**Overall assessment:** This is a solid bug fix that should be merged. The technical approach is correct, and the code follows kernel conventions. The only issues are minor formatting/process improvements that were already pointed out by the reviewer.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: samsung-dsim: Fix memory leak in error path

[Claude Code Review Bot]

Patch Review

**Commit Message Review:**

The commit message clearly explains the problem and solution. However, there's a formatting issue:

```
v2: 
- Move samsung_dsim_unregister_te_irq() function
- Add Fixes tag
- Add Cc tag
```

**Issue:** Version changelog should appear after the `---` separator (line 117), not before the Signed-off-by tag. This prevents the version notes from being included in the permanent git history.

**Technical Review:**

**1. Function movement (lines 134-140):**
```c
+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);
+	}
+}
```

**Analysis:** Moving this function upward is necessary and correct. The function is unchanged, which is good - it keeps the patch focused on the fix rather than mixing refactoring with bug fixes. This is proper kernel style.

**2. Error path for TE IRQ registration (lines 146-150):**
```c
 	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;
 	}
```

**Analysis:** Correct. The bridge was added earlier via `drm_bridge_add()` (line 1950 in original code, not shown in diff), so this error path must remove it.

**3. Error path for host attach (lines 153-157):**
```c
 	if (pdata->host_ops && pdata->host_ops->attach) {
 		ret = pdata->host_ops->attach(dsi, device);
 		if (ret)
-			return ret;
+			goto err_unregister_te_irq;
 	}
```

**Analysis:** Correct. At this point, both the bridge has been added AND the TE IRQ may have been registered (depending on the mode). The error label correctly handles both.

**4. Error unwinding implementation (lines 173-178):**
```c
+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;
 }
```

**Critical issue to verify:** The conditional check `if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))` in the error path mirrors the check used when registering the TE IRQ. This is correct because:
- TE IRQ is only registered when NOT in video mode
- The unregister function itself has a null check (`if (dsi->te_gpio)`)
- This provides defense-in-depth

**Potential concern:** The label naming follows reverse-chronological unwinding order (you jump to the LATER operation's label, then fall through to earlier ones). Let me verify the order:

1. `drm_bridge_add()` happens first (not shown in diff)
2. `samsung_dsim_register_te_irq()` happens second (conditionally)
3. `pdata->host_ops->attach()` happens third

Error unwinding should be in reverse order:
1. First we might need to unregister TE IRQ (if it was registered)
2. Then we always need to remove bridge

The implementation is **correct** - `err_unregister_te_irq` cleans up TE IRQ (if needed) then falls through to `err_remove_bridge`.

**5. Symmetry check:**

Looking at the detach path (`samsung_dsim_host_detach`, line 181), it should mirror this cleanup. The original code shows `samsung_dsim_unregister_te_irq(dsi)` was already there, so symmetry is maintained.

**Code Style Review:**
- Indentation: Correct (tabs)
- Goto labels: Properly placed (no indentation for labels, consistent with kernel style)
- Line length: Within acceptable limits
- Error handling pattern: Standard kernel unwinding style

**Fixes Tag Review:**
```
Fixes: e7447128ca4a ("drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge")
```

The Fixes tag appears appropriate. This commit likely introduced the `drm_bridge_add()` call without proper cleanup on error paths. A reviewer with access to git history should verify this is the correct commit.

**Testing Considerations:**

This patch modifies error paths, which are difficult to test without fault injection. The fix is correct by inspection, but ideally should be tested by:
1. Forcing `samsung_dsim_register_te_irq()` to fail
2. Forcing `pdata->host_ops->attach()` to fail
3. Verifying `drm_bridge_remove()` is called and no resources leak

**Minor observations:**

The patch shows version notes at line 118-121 within the commit message body. As mentioned by Luca Ceresoli's review (lines 282-290), these should be below the `---` line. The current location will cause them to be included in the permanent git history.

**Verdict:**

**Acked-by: [Reviewer]** (technical content is correct)

The code changes are sound and properly implement error unwinding. The only issue is the changelog placement, which is a process/formatting issue rather than a technical problem. The patch should be accepted with the understanding that future submissions should use proper version marking in the subject line and place version notes after the `---` separator.

---
Generated by Claude Code Patch Reviewer