[PATCH] gpu: host1x: Fix device reference leak in device_add() error path

[PATCH] gpu: host1x: Fix device reference leak in device_add() error path

[Guangshuo Li]

After device_initialize(), the embedded struct device in struct
host1x_device should be released through the device core with
put_device().

In host1x_device_add(), the empty-subdevice path calls
device_add(&device->dev), but if that fails it only logs the error and
continues without dropping the device reference. That leaks the
reference held on the embedded struct device.

Fix this by removing the device from host1x->devices and calling
put_device() when device_add() fails.

Fixes: fab823d82ee50 ("gpu: host1x: Allow loading tegra-drm without enabled engines")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/gpu/host1x/bus.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index f97567e6ae87..e3ac85848aec 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -477,8 +477,12 @@ static int host1x_device_add(struct host1x *host1x,
 	 */
 	if (list_empty(&device->subdevs)) {
 		err = device_add(&device->dev);
-		if (err < 0)
+		if (err < 0) {
 			dev_err(&device->dev, "failed to add device: %d\n", err);
+			list_del(&device->list);
+			put_device(&device->dev);
+			return err;
+		}
 		else
 			device->registered = true;
 	}
-- 
2.43.0

Claude review: gpu: host1x: Fix device reference leak in device_add() error path

[Claude Code Review Bot]

Overall Series Review

Subject: gpu: host1x: Fix device reference leak in device_add() error path
Author: Guangshuo Li <lgs201920130244@gmail.com>
Patches: 1
Reviewed: 2026-04-13T19:10:39.741073

---

This is a single-patch fix targeting a device reference leak in the `host1x_device_add()` error path when `device_add()` fails for a device with no subdevices. The identified bug is real: the original code logs the error but falls through to `return 0`, leaking the device reference and leaving a half-initialized device on `host1x->devices`.

However, **the proposed fix introduces a new bug** — a double list removal that will crash or warn on most kernel configurations. The `list_del()` call poisons the list pointers, and then `put_device()` triggers the release callback (`host1x_device_release` → `__host1x_device_del`) which calls `list_del_init()` on those same poisoned pointers. On kernels without `CONFIG_DEBUG_LIST` / `CONFIG_LIST_HARDENED` (i.e., most production kernels), this dereferences `LIST_POISON1`/`LIST_POISON2` and causes a kernel oops.

**Recommendation: Needs revision (NAK as-is).**

---

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: host1x: Fix device reference leak in device_add() error path

[Claude Code Review Bot]

Patch Review

**Bug identification is correct.** In the original code at `bus.c:511-520` (pre-patch), when `device_add()` fails:
```c
if (list_empty(&device->subdevs)) {
    err = device_add(&device->dev);
    if (err < 0)
        dev_err(&device->dev, "failed to add device: %d\n", err);
    else
        device->registered = true;
}

return 0;
```
The function always returns 0, the device stays on `host1x->devices`, and the reference from `device_initialize()` is never dropped. This is indeed a leak.

**Critical bug in the fix — double list removal:**

The patch adds:
```c
if (err < 0) {
    dev_err(&device->dev, "failed to add device: %d\n", err);
    list_del(&device->list);       /* poisons prev/next */
    put_device(&device->dev);      /* triggers release callback */
    return err;
}
```

`put_device()` drops the last reference, triggering `host1x_device_release()` (set at `bus.c:479`):
```c
static void host1x_device_release(struct device *dev)
{
    struct host1x_device *device = to_host1x_device(dev);
    __host1x_device_del(device);
    kfree(device);
}
```

And `__host1x_device_del()` at line 443 does:
```c
list_del_init(&device->list);
```

Since `list_del()` already poisoned the pointers with `LIST_POISON1`/`LIST_POISON2`, this second removal dereferences those sentinel addresses. Without `CONFIG_DEBUG_LIST`, `__list_del_entry_valid()` returns `true` unconditionally and the kernel will oops trying to write through poisoned pointers.

**Fix:** Remove the `list_del(&device->list)` line entirely. The `put_device()` → `host1x_device_release` → `__host1x_device_del` path already handles list removal at line 443. So the error handling should be simply:
```c
if (err < 0) {
    dev_err(&device->dev, "failed to add device: %d\n", err);
    put_device(&device->dev);
    return err;
}
```

Alternatively, if the author wants to keep the explicit `list_del`, use `list_del_init()` instead so the entry points to itself and the subsequent `list_del_init()` in `__host1x_device_del` is a harmless no-op. But this is less clean since it duplicates logic already in the release path.

**Minor style issue:** The patch produces:
```c
    }
    else
        device->registered = true;
```
Per kernel coding style (Chapter 3), when the `if` branch uses braces, the `else` should be on the same line as the closing brace and should also use braces:
```c
    } else {
        device->registered = true;
    }
```
The original code had no braces on either branch so it was consistent; now it's mixed.

**Pre-existing issue (not this patch's fault):** The `host1x_device_parse_dt()` error path at line 487-489 uses `kfree(device)` directly after `device_initialize()`, which is technically incorrect (should use `put_device()`). However, it happens to be safe because the device isn't on any list at that point, and the release function's `list_del_init` on a self-pointing entry is benign. Mentioning this only for completeness — it shouldn't be addressed in this patch.

---
Generated by Claude Code Patch Reviewer