public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/amdgpu,radeon: fix integer overflow in pitch alignment
@ 2026-04-06 22:50 Werner Kasselman
  2026-04-06 22:50 ` [PATCH 1/2] drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch() Werner Kasselman
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Werner Kasselman @ 2026-04-06 22:50 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: David Airlie, Simona Vetter, Thomas Zimmermann,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org

Both amdgpu_gem_align_pitch() and radeon_align_pitch() use signed int
for the pitch calculation. When alignment rounding pushes the width to
a boundary value, 'aligned * cpp' overflows signed 32-bit int to 0.

This defeats the overflow guards in drm_mode_create_dumb() because
these drivers bypass drm_mode_size_dumb() and perform their own
alignment rounding, which can push the pitch past the pre-validated
range.

A zero pitch propagates to a zero-size GEM object allocation reachable
from unprivileged userspace via DRM_IOCTL_MODE_CREATE_DUMB on the
render node.

Both drivers need the same fix: add an overflow check in the alignment
function and reject zero pitch/size in the dumb_create callback. The
proper long-term fix is to convert both drivers to use
drm_mode_size_dumb() as Thomas Zimmermann's series is doing for other
drivers.

Werner Kasselman (2):
  drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch()
  drm/radeon: fix integer overflow in radeon_align_pitch()

 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
 drivers/gpu/drm/radeon/radeon_gem.c     |  9 +++++++++
 2 files changed, 22 insertions(+)

-- 
2.43.0


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

* [PATCH 1/2] drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch()
  2026-04-06 22:50 [PATCH 0/2] drm/amdgpu,radeon: fix integer overflow in pitch alignment Werner Kasselman
@ 2026-04-06 22:50 ` Werner Kasselman
  2026-04-12  4:15   ` Claude review: " Claude Code Review Bot
  2026-04-06 22:50 ` [PATCH 2/2] drm/radeon: fix integer overflow in radeon_align_pitch() Werner Kasselman
  2026-04-12  4:15 ` Claude review: drm/amdgpu,radeon: fix integer overflow in pitch alignment Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Werner Kasselman @ 2026-04-06 22:50 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: David Airlie, Simona Vetter, Thomas Zimmermann,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org

amdgpu_gem_align_pitch() uses signed int for the pitch calculation.
When alignment rounding pushes the width to a boundary value (e.g.,
2^30 for cpp=4), the multiplication 'aligned * cpp' overflows signed
32-bit int, producing 0 or a negative value.

The overflow guard in drm_mode_create_dumb() validates width * cpp
BEFORE the driver callback, but amdgpu_mode_dumb_create() bypasses the
generic drm_mode_size_dumb() helper and performs its own alignment
rounding, which can push the pitch past the pre-validated range.

A zero pitch propagates to a zero-size GEM object allocation via
amdgpu_gem_object_create(). The 0-byte BO passes
amdgpu_bo_validate_size() (since 0 < man->size) and is returned to
userspace with a valid handle. This object can then be mmap'd or
referenced in GPU command submissions, potentially causing out-of-bounds
access to adjacent slab memory.

DRM_IOCTL_MODE_CREATE_DUMB requires no DRM authentication, so any local
user with access to /dev/dri/renderD* can trigger this with e.g.
width=1073741760, bpp=32, height=1.

Add an overflow check in amdgpu_gem_align_pitch() to detect when
'aligned * cpp' would exceed INT_MAX, returning 0 in that case. Add
corresponding checks in amdgpu_mode_dumb_create() to reject pitch=0
and size=0 with -EINVAL.

The proper long-term fix is to convert amdgpu to use
drm_mode_size_dumb() which centralizes pitch/size calculation with
proper overflow guards, as is being done for other drivers in Thomas
Zimmermann's dumb-buffer series.

Found via AST-based call-graph analysis using sqry.

Fixes: 087451f372bf ("drm/amdgpu: use generic fb helpers instead of setting up AMD own's.")
Cc: stable@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index a6107109a2b8..b4341abba20c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -1246,6 +1246,15 @@ static int amdgpu_gem_align_pitch(struct amdgpu_device *adev,
 
 	aligned += pitch_mask;
 	aligned &= ~pitch_mask;
+
+	/* Sanity check to avoid integer overflow in aligned * cpp.
+	 * The caller (drm_mode_create_dumb) validates width * cpp fits
+	 * in u32 before alignment, but rounding up can push aligned
+	 * past INT_MAX / cpp, causing signed overflow to 0 or negative.
+	 */
+	if (aligned > INT_MAX / (cpp ? cpp : 1) || aligned <= 0)
+		return 0;
+
 	return aligned * cpp;
 }
 
@@ -1273,8 +1282,12 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
 
 	args->pitch = amdgpu_gem_align_pitch(adev, args->width,
 					     DIV_ROUND_UP(args->bpp, 8), 0);
+	if (!args->pitch)
+		return -EINVAL;
 	args->size = (u64)args->pitch * args->height;
 	args->size = ALIGN(args->size, PAGE_SIZE);
+	if (!args->size)
+		return -EINVAL;
 	domain = amdgpu_bo_get_preferred_domain(adev,
 				amdgpu_display_supported_domains(adev, flags));
 	r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
-- 
2.43.0


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

* [PATCH 2/2] drm/radeon: fix integer overflow in radeon_align_pitch()
  2026-04-06 22:50 [PATCH 0/2] drm/amdgpu,radeon: fix integer overflow in pitch alignment Werner Kasselman
  2026-04-06 22:50 ` [PATCH 1/2] drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch() Werner Kasselman
