[PATCH v2 0/2] drm/bridge: add drm_bridge_clear_and_put()

[PATCH v2 0/2] drm/bridge: add drm_bridge_clear_and_put()

[Luca Ceresoli]

This small series introduces drm_bridge_clear_and_put(), to give a easy and
safe way to put a bridge reference and clear the pointer without leaving a
possible use-after-free window.

This is part of the work to support hotplug of DRM bridges. The grand plan
was discussed in [0].

Here's the work breakdown (➜ marks the current series):

 1. … add refcounting to DRM bridges struct drm_bridge,
      based on devm_drm_bridge_alloc()
    A. ✔ add new alloc API and refcounting (v6.16)
    B. ✔ convert all bridge drivers to new API (v6.17)
    C. ✔ kunit tests (v6.17)
    D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
         and warn on old allocation pattern (v6.17)
    E. … add get/put on drm_bridge accessors
       1. ✔ drm_bridge_chain_get_first_bridge(), add cleanup action (v6.18)
       2. ✔ drm_bridge_get_prev_bridge() (v6.18)
       3. ✔ drm_bridge_get_next_bridge() (v6.19)
       4. ✔ drm_for_each_bridge_in_chain() (v6.19)
       5. ✔ drm_bridge_connector_init (v6.19)
       6. … protect encoder bridge chain with a mutex
       7. … of_drm_find_bridge
          a. ✔ add of_drm_get_bridge() (v7.0),
	       convert basic direct users (v7.0-v7.1)
	  b. ✔ convert direct of_drm_get_bridge() users, part 2 (v7.0)
	  c. ✔ convert direct of_drm_get_bridge() users, part 3 (v7.0)
	  d. ✔… convert direct of_drm_get_bridge() users, part 4
	        (some v7.1, some pending)
	  e.   convert bridge-only drm_of_find_panel_or_bridge() users
       8. drm_of_find_panel_or_bridge, *_of_get_bridge
       9. ✔ enforce drm_bridge_add before drm_bridge_attach (v6.19)
    F. ✔ debugfs improvements
       1. ✔ add top-level 'bridges' file (v6.16)
       2. ✔ show refcount and list lingering bridges (v6.19)
 2. ➜ handle gracefully atomic updates during bridge removal
    A. ✔ Add drm_dev_enter/exit() to protect device resources (v6.20?)
    B. … protect private_obj removal from list
    C. ➜ Add drm_bridge_clear_and_put()
 3. … DSI host-device driver interaction
 4. ✔ removing the need for the "always-disconnected" connector
 5. … Migrate i.MX LCDIF driver to bridge-connector
 6.   DRM bridge hotplug
    A.   Bridge hotplug management in the DRM core
    B.   Device tree description

[0] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/#t

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v2:
- Rebased on current drm-misc-next fixing trivial conflict
- Link to v1: https://lore.kernel.org/r/20260206-drm-bridge-atomic-vs-remove-clear_and_put-v1-0-6f1a7d03c45f@bootlin.com

---
Luca Ceresoli (2):
      drm/bridge: add drm_bridge_clear_and_put()
      drm/bridge: samsung-dsim: use drm_bridge_clear_and_put() to put the next bridge

 drivers/gpu/drm/bridge/samsung-dsim.c |  7 ++-----
 drivers/gpu/drm/drm_bridge.c          | 34 ++++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h              |  1 +
 3 files changed, 37 insertions(+), 5 deletions(-)
---
base-commit: 08c6a9b501d2fdf161094a98e1febec41f566c8f
change-id: 20260206-drm-bridge-atomic-vs-remove-clear_and_put-a424c752a061

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

[PATCH v2 1/2] drm/bridge: add drm_bridge_clear_and_put()

[Luca Ceresoli]

Drivers having a struct drm_bridge pointer pointing to a bridge in many
cases hold that reference until the owning device is removed. In those
cases the reference to the bridge can be put in the .remove callback
(possibly using devm actions) or in the .destroy func (possibly with the
help of struct drm_bridge::next_bridge). At those moments the driver should
not be operating anymore and won't dereference the bridge pointer after it
is put.

However there are cases when drivers need to stop holding a reference to a
bridge even when their device is not being removed. This is the case for
bridge hot-unplug, when a bridge is removed but the previous entity (bridge
or encoder) is staying. In such case the "previous entity" needs to put it
but cannot do it via devm or .destroy, because it is not being removed.

The easy way to dispose of such pointer is:

  drm_bridge_put(my_priv->some_bridge);
  my_priv->some_bridge = NULL;

