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/amdgpu: validate doorbell_offset in user queue creation Date: Wed, 25 Mar 2026 06:55:42 +1000 Message-ID: In-Reply-To: References: X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Positive aspects:** - Correctly identifies a missing bounds check on user-supplied input - Properly uses `(u64)` cast to prevent overflow when `doorbell_offset` (ui= nt32_t) is multiplied by `db_size` (int, value 8) - Uses the existing `unpin_bo` error path correctly - Appropriate Fixes/Cc-stable tags **Issue =E2=80=94 missing error message:** The existing error paths in this function all log a diagnostic message via = `drm_file_err()` before jumping to the error label. This new validation sil= ently returns `-EINVAL`: ```c + if ((u64)db_info->doorbell_offset * db_size + db_size > + amdgpu_bo_size(db_obj->obj)) { + r =3D -EINVAL; + goto unpin_bo; + } ``` Adding a `drm_file_err()` (or at minimum `drm_dbg_driver()`) message here w= ould be consistent with the rest of the function and helpful for debugging.= Something like: ```c drm_file_err(uq_mgr->file, "[Usermode queues] doorbell_offset %u out of range for doorbell BO\n", db_info->doorbell_offset); ``` **Minor style note:** The bounds check `(u64)db_info->doorbell_offset * db_size + db_size` comput= es `(offset + 1) * db_size`. This is correct =E2=80=94 it verifies that the= last byte of the doorbell slot at the given offset fits within the BO. It = could arguably be written more clearly as `((u64)db_info->doorbell_offset += 1) * db_size`, but the current form is also fine and equivalent. **Overall:** The fix is correct and addresses a valid security concern. Wit= h the addition of an error log message for consistency and debuggability, t= his would be ready to merge. --- Generated by Claude Code Patch Reviewer