public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/dumb-buffer: Drop buffer-size limits for now
@ 2026-06-02 11:24 Thomas Zimmermann
  2026-06-03  8:53 ` Saarinen, Jani
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2026-06-02 11:24 UTC (permalink / raw)
  To: rajat.gupta, jani.nikula, jani.saarinen, simona, airlied, mripard,
	maarten.lankhorst
  Cc: dri-devel, intel-gfx, intel-xe, Thomas Zimmermann

The size limits break some of the CI tests. So drop them for now. Keep
the other overflow tests from commit 5ab62dd3687b ("drm: prevent integer
overflows in dumb buffer creation helpers") in place.

There is still a pre-existing overflow check for 32-bit type limits in
drm_mode_create_dumb() that will catch the really absurd size requests.
Drivers that still do not use drm_mode_size_dumb() should be updated. The
helper calculates dumb-buffer geometry with overflow checks.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 5ab62dd3687b ("drm: prevent integer overflows in dumb buffer creation helpers")
Reported-by: Jani Nikula <jani.nikula@linux.intel.com>
Closes: https://lore.kernel.org/dri-devel/ddf0233e50044059c85279f928661563ef6a55bf@intel.com/
Cc: Rajat Gupta <rajat.gupta@oss.qualcomm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_dumb_buffers.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index e60130b2bb0c..8e9ff17538e7 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -201,13 +201,6 @@ int drm_mode_create_dumb(struct drm_device *dev,
 	if (!args->width || !args->height || !args->bpp)
 		return -EINVAL;
 
-	/* Reject unreasonable inputs early.  Dumb buffers are for software
-	 * rendering; nothing legitimate needs more than 8192x8192 at 32bpp.
-	 * This prevents overflows in downstream alignment helpers.
-	 */
-	if (args->width >= 8192 || args->height >= 8192 || args->bpp > 32)
-		return -EINVAL;
-
 	/* overflow checks for 32bit size calculations */
 	if (args->bpp > U32_MAX - 8)
 		return -EINVAL;

base-commit: a980196655477a8f5067112946401fe52e510664
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH] drm/dumb-buffer: Drop buffer-size limits for now
  2026-06-02 11:24 [PATCH] drm/dumb-buffer: Drop buffer-size limits for now Thomas Zimmermann
@ 2026-06-03  8:53 ` Saarinen, Jani
  2026-06-03 10:30 ` Jani Nikula
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Saarinen, Jani @ 2026-06-03  8:53 UTC (permalink / raw)
  To: Thomas Zimmermann, rajat.gupta@oss.qualcomm.com,
	jani.nikula@linux.intel.com, simona@ffwll.ch, airlied@gmail.com,
	mripard@kernel.org, maarten.lankhorst@linux.intel.com
  Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org

Hi, 
> -----Original Message-----
> From: Thomas Zimmermann <tzimmermann@suse.de>
> Sent: Tuesday, 2 June 2026 14.24
> To: rajat.gupta@oss.qualcomm.com; jani.nikula@linux.intel.com; Saarinen,
> Jani <jani.saarinen@intel.com>; simona@ffwll.ch; airlied@gmail.com;
> mripard@kernel.org; maarten.lankhorst@linux.intel.com
> Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; Thomas Zimmermann <tzimmermann@suse.de>
> Subject: [PATCH] drm/dumb-buffer: Drop buffer-size limits for now
> 
> The size limits break some of the CI tests. So drop them for now. Keep the
> other overflow tests from commit 5ab62dd3687b ("drm: prevent integer
> overflows in dumb buffer creation helpers") in place.
> 
> There is still a pre-existing overflow check for 32-bit type limits in
> drm_mode_create_dumb() that will catch the really absurd size requests.
> Drivers that still do not use drm_mode_size_dumb() should be updated. The
> helper calculates dumb-buffer geometry with overflow checks.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 5ab62dd3687b ("drm: prevent integer overflows in dumb buffer
> creation helpers")
> Reported-by: Jani Nikula <jani.nikula@linux.intel.com>
> Closes: https://lore.kernel.org/dri-
> devel/ddf0233e50044059c85279f928661563ef6a55bf@intel.com/
> Cc: Rajat Gupta <rajat.gupta@oss.qualcomm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
Seems to satisfy CI :
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167736v1/index.html?testfilter=vgem%7Ckms_big.*line

Br,
Jani
> ---
>  drivers/gpu/drm/drm_dumb_buffers.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
> b/drivers/gpu/drm/drm_dumb_buffers.c
> index e60130b2bb0c..8e9ff17538e7 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -201,13 +201,6 @@ int drm_mode_create_dumb(struct drm_device
> *dev,
>  	if (!args->width || !args->height || !args->bpp)
>  		return -EINVAL;
> 
> -	/* Reject unreasonable inputs early.  Dumb buffers are for software
> -	 * rendering; nothing legitimate needs more than 8192x8192 at
> 32bpp.
> -	 * This prevents overflows in downstream alignment helpers.
> -	 */
> -	if (args->width >= 8192 || args->height >= 8192 || args->bpp > 32)
> -		return -EINVAL;
> -
>  	/* overflow checks for 32bit size calculations */
>  	if (args->bpp > U32_MAX - 8)
>  		return -EINVAL;
> 
> base-commit: a980196655477a8f5067112946401fe52e510664
> --
> 2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/dumb-buffer: Drop buffer-size limits for now
  2026-06-02 11:24 [PATCH] drm/dumb-buffer: Drop buffer-size limits for now Thomas Zimmermann
  2026-06-03  8:53 ` Saarinen, Jani
