public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/2] accel/ethosu: fix two command stream parser bugs
@ 2026-05-23 21:07 Muhammad Bilal
  2026-05-23 21:07 ` [PATCH 1/2] accel/ethosu: reject NPU_OP_RESIZE commands from userspace Muhammad Bilal
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Muhammad Bilal @ 2026-05-23 21:07 UTC (permalink / raw)
  To: robh
  Cc: tomeu, ogabbay, tzimmermann, Frank.Li, dri-devel, linux-kernel,
	stable, Muhammad Bilal

While investigating the IFM region index out-of-bounds fix already sent
[1], two additional bugs were found in the same command stream parser
function ethosu_gem_cmdstream_copy_and_validate():

Patch 1: NPU_OP_RESIZE unconditionally triggers WARN_ON(1), allowing
any unprivileged user with DRM device access to spam the kernel log or
panic the kernel if panic_on_warn is set.

Patch 2: NPU_SET_SCALE1_LENGTH on U85 hardware assigns the user-supplied
length to weight[1] instead of weight[2], mismatching its BASE handler
and corrupting the software bounds-check state for both weight buffers.

Both fixes apply cleanly on top of the IFM patch and target the same
Fixes: tag since all three bugs originate in the same commit.

[1] <20260523195159.55801-1-meatuni001@gmail.com>

Muhammad Bilal (2):
  accel/ethosu: reject NPU_OP_RESIZE commands from userspace
  accel/ethosu: fix wrong weight index in NPU_SET_SCALE1_LENGTH on U85

 drivers/accel/ethosu/ethosu_gem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

-- 
2.53.0


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

* [PATCH 1/2] accel/ethosu: reject NPU_OP_RESIZE commands from userspace
  2026-05-23 21:07 [PATCH 0/2] accel/ethosu: fix two command stream parser bugs Muhammad Bilal
@ 2026-05-23 21:07 ` Muhammad Bilal
  2026-05-25  7:19   ` Claude review: " Claude Code Review Bot
  2026-05-23 21:07 ` [PATCH 2/2] accel/ethosu: fix wrong weight index in NPU_SET_SCALE1_LENGTH on U85 Muhammad Bilal
  2026-05-25  7:19 ` Claude review: accel/ethosu: fix two command stream parser bugs Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Muhammad Bilal @ 2026-05-23 21:07 UTC (permalink / raw)
  To: robh
  Cc: tomeu, ogabbay, tzimmermann, Frank.Li, dri-devel, linux-kernel,
	stable, Muhammad Bilal

NPU_OP_RESIZE is a U85-only command that the driver does not yet
implement. The existing WARN_ON(1) placeholder fires unconditionally
whenever userspace submits this command via DRM_IOCTL_ETHOSU_GEM_CREATE,
causing unbounded kernel log spam.

If panic_on_warn is set the kernel panics, giving any unprivileged user
with access to the DRM device a trivial denial-of-service primitive.

Replace the WARN_ON(1) with an explicit -EINVAL return so the ioctl
rejects the command before it reaches hardware.

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 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 80d4bc21c28f..043541407a8f 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -433,8 +433,7 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 				return ret;
 			break;
 		case NPU_OP_RESIZE: // U85 only
-			WARN_ON(1); // TODO
-			break;
+			return -EINVAL;
 		case NPU_SET_KERNEL_WIDTH_M1:
 			st.ifm.width = param;
 			break;
-- 
2.53.0


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

* [PATCH 2/2] accel/ethosu: fix wrong weight index in NPU_SET_SCALE1_LENGTH on U85
  2026-05-23 21:07 [PATCH 0/2] accel/ethosu: fix two command stream parser bugs Muhammad Bilal
  2026-05-23 21:07 ` [PATCH 1/2] accel/ethosu: reject NPU_OP_RESIZE commands from userspace Muhammad Bilal
@ 2026-05-23 21:07 ` Muhammad Bilal
  2026-05-25  7:19   ` Claude review: " Claude Code Review Bot
  2026-05-25  7:19 ` Claude review: accel/ethosu: fix two command stream parser bugs Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Muhammad Bilal @ 2026-05-23 21:07 UTC (permalink / raw)
  To: robh
  Cc: tomeu, ogabbay, tzimmermann, Frank.Li, dri-devel, linux-kernel,
	stable, Muhammad Bilal

On non-U65 hardware (e.g. U85), opcode 0x4093 is NPU_SET_WEIGHT2_LENGTH.
The BASE handler for the same opcode correctly assigns to
st.weight[2].base, but the LENGTH handler mistakenly assigns cmds[1]
to st.weight[1].length instead of st.weight[2].length.

This leaves weight[2].length at its initialised sentinel value of
0xffffffff and corrupts weight[1].length with the user-supplied value,
breaking the software bounds-check state for both weight buffers on U85.

Fix the index to match the BASE handler.

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 043541407a8f..5a02285a4986 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -600,7 +600,7 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			if (ethosu_is_u65(edev))
 				st.scale[1].length = cmds[1];
 			else
-				st.weight[1].length = cmds[1];
+				st.weight[2].length = cmds[1];
 			break;
 		case NPU_SET_WEIGHT3_BASE:
 			st.weight[3].base = addr;
-- 
2.53.0


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

* Claude review: accel/ethosu: fix two command stream parser bugs
  2026-05-23 21:07 [PATCH 0/2] accel/ethosu: fix two command stream parser bugs Muhammad Bilal
  2026-05-23 21:07 ` [PATCH 1/2] accel/ethosu: reject NPU_OP_RESIZE commands from userspace Muhammad Bilal
  2026-05-23 21:07 ` [PATCH 2/2] accel/ethosu: fix wrong weight index in NPU_SET_SCALE1_LENGTH on U85 Muhammad Bilal
@ 2026-05-25  7:19 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  7:19 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: accel/ethosu: fix two command stream parser bugs
Author: Muhammad Bilal <meatuni001@gmail.com>
Patches: 3
Reviewed: 2026-05-25T17:19:39.985753

---

This is a clean, well-targeted two-patch series fixing two real bugs in the Ethos-U NPU command stream parser (`ethosu_gem_cmdstream_copy_and_validate()`). Both patches are minimal, correct, and address genuine issues in the same function. The commit messages are well-written with clear explanations of root cause and impact.

The series is **good to merge** with one minor observation on patch 1.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: accel/ethosu: reject NPU_OP_RESIZE commands from userspace
  2026-05-23 21:07 ` [PATCH 1/2] accel/ethosu: reject NPU_OP_RESIZE commands from userspace Muhammad Bilal
@ 2026-05-25  7:19   ` Claude Code Review Bot
  0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  7:19 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Verdict: Correct fix, one minor style observation.**

The change replaces:
```c
case NPU_OP_RESIZE: // U85 only
    WARN_ON(1); // TODO
    break;
```
with:
```c
case NPU_OP_RESIZE: // U85 only
    return -EINVAL;
```

This is the right approach. The `WARN_ON(1)` was unconditionally user-triggerable from unprivileged userspace via `DRM_IOCTL_ETHOSU_GEM_CREATE`, making it a DoS vector (kernel log spam, or panic if `panic_on_warn` is set). Returning `-EINVAL` properly rejects the command at the ioctl level.

**Minor observation:** The `default:` case at line 650 silently ignores unknown opcodes (`break` with no error). This means `NPU_OP_RESIZE` is now treated *more strictly* than completely unknown/garbage opcodes — it returns an error, while unknown commands are silently accepted and passed through. This is arguably the right behavior for a known-but-unimplemented opcode, but the maintainer may want to consider whether `default:` should also return `-EINVAL` in a follow-up, since silently ignoring unrecognized commands seems fragile for a security-sensitive parser.

**No issues with fall-through:** The `return -EINVAL` exits the function entirely, so the absence of `break` before `case NPU_SET_KERNEL_WIDTH_M1:` is correct — there's no fall-through path.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: accel/ethosu: fix wrong weight index in NPU_SET_SCALE1_LENGTH on U85
  2026-05-23 21:07 ` [PATCH 2/2] accel/ethosu: fix wrong weight index in NPU_SET_SCALE1_LENGTH on U85 Muhammad Bilal
@ 2026-05-25  7:19   ` Claude Code Review Bot
  0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  7:19 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Verdict: Correct, obvious typo/copy-paste fix.**

The change at line 601 (pre-patch):
```c
case NPU_SET_SCALE1_LENGTH: // NPU_SET_WEIGHT2_LENGTH (U85)
    if (ethosu_is_u65(edev))
        st.scale[1].length = cmds[1];
    else
        st.weight[1].length = cmds[1];  // BUG: should be weight[2]
```

Fixed to:
```c
    else
        st.weight[2].length = cmds[1];
```

This is unambiguously correct. The opcode `0x4093` is `NPU_SET_WEIGHT2_LENGTH` on non-U65 hardware (confirmed in `ethosu_device.h:148`). The BASE handler for the same opcode at line 591-596 correctly writes to `st.weight[2].base`, so the LENGTH handler must match with `st.weight[2].length`. The original code wrote to `weight[1]`, which is handled by the separate `NPU_SET_WEIGHT1_LENGTH` case at line 588-589 — so the bug both corrupted `weight[1].length` and left `weight[2].length` at its sentinel value of `0xffffffff`, breaking bounds checks for two weight buffers simultaneously.

The Fixes tag and Cc: stable are appropriate for both patches.

---
Generated by Claude Code Patch Reviewer

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

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

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-23 21:07 [PATCH 0/2] accel/ethosu: fix two command stream parser bugs Muhammad Bilal
2026-05-23 21:07 ` [PATCH 1/2] accel/ethosu: reject NPU_OP_RESIZE commands from userspace Muhammad Bilal
2026-05-25  7:19   ` Claude review: " Claude Code Review Bot
2026-05-23 21:07 ` [PATCH 2/2] accel/ethosu: fix wrong weight index in NPU_SET_SCALE1_LENGTH on U85 Muhammad Bilal
2026-05-25  7:19   ` Claude review: " Claude Code Review Bot
2026-05-25  7:19 ` Claude review: accel/ethosu: fix two command stream parser bugs 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