From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups Date: Thu, 04 Jun 2026 15:15:36 +1000 Message-ID: In-Reply-To: <20260530-v3d-fix-indirect-csd-v1-1-15533948663f@igalia.com> References: <20260530-v3d-fix-indirect-csd-v1-0-15533948663f@igalia.com> <20260530-v3d-fix-indirect-csd-v1-1-15533948663f@igalia.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 **Assessment: Good. No issues.** This is a straightforward and correct fix. The function `v3d_rewrite_csd_jo= b_wg_counts_from_indirect()` maps two BOs via `v3d_get_bo_vaddr()` (which c= alls `vmap()` internally), then checks if any workgroup count is zero: ```c if (wg_counts[0] =3D=3D 0 || wg_counts[1] =3D=3D 0 || wg_counts[2] =3D=3D 0) return; ``` The bare `return` skips the cleanup at the end of the function: ```c v3d_put_bo_vaddr(indirect); v3d_put_bo_vaddr(bo); ``` The fix correctly changes `return` to `goto unmap_bo` and adds the label be= fore the cleanup calls. The unmap order (indirect first, then bo) matches t= he reverse of the mapping order, which is good practice. The label name `un= map_bo` is a reasonable label =E2=80=94 arguably `unmap` would be more prec= ise since it unmaps both `bo` and `indirect`, but this follows common kerne= l style where the label names the first cleanup action. --- --- Generated by Claude Code Patch Reviewer