[PATCH] drm/connector: check if destroy function exist

[PATCH] drm/connector: check if destroy function exist

[Andreas Kemnade]

Connectors might have not resources needed to be manually freed.
E.g. the drm_bridge_connector does not have such a callback.

Fixes: c12907be57b1 ("drm/bridge-connector: switch to using drmm allocations")
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/gpu/drm/drm_connector.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 47dc53c4a738..d2dfc6e8f7d0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -198,7 +198,9 @@ static void drm_connector_free(struct kref *kref)
 	struct drm_device *dev = connector->dev;
 
 	drm_mode_object_unregister(dev, &connector->base);
-	connector->funcs->destroy(connector);
+
+	if (connector->funcs->destroy)
+		connector->funcs->destroy(connector);
 }
 
 void drm_connector_free_work_fn(struct work_struct *work)
@@ -216,7 +218,9 @@ void drm_connector_free_work_fn(struct work_struct *work)
 
 	llist_for_each_entry_safe(connector, n, freed, free_node) {
 		drm_mode_object_unregister(dev, &connector->base);
-		connector->funcs->destroy(connector);
+
+		if (connector->funcs->destroy)
+			connector->funcs->destroy(connector);
 	}
 }
 
-- 
2.47.3

Claude review: drm/connector: check if destroy function exist

[Claude Code Review Bot]

Overall Series Review

Subject: drm/connector: check if destroy function exist
Author: Andreas Kemnade <andreas@kemnade.info>
Patches: 1
Reviewed: 2026-05-25T20:21:15.327783

---

This is a single-patch fix for a NULL function pointer dereference in `drm_connector_free()` and `drm_connector_free_work_fn()`. The fix is correct and necessary. Connectors initialized via `drmm_connector_init()` are explicitly required to have `.destroy = NULL` (enforced by a `drm_WARN_ON` check at `drm_connector.c:533`), yet the `drm_connector_free` and `drm_connector_free_work_fn` paths unconditionally call `connector->funcs->destroy(connector)` without a NULL check. This can lead to a kernel oops when a `drmm`-managed connector's refcount drops to zero.

The fix is minimal, correct in scope, and targets the right locations. The referenced `Fixes:` tag (`c12907be57b1`) makes sense — that's when `drm_bridge_connector` was switched to `drmm` allocations, which would have introduced the possibility of reaching this code with a NULL `.destroy`.

**Verdict: Patch looks good. Recommend acceptance.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/connector: check if destroy function exist

[Claude Code Review Bot]

Patch Review

**Correctness: Good.**

The `drmm_connector_init()` function at line 525 of `drm_connector.c` explicitly documents and enforces that `.destroy` must be NULL:

```c
/* line 520 */ * The @drm_connector_funcs.destroy hook must be NULL.
...
/* line 533 */ if (drm_WARN_ON(dev, funcs && funcs->destroy))
                   return -EINVAL;
```

This means any connector using `drmm_connector_init()` (like `drm_bridge_connector` at `drm_bridge_connector.c:993`) will have `funcs->destroy == NULL`. Cleanup for these connectors is handled via `drmm_add_action_or_reset()` calling `drm_connector_cleanup_action()` (line 540), not through `.destroy`.

The two call sites fixed are:

1. **`drm_connector_free()` (line 194)** — the `kref` put callback:
```c
if (connector->funcs->destroy)
    connector->funcs->destroy(connector);
```

2. **`drm_connector_free_work_fn()` (line 206)** — the deferred-free worker:
```c
if (connector->funcs->destroy)
    connector->funcs->destroy(connector);
```

Both are correct. When `.destroy` is NULL, there is nothing to call — `drmm` handles the cleanup separately through the managed action.

**Minor observations:**

- The commit message subject has a minor grammatical issue: "check if destroy function exist" should be "check if destroy function exists". Trivial, not blocking.
- It might be worth also checking `connector->funcs` itself for NULL, but in practice `funcs` is always set during `drm_connector_init_only()` and should never be NULL at this point. The current check is sufficient.
- No concern about memory leaks: when `.destroy` is NULL (the `drmm` path), cleanup is already registered via `drmm_add_action_or_reset` calling `drm_connector_cleanup()`, and the memory was allocated with `drmm_kzalloc()` which is also automatically freed. Skipping `.destroy` here is the correct behavior — calling it would have been a double-free.

**Verdict: Correct, minimal, appropriate fix. The `Fixes:` tag is accurate.**

---
Generated by Claude Code Patch Reviewer