@ 2026-04-06 22:50 ` Werner Kasselman
  2026-04-12  4:15   ` Claude review: " Claude Code Review Bot
  2026-04-12  4:15 ` Claude review: drm/amdgpu,radeon: fix integer overflow in pitch alignment Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Werner Kasselman @ 2026-04-06 22:50 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: David Airlie, Simona Vetter, Thomas Zimmermann,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org

radeon_align_pitch() has the same integer overflow as amdgpu's variant:
'aligned * cpp' can overflow signed int to 0 when alignment rounding
pushes the width past INT_MAX/cpp. This produces a 0-byte GEM buffer
via radeon_mode_dumb_create(), reachable from unprivileged userspace
via DRM_IOCTL_MODE_CREATE_DUMB on the render node.

Add an overflow check in radeon_align_pitch() and reject zero pitch/size
in radeon_mode_dumb_create().

Found via AST-based call-graph analysis using sqry.

Fixes: ff72145badb8 ("drm: dumb scanout create/mmap for intel/radeon (v3)")
Cc: stable@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 drivers/gpu/drm/radeon/radeon_gem.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 20fc87409f2e..2cd179fef347 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -828,6 +828,11 @@ int radeon_align_pitch(struct radeon_device *rdev, int width, int cpp, bool tile
 
 	aligned += pitch_mask;
 	aligned &= ~pitch_mask;
+
+	/* Guard against integer overflow in aligned * cpp. */
+	if (aligned > INT_MAX / (cpp ? cpp : 1) || aligned <= 0)
+		return 0;
+
 	return aligned * cpp;
 }
 
@@ -842,8 +847,12 @@ int radeon_mode_dumb_create(struct drm_file *file_priv,
 
 	args->pitch = radeon_align_pitch(rdev, args->width,
 					 DIV_ROUND_UP(args->bpp, 8), 0);
+	if (!args->pitch)
+		return -EINVAL;
 	args->size = (u64)args->pitch * args->height;
 	args->size = ALIGN(args->size, PAGE_SIZE);
+	if (!args->size)
+		return -EINVAL;
 
 	r = radeon_gem_object_create(rdev, args->size, 0,
 				     RADEON_GEM_DOMAIN_VRAM, 0,
-- 
2.43.0


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

* Claude review: drm/amdgpu,radeon: fix integer overflow in pitch alignment
  2026-04-06 22:50 [PATCH 0/2] drm/amdgpu,radeon: fix integer overflow in pitch alignment Werner Kasselman
  2026-04-06 22:50 ` [PATCH 1/2] drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch() Werner Kasselman
  2026-04-06 22:50 ` [PATCH 2/2] drm/radeon: fix integer overflow in radeon_align_pitch() Werner Kasselman
@ 2026-04-12  4:15 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-12  4:15 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/amdgpu,radeon: fix integer overflow in pitch alignment
Author: Werner Kasselman <werner@verivus.ai>
Patches: 3
Reviewed: 2026-04-12T14:15:40.221211

---

This is a well-motivated 2-patch series fixing a real integer overflow vulnerability in the pitch alignment functions of both the amdgpu and radeon DRM drivers. The bug allows an unprivileged local user to create a zero-size GEM buffer object via `DRM_IOCTL_MODE_CREATE_DUMB`, which could lead to out-of-bounds memory access.

The root cause analysis is correct: `drm_mode_create_dumb()` validates `cpp * width` fits in u32 *before* calling the driver, but both `amdgpu_gem_align_pitch()` and `radeon_align_pitch()` round up `width` by up to 255 pixels, which can push the subsequent `aligned * cpp` multiplication past `INT_MAX`, causing signed integer overflow (undefined behavior in C) that wraps to 0. The patches are minimal, correct, and appropriate for stable backport.

One minor series-level observation: the `Signed-off-by` uses `werner@verivus.com` while the `From:` header shows `werner@verivus.ai` -- this domain mismatch may need clarification for the DCO.

The commit messages correctly note that the long-term fix is converting to `drm_mode_size_dumb()`, making these targeted stopgap fixes appropriate.

---

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch()
  2026-04-06 22:50 ` [PATCH 1/2] drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch() Werner Kasselman
