[PATCH] drm/bridge: it6505: fix use-after-free in it6505_parse_dt()

[PATCH] drm/bridge: it6505: fix use-after-free in it6505_parse_dt()

[Wentao Liang]

In it6505_parse_dt(), of_node_put(ep) is called prematurely before the
last access to ep when reading the "link-frequencies" property, leading
to a use-after-free if the node's reference count drops to zero. Move
the of_node_put() calls after the last use of ep in both the success
and error paths.

Fixes: 380d920b582d ("drm/bridge: add it6505 driver to read data-lanes and link-frequencies from dt")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index a094803ba7aa..4155b5e67c2d 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -3361,13 +3361,13 @@ static void it6505_parse_dt(struct it6505 *it6505)
 	}
 
 	ep = of_graph_get_endpoint_by_regs(np, 0, 0);
-	of_node_put(ep);
 
 	if (ep) {
 		len = of_property_read_variable_u64_array(ep,
 							  "link-frequencies",
 							  &link_frequencies, 0,
 							  1);
+		of_node_put(ep);
 		if (len >= 0) {
 			do_div(link_frequencies, 1000);
 			if (link_frequencies > 297000) {
@@ -3382,6 +3382,7 @@ static void it6505_parse_dt(struct it6505 *it6505)
 			*max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
 		}
 	} else {
+		of_node_put(ep);
 		dev_err(dev, "error endpoint, use default");
 		*max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
 	}
-- 
2.34.1

Claude review: drm/bridge: it6505: fix use-after-free in it6505_parse_dt()

[Claude Code Review Bot]

Overall Series Review

Subject: drm/bridge: it6505: fix use-after-free in it6505_parse_dt()
Author: Wentao Liang <vulab@iscas.ac.cn>
Patches: 1
Reviewed: 2026-04-12T11:30:02.603337

---

This is a single patch claiming to fix a use-after-free in `it6505_parse_dt()`. The identified bug is real — the original code calls `of_node_put(ep)` before `ep` is subsequently passed to `of_property_read_variable_u64_array()`, which is a use-after-free if the refcount drops to zero. However, the patch has two issues: (1) it adds a spurious `of_node_put(ep)` in the `else` branch where `ep` is guaranteed to be NULL, and (2) it fails to fix the **exact same bug** present earlier in the same function for the first `of_graph_get_endpoint_by_regs()` call.

**Verdict: Needs revision.** The core fix is correct for the second endpoint lookup, but the patch is incomplete — the same use-after-free pattern exists a few lines above and is left unfixed.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: it6505: fix use-after-free in it6505_parse_dt()

[Claude Code Review Bot]

Patch Review

**The identified bug is real.** In the original code:
```c
ep = of_graph_get_endpoint_by_regs(np, 0, 0);
of_node_put(ep);

if (ep) {
    len = of_property_read_variable_u64_array(ep, /* use-after-free */
                          "link-frequencies",
                          &link_frequencies, 0, 1);
```
`of_node_put(ep)` is called before `ep` is used in `of_property_read_variable_u64_array()`. Moving the put after the last use of `ep` is the correct fix for this instance.

**Issue 1 — Unnecessary `of_node_put(ep)` in the `else` branch.** The patch adds:
```c
} else {
    of_node_put(ep);
    dev_err(dev, "error endpoint, use default");
```
The `else` branch is only entered when `ep == NULL`. `of_node_put(NULL)` is a no-op (it checks for NULL internally), so this call is dead code. It's harmless but misleading — it gives the impression that a real reference is being released. This should be removed.

**Issue 2 — Same bug exists at lines 3344–3353 (unfixed).** The exact same use-after-free pattern exists earlier in the function for the *first* endpoint lookup:
```c
ep = of_graph_get_endpoint_by_regs(np, 1, 0);   /* line 3344 */
of_node_put(ep);                                  /* line 3345 — too early! */

if (ep) {
    len = it6505_get_data_lanes_count(ep, 1, 4);  /* line 3348 — use-after-free */

    if (len > 0 && len != 3) {
        of_property_read_u32_array(ep, "data-lanes",  /* line 3351 — use-after-free */
                       data_lanes, len);
```
Here `ep` is used in both `it6505_get_data_lanes_count()` and `of_property_read_u32_array()` after the refcount has already been dropped. A complete fix should address both instances. Since the commit message says "fix use-after-free in it6505_parse_dt()" and the Fixes tag references the commit that introduced *both* patterns, readers would expect both to be fixed.

**Recommendation:** Resubmit with both instances fixed, and drop the no-op `of_node_put(ep)` from the `else` branches. The fix for both should be the same: move `of_node_put(ep)` to inside the `if (ep)` block, after the last use of `ep`.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: it6505: fix use-after-free in it6505_parse_dt()

[Claude Code Review Bot]

Overall Series Review

Subject: drm/bridge: it6505: fix use-after-free in it6505_parse_dt()
Author: Wentao Liang <vulab@iscas.ac.cn>
Patches: 1
Reviewed: 2026-04-12T13:40:31.451726

---

This is a single patch claiming to fix a use-after-free in `it6505_parse_dt()`. The claim is that `of_node_put(ep)` is called before `ep` is used inside the `if (ep)` block. While the diagnosis of a real bug is correct — calling `of_node_put()` before using the node is wrong — there is a significant gap in the fix: it only places `of_node_put(ep)` on the `if (ep)` (true) path and **omits it on the `else` path and on early-return/error paths**. However, since `ep` is `NULL` in the `else` branch, `of_node_put(NULL)` is a no-op, so this omission is actually correct behavior (not a leak). The fix is functionally correct.

**Verdict: The patch is correct and fixes a real bug, but is cosmetically imperfect in its commit message framing.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: it6505: fix use-after-free in it6505_parse_dt()

[Claude Code Review Bot]

Patch Review

**The bug is real.** In the original code, the pattern is:

```c
ep = of_graph_get_endpoint_by_regs(np, 1, 0);
of_node_put(ep);          // <-- drops refcount immediately

if (ep) {
    len = it6505_get_data_lanes_count(ep, 1, 4);  // <-- uses freed node
    ...
```

`of_graph_get_endpoint_by_regs()` returns a refcounted `device_node`. Calling `of_node_put()` immediately can drop the refcount to zero and free the node, making the subsequent dereference inside the `if (ep)` block a use-after-free. This same pattern occurs twice — once for endpoint (1,0) and once for endpoint (0,0).

**The fix is correct.** The patch moves `of_node_put(ep)` to the end of each `if (ep)` block, after `ep` is no longer needed:

```c
ep = of_graph_get_endpoint_by_regs(np, 1, 0);

if (ep) {
    len = it6505_get_data_lanes_count(ep, 1, 4);
    ...
    of_node_put(ep);       // <-- moved here, after all uses
} else {
    ...
}
```

**Observations:**

1. **The `else` branches don't need `of_node_put()`** — when `ep` is `NULL`, `of_node_put(NULL)` is a documented no-op in the kernel, so omitting it is fine and arguably cleaner.

2. **The fix is applied consistently** to both occurrences of the pattern in the function (endpoint regs (1,0) and (0,0)).

3. **Minor nit on the commit message:** The phrase "the node may be freed before being accessed in the if (ep) block" is slightly misleading. `of_node_put()` *does* drop the reference, and if the refcount reaches zero the node *will* be freed — it's not just "may be." In practice, `of_graph_get_endpoint_by_regs()` typically returns a node with refcount 1, so `of_node_put()` will free it. A more precise commit message would say "the node is freed before being accessed."

4. **Cc: stable is appropriate** given this is a real use-after-free bug, and the `Fixes:` tag correctly identifies commit `380d920b582d` which introduced this buggy pattern.

5. **No functional concerns** — the surrounding logic is unchanged, and the ref is properly balanced (one `of_graph_get_endpoint_by_regs()` get, one `of_node_put()` put, on the only path where `ep` is non-NULL).

**Reviewed-by: Looks good.** The patch correctly fixes a real use-after-free. The only suggestion would be a minor commit message wording improvement ("is freed" vs "may be freed"), which is not worth a respin.

---
Generated by Claude Code Patch Reviewer