public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] accel/ethosu: fix IFM region index out-of-bounds in command stream parser
@ 2026-05-23 19:51 Muhammad Bilal
  2026-05-25  7:21 ` Claude review: " Claude Code Review Bot
  2026-05-25  7:21 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Muhammad Bilal @ 2026-05-23 19:51 UTC (permalink / raw)
  To: robh
  Cc: tomeu, ogabbay, tzimmermann, Frank.Li, dri-devel, linux-kernel,
	stable, Muhammad Bilal

NPU_SET_IFM_REGION extracts the region index with param & 0x7f, giving
a maximum value of 127. However region_size[] and output_region[] in
struct ethosu_validated_cmdstream_info are both sized to
NPU_BASEP_REGION_MAX (8), giving valid indices [0..7].

Every other region assignment in the same switch uses param & 0x7:
  NPU_SET_OFM_REGION:  st.ofm.region  = param & 0x7;
  NPU_SET_IFM2_REGION: st.ifm2.region = param & 0x7;
  NPU_SET_WEIGHT_REGION: st.weight[0].region = param & 0x7;
  NPU_SET_SCALE_REGION:  st.scale[0].region  = param & 0x7;

The 0x7f mask on IFM is inconsistent and appears to be a typo.

feat_matrix_length() and calc_sizes() use the region index directly
as an array subscript into the kzalloc'd info struct:
  info->region_size[fm->region] = max(...);

A userspace caller supplying NPU_SET_IFM_REGION with param > 7 causes
a write up to 127*8 = 1016 bytes past the start of region_size[],
corrupting adjacent kernel heap data.

Fix by applying the same & 0x7 mask used by all other region
assignments.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/accel/ethosu/ethosu_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index f526f4aedffd..80d4bc21c28f 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -466,7 +466,7 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			st.ifm.broadcast = param;
 			break;
 		case NPU_SET_IFM_REGION:
-			st.ifm.region = param & 0x7f;
+			st.ifm.region = param & 0x7;
 			break;
 		case NPU_SET_IFM_WIDTH0_M1:
 			st.ifm.width0 = param;
-- 
2.53.0


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

* Claude review: accel/ethosu: fix IFM region index out-of-bounds in command stream parser
  2026-05-23 19:51 [PATCH] accel/ethosu: fix IFM region index out-of-bounds in command stream parser Muhammad Bilal
@ 2026-05-25  7:21 ` Claude Code Review Bot
  2026-05-25  7:21 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  7:21 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: accel/ethosu: fix IFM region index out-of-bounds in command stream parser
Author: Muhammad Bilal <meatuni001@gmail.com>
Patches: 1
Reviewed: 2026-05-25T17:21:01.362311

---

This is a single-patch fix for a genuine out-of-bounds write vulnerability in the Arm Ethos-U NPU accelerator driver. The analysis in the commit message is thorough and accurate: `NPU_SET_IFM_REGION` used `param & 0x7f` (mask allowing values 0–127) while the target arrays `region_size[]` and `output_region[]` are sized to `NPU_BASEP_REGION_MAX` (8, valid indices 0–7). Every other region assignment in the same switch uses `param & 0x7`. The fix is a one-character change from `0x7f` to `0x7`, making it consistent with all other region mask operations.

**Verdict: This patch looks correct and should be applied.** It fixes a heap out-of-bounds write reachable from userspace.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: accel/ethosu: fix IFM region index out-of-bounds in command stream parser
  2026-05-23 19:51 [PATCH] accel/ethosu: fix IFM region index out-of-bounds in command stream parser Muhammad Bilal
  2026-05-25  7:21 ` Claude review: " Claude Code Review Bot
@ 2026-05-25  7:21 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  7:21 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**The bug:**  
At `ethosu_gem.c:467` (pre-patch), the IFM region was masked with `0x7f`:
```c
case NPU_SET_IFM_REGION:
    st.ifm.region = param & 0x7f;
```

This permits values up to 127, but `region_size[]` and `output_region[]` in `struct ethosu_validated_cmdstream_info` (`ethosu_gem.h:12-13`) are both `NPU_BASEP_REGION_MAX` (8) elements:
```c
u64 region_size[NPU_BASEP_REGION_MAX];   // 8 entries
bool output_region[NPU_BASEP_REGION_MAX]; // 8 entries
```

The region index is later used as a direct array subscript in `calc_sizes()` at `ethosu_gem.c:229`:
```c
info->region_size[fm->region] = max(info->region_size[fm->region], addr + 1);
```

A userspace-controlled `param > 7` causes a heap buffer overflow.

**The fix is correct:**  
All other region assignments use `& 0x7`:
- `ethosu_gem.c:507`: `st.ofm.region = param & 0x7`
- `ethosu_gem.c:541`: `st.ifm2.region = param & 0x7`
- `ethosu_gem.c:569`: `st.weight[0].region = param & 0x7`
- `ethosu_gem.c:572`: `st.scale[0].region = param & 0x7`
- `ethosu_gem.c:615`: `st.dma.src.region = param & 0x7`
- `ethosu_gem.c:622`: `st.dma.dst.region = param & 0x7`

The mask `0x7` limits the value to 0–7, exactly matching the array size of 8.

**Tags:** The `Fixes:` tag and `Cc: stable` are appropriate for a security-relevant bug fix.

**Commit message quality:** Excellent — clearly explains the inconsistency, the data flow from mask to array subscript, and the impact (heap corruption). This is a well-written single-line fix with a thorough justification.

**No issues found.** Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-05-25  7:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-23 19:51 [PATCH] accel/ethosu: fix IFM region index out-of-bounds in command stream parser Muhammad Bilal
2026-05-25  7:21 ` Claude review: " Claude Code Review Bot
2026-05-25  7:21 ` 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