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/amdkfd: fix NULL dereference in get_queue_ids() Date: Mon, 25 May 2026 17:31:37 +1000 Message-ID: In-Reply-To: <20260523165646.25645-1-meatuni001@gmail.com> References: <20260523165646.25645-1-meatuni001@gmail.com> <20260523165646.25645-1-meatuni001@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 original code: ```c size_t array_size =3D num_queues * sizeof(uint32_t); ``` computes `array_size` with a bare multiplication that can wrap on 32-bit ke= rnels (where `size_t` is 32 bits). With `num_queues =3D 0x40000001`, the pr= oduct `0x100000004` truncates to `0x4`, and `memdup_user()` allocates 4 byt= es. `q_array_invalidate()` then writes `num_queues` entries into that 4-byt= e buffer =E2=80=94 heap overflow. The fix: ```c size_t array_size; ... if (check_mul_overflow((size_t)num_queues, sizeof(uint32_t), &array_size)) return ERR_PTR(-EINVAL); ``` is correct. The `(size_t)` cast on `num_queues` ensures all three arguments= to `check_mul_overflow()` share the same type, avoiding implicit-type issu= es with compiler builtins. The `#include ` is appropriate= rather than relying on transitive includes. **Callers handle the error correctly**: Both `suspend_queues()` (`kfd_devic= e_queue_manager.c:3607`) and `resume_queues()` (`kfd_device_queue_manager.c= :3503`) check `IS_ERR(queue_ids)` and propagate via `PTR_ERR()`. **Note on the `copy_to_user()` calls**: The commit message correctly observ= es that the `num_queues * sizeof(uint32_t)` in `copy_to_user()` (lines 3588= , 3704) is safe because it's only reachable after `get_queue_ids()` succeed= ed with the same `num_queues`, meaning the multiplication was already valid= ated. This is sound reasoning. **No issues found.** --- Generated by Claude Code Patch Reviewer