From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch()
Date: Sun, 12 Apr 2026 14:15:40 +1000
Message-ID: <review-patch1-20260406225008.2787532-2-werner@verivus.com>
In-Reply-To: <20260406225008.2787532-2-werner@verivus.com>
References: <20260406225008.2787532-1-werner@verivus.com> <20260406225008.2787532-2-werner@verivus.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

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