From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: gpu: host1x: Fix device reference leak in host1x_device_parse_dt() error path Date: Mon, 13 Apr 2026 19:07:35 +1000 Message-ID: In-Reply-To: <20260412122633.2487800-1-lgs201920130244@gmail.com> References: <20260412122633.2487800-1-lgs201920130244@gmail.com> <20260412122633.2487800-1-lgs201920130244@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness: Good.** The commit message accurately describes the problem.= After `device_initialize()` at `bus.c:466`, the embedded `struct device` h= olds a reference count of 1. Using `kfree()` directly bypasses the device c= ore's lifetime management. Switching to `put_device()` is the right fix =E2= =80=94 it will decrement the refcount to 0 and invoke the `host1x_device_re= lease` callback set at `bus.c:479`, which calls `__host1x_device_del()` fol= lowed by `kfree()`. **Release callback safety:** The `__host1x_device_del()` function (`bus.c:3= 98`) is safe to call in this early failure state: - `device->active` =E2=80=94 empty, since no subdevices have been registere= d with clients. The loop at line 406 is a no-op. - `device->subdevs` =E2=80=94 may have entries from the partially-completed= `host1x_device_parse_dt()`. These are properly cleaned up by `host1x_subde= v_del()` at line 427-428, which does `list_del()`, `of_node_put()`, and `kf= ree()`. This is actually a secondary fix: the old `kfree(device)` code woul= d also leak any subdevices that were added before `host1x_device_parse_dt()= ` failed. - `device->clients` =E2=80=94 empty, loop at line 436 is a no-op. - `list_del_init(&device->list)` at line 443 =E2=80=94 safe because `INIT_L= IST_HEAD(&device->list)` was called at line 473. Calling `list_del_init` on= a self-linked list node is a no-op. **Pre-existing issue worth noting:** `host1x_device_parse_dt()` (`bus.c:88`= ) and `host1x_subdev_add()` (`bus.c:39`) have their own cleanup gap =E2=80= =94 when `host1x_subdev_add()` fails mid-recursion at line 62-64 (note the = `/* XXX cleanup? */` comment), already-added subdevices from earlier iterat= ions are left on the list without cleanup. However, with this patch applied= , those orphaned subdevices are now properly freed through the `put_device(= )` =E2=86=92 `host1x_device_release()` =E2=86=92 `__host1x_device_del()` pa= th, which iterates and frees all entries on `device->subdevs`. So this patc= h incidentally fixes that leak too. **Minor nits:** - The commit message title says "in host1x_device_parse_dt() error path" = =E2=80=94 it would be slightly more precise as "in host1x_device_add() erro= r path" since the fix is in `host1x_device_add()`, not in `host1x_device_pa= rse_dt()` itself. But this is a cosmetic quibble. - The Fixes tag references `f4c5cf88fbd50`, which introduced `device_initia= lize()` usage. This is appropriate. **Reviewed-by recommendation:** This patch is straightforward, correct, and= appropriate for stable backport. --- Generated by Claude Code Patch Reviewer