@ 2026-06-03 10:30 ` Jani Nikula
  2026-06-04  2:51 ` Claude review: " Claude Code Review Bot
  2026-06-04  2:51 ` Claude Code Review Bot
  3 siblings, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2026-06-03 10:30 UTC (permalink / raw)
  To: Thomas Zimmermann, rajat.gupta, jani.saarinen, simona, airlied,
	mripard, maarten.lankhorst
  Cc: dri-devel, intel-gfx, intel-xe, Thomas Zimmermann

On Tue, 02 Jun 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> The size limits break some of the CI tests. So drop them for now. Keep
> the other overflow tests from commit 5ab62dd3687b ("drm: prevent integer
> overflows in dumb buffer creation helpers") in place.
>
> There is still a pre-existing overflow check for 32-bit type limits in
> drm_mode_create_dumb() that will catch the really absurd size requests.
> Drivers that still do not use drm_mode_size_dumb() should be updated. The
> helper calculates dumb-buffer geometry with overflow checks.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 5ab62dd3687b ("drm: prevent integer overflows in dumb buffer creation helpers")
> Reported-by: Jani Nikula <jani.nikula@linux.intel.com>
> Closes: https://lore.kernel.org/dri-devel/ddf0233e50044059c85279f928661563ef6a55bf@intel.com/
> Cc: Rajat Gupta <rajat.gupta@oss.qualcomm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>

Thanks for following up. Maybe we need to return to this with less
urgency.

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/drm_dumb_buffers.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index e60130b2bb0c..8e9ff17538e7 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -201,13 +201,6 @@ int drm_mode_create_dumb(struct drm_device *dev,
>  	if (!args->width || !args->height || !args->bpp)
>  		return -EINVAL;
>  
> -	/* Reject unreasonable inputs early.  Dumb buffers are for software
> -	 * rendering; nothing legitimate needs more than 8192x8192 at 32bpp.
> -	 * This prevents overflows in downstream alignment helpers.
> -	 */
> -	if (args->width >= 8192 || args->height >= 8192 || args->bpp > 32)
> -		return -EINVAL;
> -
>  	/* overflow checks for 32bit size calculations */
>  	if (args->bpp > U32_MAX - 8)
>  		return -EINVAL;
>
> base-commit: a980196655477a8f5067112946401fe52e510664

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: drm/dumb-buffer: Drop buffer-size limits for now
  2026-06-02 11:24 [PATCH] drm/dumb-buffer: Drop buffer-size limits for now Thomas Zimmermann
  2026-06-03  8:53 ` Saarinen, Jani
  2026-06-03 10:30 ` Jani Nikula
@ 2026-06-04  2:51 ` Claude Code Review Bot
  2026-06-04  2:51 ` Claude Code Review Bot
  3 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-06-04  2:51 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/dumb-buffer: Drop buffer-size limits for now
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 3
Reviewed: 2026-06-04T12:51:35.907861

---

This is a single patch that removes overly restrictive hard-coded size limits (8192x8192 @ 32bpp) from `drm_mode_create_dumb()`, added by the recent commit 5ab62dd3687b. The limits broke CI tests and were too aggressive — they'd reject legitimate use cases like 8K displays (7680x4320) and 64bpp formats (`XRGB16161616F`) that are explicitly supported elsewhere in the same file (line 176 of the current tree). The removal is correct: the remaining arithmetic overflow checks in `drm_mode_create_dumb()` are mathematically sound and sufficient to prevent the integer overflows the original commit was targeting.

**Verdict: The patch is good and should be applied.** Minor nits on tag ordering below.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: drm/dumb-buffer: Drop buffer-size limits for now
  2026-06-02 11:24 [PATCH] drm/dumb-buffer: Drop buffer-size limits for now Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2026-06-04  2:51 ` Claude review: " Claude Code Review Bot
