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/dumb-buffers: Increase size limits to match current devices Date: Thu, 04 Jun 2026 13:08:23 +1000 Message-ID: In-Reply-To: <20260602073027.192758-1-tzimmermann@suse.de> References: <20260602073027.192758-1-tzimmermann@suse.de> <20260602073027.192758-1-tzimmermann@suse.de> 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 of the new limits:** The worst-case arithmetic at the new limits (16384 x 16384 x 64bpp): - `cpp =3D DIV_ROUND_UP(64, 8) =3D 8` - `stride =3D 8 * 16384 =3D 131,072` - `size =3D 16384 * 131,072 =3D 2,147,483,648` (0x80000000) This fits within `u32` (max ~4.29 billion) and `PAGE_ALIGN(0x80000000)` is = non-zero, so all downstream overflow checks in the old `drm_mode_create_dum= b()` pass. The arithmetic is safe. **The `>=3D` to `>` change is correct and intentional:** ```c - if (args->width >=3D 8192 || args->height >=3D 8192 || args->bpp > 32) + if (args->width > 16384 || args->height > 16384 || args->bpp > 64) ``` The old code rejected `width =3D=3D 8192` (off-by-one =E2=80=94 the comment= said "8192x8192" but the `>=3D` operator excluded that size). The new code= correctly allows `width =3D=3D 16384` using `>`. This is a good fix. **64bpp is a legitimate value:** The current drm-next tree explicitly handles `case 64` in `drm_mode_size_du= mb()` for `DRM_FORMAT_{XRGB,XBGR,ARGB,ABGR}16161616F` formats, confirming 6= 4bpp is a real use case. The original 32bpp limit was too restrictive. **Comment update is appropriate:** ```c + /* + * Reject unreasonable inputs early. Dumb buffers are for software + * rendering; nothing legitimate needs more than 16384x16384 at + * 64 bpp. This prevents overflows in downstream alignment helpers. */ ``` The comment accurately describes the new limits and the multi-line comment = style is reformatted to kernel coding style (opening `/*` on its own line). **Minor observations:** 1. The `Fixes:` tag placement is after `Signed-off-by:` =E2=80=94 while not= strictly wrong, the conventional ordering in the kernel is `Fixes:` before= `Signed-off-by:`. This is a trivial style point. 2. The patch is well-scoped and minimal =E2=80=94 it changes exactly what n= eeds to change for the fix. 3. As noted above, this patch is **superseded on drm-next** where the hardc= oded limits have been entirely replaced with overflow-safe arithmetic. If t= his is intended for stable backport to kernels that still have the original= limiting code, it looks correct. If it's intended for drm-next, it will ne= ed to be dropped or reworked since the target code no longer exists. **Verdict:** The patch is correct for the codebase version it targets. The = arithmetic is safe within u32 bounds, the off-by-one fix (`>=3D` to `>`) is= an improvement, and 64bpp / 16384 resolution are justified by real hardwar= e and formats. The main concern is applicability =E2=80=94 this should be a= pplied to the branch containing `5ab62dd3687b`, not drm-next where the code= has been refactored away. --- Generated by Claude Code Patch Reviewer