However this is risky because there is a time window between the two lines
where the reference is put, and thus the bridge could be deallocated, but
the pointer is still assigned. If other functions of the same driver were
invoked concurrently they might dereference my_priv->some_bridge during
that window, resulting in use-after-free.

A correct solution is to clear the pointer before putting the reference,
but that needs a temporary variable:

  struct drm_bridge *temp = my_priv->some_bridge;
  my_priv->some_bridge = NULL;
  drm_bridge_put(temp);

This solution is however annoying to write, so the incorrect version might
still sneak in.

Add a simple, easy to use function to put a bridge after setting its
pointer to NULL in the correct way.

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

---

This is a renamed version of drm_bridge_put_and_clear() which was sent as
part of a larger patch [0] but with largely rewritten documentation and a
detailed commit message.

[0] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-14-9d6f2c9c3058@bootlin.com/
---
 drivers/gpu/drm/drm_bridge.c | 34 ++++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     |  1 +
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index f8b0333a0a3b..0260b16cb47d 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -304,6 +304,9 @@ EXPORT_SYMBOL(drm_bridge_get);
  *
  * This function decrements the bridge's reference count and frees the
  * object if the reference count drops to zero.
+ *
+ * See also drm_bridge_clear_and_put() if you also need to set the pointer
+ * to NULL
  */
 void drm_bridge_put(struct drm_bridge *bridge)
 {
@@ -312,6 +315,37 @@ void drm_bridge_put(struct drm_bridge *bridge)
 }
 EXPORT_SYMBOL(drm_bridge_put);
 
+/**
+ * drm_bridge_clear_and_put - Given a bridge pointer, clear the pointer
+ *                            then put the bridge
+ * @bridge_pp: pointer to pointer to a struct drm_bridge; ``bridge_pp``
+ *             must be non-NULL; if ``*bridge_pp`` is NULL this function
+ *             does nothing
+ *
+ * Helper to put a DRM bridge, but only after setting its pointer to
+ * NULL. Useful when a struct drm_bridge reference must be dropped without
+ * leaving a use-after-free window where the pointed bridge might have been
+ * freed while still holding a pointer to it.
+ *
+ * For struct ``drm_bridge *some_bridge``, this code::
+ *
+ *     drm_bridge_clear_and_put(&some_bridge);
+ *
+ * is equivalent to the more complex::
+ *
+ *     struct drm_bridge *temp = some_bridge;
+ *     some_bridge = NULL;
+ *     drm_bridge_put(temp);
+ */
+void drm_bridge_clear_and_put(struct drm_bridge **bridge_pp)
+{
+	struct drm_bridge *bridge = *bridge_pp;
+
+	*bridge_pp = NULL;
+	drm_bridge_put(bridge);
+}
+EXPORT_SYMBOL(drm_bridge_clear_and_put);
+
 /**
  * drm_bridge_put_void - wrapper to drm_bridge_put() taking a void pointer
  *
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4f19f7064ee3..66ab89cf48aa 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1290,6 +1290,7 @@ void drm_bridge_unplug(struct drm_bridge *bridge);
 
 struct drm_bridge *drm_bridge_get(struct drm_bridge *bridge);
 void drm_bridge_put(struct drm_bridge *bridge);
+void drm_bridge_clear_and_put(struct drm_bridge **bridge_pp);
 
 /* Cleanup action for use with __free() */
 DEFINE_FREE(drm_bridge_put, struct drm_bridge *, if (_T) drm_bridge_put(_T))

-- 
2.53.0

[PATCH v2 2/2] drm/bridge: samsung-dsim: use drm_bridge_clear_and_put() to put the next bridge

[Luca Ceresoli]

drm_bridge_clear_and_put() is simpler to write and it prevents any
potential future use-after-free.

Reviewed-by: Osama Abdelkader <osama.abdelkader@gmail.com>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index ec632f268644..c3eb437ef1b0 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1988,9 +1988,7 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 	return 0;
 
 err_release_next_bridge:
-	drm_bridge_put(dsi->bridge.next_bridge);
-	dsi->bridge.next_bridge = NULL;
-
+	drm_bridge_clear_and_put(&dsi->bridge.next_bridge);
 	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))
 		samsung_dsim_unregister_te_irq(dsi);
 err_remove_bridge:
@@ -2007,8 +2005,7 @@ static int samsung_dsim_host_detach(struct mipi_dsi_host *host,
 	if (pdata->host_ops && pdata->host_ops->detach)
 		pdata->host_ops->detach(dsi, device);
 
-	drm_bridge_put(dsi->bridge.next_bridge);
-	dsi->bridge.next_bridge = NULL;
+	drm_bridge_clear_and_put(&dsi->bridge.next_bridge);
 
 	samsung_dsim_unregister_te_irq(dsi);
 

-- 
2.53.0

Claude review: drm/bridge: add drm_bridge_clear_and_put()

[Claude Code Review Bot]

Overall Series Review

Subject: drm/bridge: add drm_bridge_clear_and_put()
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
Patches: 3
Reviewed: 2026-03-11T13:16:32.078777

---

This is a clean, well-motivated 2-patch series that adds a `drm_bridge_clear_and_put()` helper and converts the first user (samsung-dsim) to it. The series is part of ongoing DRM bridge hotplug work. The code is simple, the documentation is thorough, and the conversion in patch 2 is straightforward.

**One significant concern**: the commit message and documentation claim this function prevents a "use-after-free window" in concurrent scenarios, but the implementation uses plain C pointer reads and writes with no memory ordering guarantees (no `WRITE_ONCE`, `smp_store_release`, or similar). If another CPU is concurrently reading `*bridge_pp`, the compiler could reorder or cache the store, and the CPU could make it visible after the `kref_put`. For the stated concurrency goal to actually hold, the NULL store should use `WRITE_ONCE()` at minimum to prevent compiler tearing/reordering, and callers reading the pointer should use `READ_ONCE()`. Without that, the "no use-after-free window" claim is not fully guaranteed by the C memory model on SMP systems.

That said, looking at the current samsung-dsim usage sites (error path in `host_attach` and the `host_detach` path), these don't appear to have actual concurrent access to `next_bridge`, so the practical risk is low today. But if the goal is to build infrastructure for hotplug where concurrency *will* matter, this should be addressed now.

**Verdict**: The series is reasonable as-is for the current use cases, but if it's meant to be a concurrency-safe primitive for hotplug, it needs memory ordering annotations.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: add drm_bridge_clear_and_put()

[Claude Code Review Bot]

Patch Review

**Code review**:

The implementation at `drivers/gpu/drm/drm_bridge.c:340-346`:
```c
void drm_bridge_clear_and_put(struct drm_bridge **bridge_pp)
{
	struct drm_bridge *bridge = *bridge_pp;

	*bridge_pp = NULL;
	drm_bridge_put(bridge);
}
```

- **Concurrency concern**: As noted above, both the read of `*bridge_pp` and the write of `NULL` use plain accesses. If this is meant to be safe against concurrent readers (as the commit message strongly implies), the write should be `WRITE_ONCE(*bridge_pp, NULL)` and the initial read should be a corresponding `READ_ONCE`. The kernel has established patterns for this — e.g., `rcu_assign_pointer()` or at minimum `WRITE_ONCE()` / `smp_store_release()`. Without these, the compiler is free to reorder the NULL store after the `drm_bridge_put()` call or even optimize it in unexpected ways.

- **Documentation quality**: The kerneldoc is well-written, with clear examples showing the equivalent manual pattern. The cross-reference added to `drm_bridge_put()` is a nice touch.

- **Naming**: `clear_and_put` follows the existing kernel convention (similar to `fput_and_clear`, though that one is actually named `fput` not `put_and_clear`). The kernel has `kfree_and_null` patterns too. The name is fine.

- **Minor**: The function doesn't explicitly document thread-safety guarantees. If it's not intended to be called concurrently with readers, that should be stated. If it is, it needs the memory barriers.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: samsung-dsim: use drm_bridge_clear_and_put() to put the next bridge

[Claude Code Review Bot]

Patch Review

**Code review**:

The conversion is mechanical and correct. In `samsung_dsim_host_attach` error path (`drivers/gpu/drm/bridge/samsung-dsim.c:1991`):
```c
err_release_next_bridge:
	drm_bridge_clear_and_put(&dsi->bridge.next_bridge);
	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))
```

And in `samsung_dsim_host_detach` (`drivers/gpu/drm/bridge/samsung-dsim.c:2008`):
```c
	drm_bridge_clear_and_put(&dsi->bridge.next_bridge);
```

Both replace the previous `drm_bridge_put()` + explicit `= NULL` pattern.

- **Correctness**: The blank line removal in the error path (between the put and the `if` check) is a minor style change bundled in. This is fine but worth noting.

- **Reviewed-by tag present**: Already has a Reviewed-by from Osama Abdelkader.

- **No functional change** beyond the ordering of NULL-assignment vs put, which is the whole point.

Overall: a clean, low-risk series. The main actionable feedback is whether `WRITE_ONCE` should be used in patch 1 to actually deliver on the stated concurrency safety promise.

---
Generated by Claude Code Patch Reviewer