@ 2026-06-04  2:51 ` Claude Code Review Bot
  3 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-06-04  2:51 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good**

The removed check was:
```c
-	if (args->width >= 8192 || args->height >= 8192 || args->bpp > 32)
-		return -EINVAL;
```

This was problematic for two reasons:

1. **The `bpp > 32` check contradicts `drm_mode_size_dumb()`**, which explicitly 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 >= 8192` check is too restrictive** — 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 `>=` rather than `>` means even exactly 8192 fails.

**Remaining overflow protection is adequate.** The existing checks that remain:

```c
if (args->bpp > U32_MAX - 8)        // prevents DIV_ROUND_UP overflow
    return -EINVAL;
cpp = DIV_ROUND_UP(args->bpp, 8);
if (cpp > U32_MAX / args->width)     // prevents pitch overflow
    return -EINVAL;
stride = cpp * args->width;
if (args->height > U32_MAX / stride) // prevents size overflow
    return -EINVAL;
size = args->height * stride;
if (PAGE_ALIGN(size) == 0)           // prevents wrap-around
    return -EINVAL;
```

These four checks form a complete chain that prevents every 32-bit arithmetic overflow in the computation path. The hard size limits were belt-and-suspenders but broke real users.

**Downstream driver concern acknowledged.** The commit message correctly notes that drivers not yet using `drm_mode_size_dumb()` should be updated. Those drivers do their own alignment math in their `dumb_create` callbacks and may still be individually vulnerable to overflow, but that's a pre-existing issue not introduced by this patch.

**Minor nit — tag ordering.** The `Fixes:` tag appears after `Signed-off-by:`, which is unconventional. Kernel convention typically places `Fixes:` before `Signed-off-by:` in the trailer block:

```
Fixes: 5ab62dd3687b ("drm: prevent integer overflows in dumb buffer creation helpers")
Reported-by: Jani Nikula <jani.nikula@linux.intel.com>
Closes: https://lore.kernel.org/dri-devel/...
Cc: ...
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
```

This is cosmetic and doesn't affect tooling, but worth fixing if a v2 is sent for other reasons.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-04  2:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-02 11:24 [PATCH] drm/dumb-buffer: Drop buffer-size limits for now Thomas Zimmermann
2026-06-03  8:53 ` Saarinen, Jani
2026-06-03 10:30 ` Jani Nikula
2026-06-04  2:51 ` Claude review: " Claude Code Review Bot
2026-06-04  2:51 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox