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-buffer: Drop buffer-size limits for now Date: Thu, 04 Jun 2026 12:51:36 +1000 Message-ID: In-Reply-To: <20260602112842.252279-1-tzimmermann@suse.de> References: <20260602112842.252279-1-tzimmermann@suse.de> <20260602112842.252279-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: Good** The removed check was: ```c - if (args->width >=3D 8192 || args->height >=3D 8192 || args->bpp > 32) - return -EINVAL; ``` This was problematic for two reasons: 1. **The `bpp > 32` check contradicts `drm_mode_size_dumb()`**, which expli= citly supports `case 64:` (for `XRGB16161616F` and friends) at line 176 of = the current tree. Any driver using `drm_mode_size_dumb()` for 64bpp formats= would have the request rejected before reaching the helper. 2. **The `width >=3D 8192` check is too restrictive** =E2=80=94 8K displays= are 7680x4320, which barely passes, but any buffer slightly wider than the= display (e.g., with stride alignment padding) would be rejected. The `>=3D= ` rather than `>` means even exactly 8192 fails. **Remaining overflow protection is adequate.** The existing checks that rem= ain: ```c if (args->bpp > U32_MAX - 8) // prevents DIV_ROUND_UP overflow return -EINVAL; cpp =3D DIV_ROUND_UP(args->bpp, 8); if (cpp > U32_MAX / args->width) // prevents pitch overflow return -EINVAL; stride =3D cpp * args->width; if (args->height > U32_MAX / stride) // prevents size overflow return -EINVAL; size =3D args->height * stride; if (PAGE_ALIGN(size) =3D=3D 0) // prevents wrap-around return -EINVAL; ``` These four checks form a complete chain that prevents every 32-bit arithmet= ic overflow in the computation path. The hard size limits were belt-and-sus= penders but broke real users. **Downstream driver concern acknowledged.** The commit message correctly no= tes that drivers not yet using `drm_mode_size_dumb()` should be updated. Th= ose drivers do their own alignment math in their `dumb_create` callbacks an= d may still be individually vulnerable to overflow, but that's a pre-existi= ng issue not introduced by this patch. **Minor nit =E2=80=94 tag ordering.** The `Fixes:` tag appears after `Signe= d-off-by:`, which is unconventional. Kernel convention typically places `Fi= xes:` before `Signed-off-by:` in the trailer block: ``` Fixes: 5ab62dd3687b ("drm: prevent integer overflows in dumb buffer creatio= n helpers") Reported-by: Jani Nikula Closes: https://lore.kernel.org/dri-devel/... Cc: ... Signed-off-by: Thomas Zimmermann ``` This is cosmetic and doesn't affect tooling, but worth fixing if a v2 is se= nt for other reasons. --- Generated by Claude Code Patch Reviewer