@ 2026-04-12  4:15   ` Claude Code Review Bot
  0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-12  4:15 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.** The overflow analysis is sound. With `width=1073741760, bpp=32` (cpp=4):
- `drm_mode_create_dumb()` passes validation: `4 * 1073741760 = 4294967040 < U32_MAX`
- In `amdgpu_gem_align_pitch()`: `aligned = 1073741760`, `pitch_mask = 63`, after rounding `aligned = 1073741824` (2^30)
- `aligned * cpp = 1073741824 * 4 = 2^32` which overflows signed `int` (max 2^31-1)

**The overflow check:**
```c
if (aligned > INT_MAX / (cpp ? cpp : 1) || aligned <= 0)
    return 0;
```

- The `INT_MAX / cpp` check correctly catches the case above: `INT_MAX/4 = 536870911 < 1073741824`.
- The `aligned <= 0` check covers the case where `args->width` (u32) overflows when assigned to `int width` in the function parameter, wrapping to negative.
- The `cpp ? cpp : 1` division-by-zero guard is technically unreachable since `drm_mode_create_dumb()` rejects `bpp=0` at line 199, but is reasonable defensive coding.

**Using 0 as error return:** Since `width=0` is already rejected by `drm_mode_create_dumb()` (line 199), 0 is safe as a sentinel here. The caller correctly checks for it.

**The `!args->size` check in `amdgpu_mode_dumb_create()`:**
```c
if (!args->size)
    return -EINVAL;
```
This is technically redundant -- if `args->pitch` is non-zero and `args->height` is non-zero (guaranteed by the core), then `(u64)pitch * height` is non-zero, and `ALIGN(non_zero, PAGE_SIZE)` is always >= `PAGE_SIZE`. Harmless belt-and-suspenders though, so not a blocking issue.

**Fixes tag:** Points to `087451f372bf ("drm/amdgpu: use generic fb helpers instead of setting up AMD own's.")`. This doesn't seem like the commit that *introduced* `amdgpu_gem_align_pitch()` itself, but rather a refactoring commit. The Fixes tag would ideally point to whatever commit introduced the vulnerable function. Worth verifying.

**Verdict: Looks correct and appropriate for merge.** Minor nit on the Fixes tag.

---

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/radeon: fix integer overflow in radeon_align_pitch()
  2026-04-06 22:50 ` [PATCH 2/2] drm/radeon: fix integer overflow in radeon_align_pitch() Werner Kasselman
@ 2026-04-12  4:15   ` Claude Code Review Bot
  0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-12  4:15 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Same fix, same analysis.** The radeon variant is structurally identical:

```c
/* Guard against integer overflow in aligned * cpp. */
if (aligned > INT_MAX / (cpp ? cpp : 1) || aligned <= 0)
    return 0;
```

And the caller checks:
```c
if (!args->pitch)
    return -EINVAL;
...
if (!args->size)
    return -EINVAL;
```

The only functional difference is that `radeon_align_pitch()` has slightly different alignment masks (e.g., `pitch_mask = align_large ? 255 : 127` for cpp=1) which doesn't change the overflow risk.

**Fixes tag:** Points to `ff72145badb8 ("drm: dumb scanout create/mmap for intel/radeon (v3)")` which originally introduced `radeon_mode_dumb_create()`. This is correct -- the vulnerability has existed since the function was first written.

**Same redundancy note** about `!args->size` applies here as well.

**Verdict: Looks correct. Same quality as patch 1.**

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-04-12  4:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-06 22:50 [PATCH 0/2] drm/amdgpu,radeon: fix integer overflow in pitch alignment Werner Kasselman
2026-04-06 22:50 ` [PATCH 1/2] drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch() Werner Kasselman
2026-04-12  4:15   ` Claude review: " Claude Code Review Bot
2026-04-06 22:50 ` [PATCH 2/2] drm/radeon: fix integer overflow in radeon_align_pitch() Werner Kasselman
2026-04-12  4:15   ` Claude review: " Claude Code Review Bot
2026-04-12  4:15 ` Claude review: drm/amdgpu,radeon: fix integer overflow in pitch alignment 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