[PATCH v2 00/19] accel: ethosu: Another batch of fixes

[PATCH v2 00/19] accel: ethosu: Another batch of fixes

[Rob Herring (Arm)]

This is another series of fixes to the ethosu driver. It's mostly probe 
error paths and cmd stream validation fixes. It's a mixture of 
AI reported issues, different solutions to other posted issues and 
fixes[1] and my own fixes. The last 2 patches are additional operation 
support.

The series adds rejecting OFM tranpose in cmd stream, but then 
the next to last patch adds support for it. I did this so the rejection 
can be backported, but maybe supporting OFM transpose is small enough 
that it should just be backported too?

Rob

[1] https://lore.kernel.org/all/20260717061145.1478139-1-zhaoguohan@kylinos.cn

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
Changes in v2:
- Dropped first 5 patches which are already applied
- Patches 11-17 and 19 are new patches with more cmd stream validation
- Add fix to diable clocks on PM setup failure
- Rework the runtime-PM to suspend after initialization and before 
  drm_dev_register(). This simplifies the error clean-up in probe.
- Handle quiescing jobs before scheduler teardown
- Rework the cmd stream register initialization tracking to use a bitmap 
  of registers instead of relying on initial state values.
- Handle U85 DMA 2D/3D modes correctly
- Link to v1: https://patch.msgid.link/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org

---
Rob Herring (Arm) (19):
      accel: ethosu: Suspend after initialization
      accel: ethosu: Fix probe error cleanup
      accel: ethosu: Disable clocks on PM setup failure
      accel: ethosu: Quiesce jobs before scheduler teardown
      accel: ethosu: Move DMA mode to src/dst struct
      accel: ethosu: Track command stream register setup
      accel: ethosu: Factor buffer bounds checks
      accel: ethosu: Validate secondary streams
      accel: ethosu: Reject unsupported commands
      accel: ethosu: Validate all feature map tiles
      accel: ethosu: Account for feature map element size
      accel: ethosu: Validate convolution parameter
      accel: ethosu: Account for kernel dilation in IFM size
      accel: ethosu: Reject reserved command encodings
      accel: ethosu: Validate accumulator input
      accel: ethosu: Restrict dynamic IFM2 weights
      accel: ethosu: Split U65 and U85 DMA length validation
      accel: ethosu: Validate OFM transpose
      accel: ethosu: Validate resize operations

 drivers/accel/ethosu/ethosu_device.h |  23 +
 drivers/accel/ethosu/ethosu_drv.c    |  23 +-
 drivers/accel/ethosu/ethosu_drv.h    |   2 +
 drivers/accel/ethosu/ethosu_gem.c    | 813 +++++++++++++++++++++++++++++++----
 drivers/accel/ethosu/ethosu_job.c    |  28 +-
 5 files changed, 798 insertions(+), 91 deletions(-)
---
base-commit: 2b39d680c9e0fb4d625f2916980977622e84248c
change-id: 20260827-ethosu-fixes-ee58386e9000

Best regards,
--  
Rob Herring (Arm) <robh@kernel.org>

[PATCH v2 01/19] accel: ethosu: Suspend after initialization

[Rob Herring (Arm)]

The initial runtime-PM reference is held only while initializing the
NPU. Release it synchronously from ethosu_init() after the final
hardware access, before registering the DRM device.

This keeps the runtime-PM setup and initial reference handling together
and leaves the autosuspend configuration in place for subsequent jobs.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_drv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index 8108622de258..df76253d01a6 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -338,6 +338,8 @@ static int ethosu_init(struct ethosu_device *ethosudev)
 		 ethosudev->npu_info.sram_size / 1024,
 		 ethosudev->npu_info.pmu_counters);
 
+	pm_runtime_put_sync_suspend(ethosudev->base.dev);
+
 	return 0;
 }
 
@@ -376,10 +378,6 @@ static int ethosu_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = drm_dev_register(&ethosudev->base, 0);
-	if (ret)
-		pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
-
-	pm_runtime_put_autosuspend(ethosudev->base.dev);
 	return ret;
 }
 

-- 
2.53.0

Re: [PATCH v2 01/19] accel: ethosu: Suspend after initialization

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] IRQ handler accesses hardware registers while device may be suspended.
--

commit db83f58315d24bc892c6b008ce7d120f57ecaff1
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Suspend after initialization

The initial runtime-PM reference is held only while initializing the NPU.
Release it synchronously from ethosu_init() after the final hardware
access, before registering the DRM device.

> diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> index 8108622de258f..df76253d01a64 100644
> --- a/drivers/accel/ethosu/ethosu_drv.c
> +++ b/drivers/accel/ethosu/ethosu_drv.c
> @@ -338,6 +338,8 @@ static int ethosu_init(struct ethosu_device *ethosudev)
>  		 ethosudev->npu_info.sram_size / 1024,
>  		 ethosudev->npu_info.pmu_counters);
>  
> +	pm_runtime_put_sync_suspend(ethosudev->base.dev);

[Severity: High]
This isn't a bug introduced by this patch, but does putting the device into
an immediate synchronous suspend here expose a vulnerability in the interrupt
handler?

Looking at ethosu_job_irq_handler() in drivers/accel/ethosu/ethosu_job.c:

static irqreturn_t ethosu_job_irq_handler(int irq, void *data)
{
	struct ethosu_device *dev = data;
	u32 status = readl_relaxed(dev->regs + NPU_REG_STATUS);
	...
}

The hardirq handler accesses hardware registers unconditionally without
verifying the device's PM state.

If a spurious interrupt occurs while the device is in the RPM_SUSPENDED
state (where clocks are disabled via ethosu_device_suspend()), could
reading NPU_REG_STATUS cause a Synchronous External Abort?

The PM subsystem guidelines mandate using pm_runtime_get_if_active() in
IRQ handlers to prevent accessing powered-off hardware registers.

> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=1

Re: [PATCH v2 01/19] accel: ethosu: Suspend after initialization

[Rob Herring]

On Sat, Sep 05, 2026 at 12:57:34AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] IRQ handler accesses hardware registers while device may be suspended.
> --
> 
> commit db83f58315d24bc892c6b008ce7d120f57ecaff1
> Author: Rob Herring (Arm) <robh@kernel.org>
> 
> accel: ethosu: Suspend after initialization
> 
> The initial runtime-PM reference is held only while initializing the NPU.
> Release it synchronously from ethosu_init() after the final hardware
> access, before registering the DRM device.
> 
> > diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> > index 8108622de258f..df76253d01a64 100644
> > --- a/drivers/accel/ethosu/ethosu_drv.c
> > +++ b/drivers/accel/ethosu/ethosu_drv.c
> > @@ -338,6 +338,8 @@ static int ethosu_init(struct ethosu_device *ethosudev)
> >  		 ethosudev->npu_info.sram_size / 1024,
> >  		 ethosudev->npu_info.pmu_counters);
> >  
> > +	pm_runtime_put_sync_suspend(ethosudev->base.dev);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does putting the device into
> an immediate synchronous suspend here expose a vulnerability in the interrupt
> handler?
> 
> Looking at ethosu_job_irq_handler() in drivers/accel/ethosu/ethosu_job.c:
> 
> static irqreturn_t ethosu_job_irq_handler(int irq, void *data)
> {
> 	struct ethosu_device *dev = data;
> 	u32 status = readl_relaxed(dev->regs + NPU_REG_STATUS);
> 	...
> }
> 
> The hardirq handler accesses hardware registers unconditionally without
> verifying the device's PM state.
> 
> If a spurious interrupt occurs while the device is in the RPM_SUSPENDED
> state (where clocks are disabled via ethosu_device_suspend()), could
> reading NPU_REG_STATUS cause a Synchronous External Abort?
> 
> The PM subsystem guidelines mandate using pm_runtime_get_if_active() in
> IRQ handlers to prevent accessing powered-off hardware registers.

Not an issue.

The h/w will only generate an interrupt if a job has been submitted in 
which case we are not suspended.

Rob

[PATCH v2 02/19] accel: ethosu: Fix probe error cleanup

[Rob Herring (Arm)]

Once the job scheduler has been initialized, failures from ethosu_init()
or drm_dev_register() return from probe without tearing it down. The
registration failure also leaves the SRAM-pool allocation in use, because
the platform remove callback is not called after a failed probe.

Unwind the initialized resources on both paths. Also do not call
drm_sched_fini() after a failed drm_sched_init(): the scheduler initializer
already unwinds its partial setup, while drm_sched_fini() requires a
successfully initialized scheduler.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - Adjust for previous patch reworking runtime-PM suspend
---
 drivers/accel/ethosu/ethosu_drv.c | 13 ++++++++++++-
 drivers/accel/ethosu/ethosu_job.c |  6 +-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index df76253d01a6..c684e28ac787 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -375,9 +375,20 @@ static int ethosu_probe(struct platform_device *pdev)
 
 	ret = ethosu_init(ethosudev);
 	if (ret)
-		return ret;
+		goto err_job_fini;
 
 	ret = drm_dev_register(&ethosudev->base, 0);
+	if (ret)
+		goto err_sram_free;
+
+	return 0;
+
+err_sram_free:
+	if (ethosudev->sram)
+		gen_pool_free(ethosudev->srampool, (unsigned long)ethosudev->sram,
+			      ethosudev->npu_info.sram_size);
+err_job_fini:
+	ethosu_job_fini(ethosudev);
 	return ret;
 }
 
diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index 8dce74db0cb4..ec65305e0cd7 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -355,14 +355,10 @@ int ethosu_job_init(struct ethosu_device *edev)
 	ret = drm_sched_init(&edev->sched, &args);
 	if (ret) {
 		dev_err(dev, "Failed to create scheduler: %d\n", ret);
-		goto err_sched;
+		return ret;
 	}
 
 	return 0;
-
-err_sched:
-	drm_sched_fini(&edev->sched);
-	return ret;
 }
 
 void ethosu_job_fini(struct ethosu_device *dev)

-- 
2.53.0

Re: [PATCH v2 02/19] accel: ethosu: Fix probe error cleanup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Active devm-managed IRQ during probe error paths can trigger use-after-free or system crash due to destroyed scheduler.

Pre-existing issues:
- [High] IRQ handler is registered before device clocks are enabled, risking system crash on pending or spurious interrupt.
--

commit 884e296c2a78cb8e0657cc20be2687762177ac12
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Fix probe error cleanup

This patch unwinds initialized resources on both failure paths in probe:
ethosu_init() and drm_dev_register(). It also avoids calling drm_sched_fini()
after a failed drm_sched_init().

> diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> index df76253d01a64..c684e28ac7871 100644
> --- a/drivers/accel/ethosu/ethosu_drv.c
> +++ b/drivers/accel/ethosu/ethosu_drv.c
> @@ -375,9 +375,20 @@ static int ethosu_probe(struct platform_device *pdev)
>  

[Severity: High]
This is a pre-existing issue, but is it safe that the IRQ handler is registered
and unmasked before the device clocks are enabled?

In ethosu_probe(), ethosu_job_init() is called before ethosu_init():

ethosu_drv.c:ethosu_probe() {
	...
	ret = ethosu_job_init(ethosudev);
	if (ret)
		return ret;

	ret = ethosu_init(ethosudev);
	...
}

Inside ethosu_job_init(), devm_request_threaded_irq() unmasks the IRQ. If a
pending interrupt from a prior boot or a spurious interrupt fires, the handler
ethosu_job_irq_handler() executes and immediately reads NPU_REG_STATUS.

However, the hardware clocks are not enabled until ethosu_init() calls
ethosu_device_resume(). Could accessing this register with disabled clocks
trigger a synchronous external abort and crash the system?

>  	ret = ethosu_init(ethosudev);
>  	if (ret)
> -		return ret;
> +		goto err_job_fini;
>  
>  	ret = drm_dev_register(&ethosudev->base, 0);
> +	if (ret)
> +		goto err_sram_free;
> +
> +	return 0;
> +
> +err_sram_free:
> +	if (ethosudev->sram)
> +		gen_pool_free(ethosudev->srampool, (unsigned long)ethosudev->sram,
> +			      ethosudev->npu_info.sram_size);
> +err_job_fini:
> +	ethosu_job_fini(ethosudev);

[Severity: High]
Could the devm-managed IRQ trigger a use-after-free or a crash during this
teardown window on probe failure?

This patch adds ethosu_job_fini() to the probe error path. When
ethosu_job_fini() executes, it tears down the DRM scheduler via
drm_sched_fini().

Because the IRQ was allocated via devm_request_threaded_irq() in
ethosu_job_init() and is not explicitly disabled before ethosu_job_fini(), the
interrupt remains active until the probe function returns and the devres
cleanup runs.

If an interrupt fires in this window, the handler could access the destroyed
scheduler when calling drm_sched_fault(&dev->sched). Additionally, since
ethosu_init() already suspended the device on its successful path, could the
handler crash the system by reading the status register with clocks disabled?

>  	return ret;
>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=2

[PATCH v2 03/19] accel: ethosu: Disable clocks on PM setup failure

[Rob Herring (Arm)]

ethosu_init() enables the clocks directly to reset and query the NPU
before runtime PM takes over. If runtime-PM setup fails, the error path
returns while those clocks remain enabled.

Disable the clocks before returning the setup error to balance the
initial direct resume.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index c684e28ac787..65b148e770e5 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -311,8 +311,10 @@ static int ethosu_init(struct ethosu_device *ethosudev)
 	pm_runtime_set_autosuspend_delay(ethosudev->base.dev, 50);
 	pm_runtime_use_autosuspend(ethosudev->base.dev);
 	ret = devm_pm_runtime_set_active_enabled(ethosudev->base.dev);
-	if (ret)
+	if (ret) {
+		ethosu_device_suspend(ethosudev->base.dev);
 		return ret;
+	}
 	pm_runtime_get_noresume(ethosudev->base.dev);
 
 	ethosudev->npu_info.id = id = readl_relaxed(ethosudev->regs + NPU_REG_ID);

-- 
2.53.0

Re: [PATCH v2 03/19] accel: ethosu: Disable clocks on PM setup failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver leaks hardware clocks on removal because it fails to synchronously suspend the device or flush pending autosuspends before PM runtime is disabled.
- [High] Destroying the DRM scheduler in `ethosu_remove()` while userspace holds open file descriptors leads to a Use-After-Free in `ethosu_postclose()`.
- [Medium] The return value of `ethosu_sram_init()` is ignored, which can cause the driver to silently mask memory allocation failures.
--

commit 527f62d6ec40e5aa68386e337081401539f1068b
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Disable clocks on PM setup failure

ethosu_init() enables the clocks directly to reset and query the NPU
before runtime PM takes over. If runtime-PM setup fails, the error path
returns while those clocks remain enabled. Disable the clocks before
returning the setup error to balance the initial direct resume.

> diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> index c684e28ac7871..65b148e770e57 100644
> --- a/drivers/accel/ethosu/ethosu_drv.c
> +++ b/drivers/accel/ethosu/ethosu_drv.c
> @@ -311,8 +311,10 @@ static int ethosu_init(struct ethosu_device *ethosudev)
>  	pm_runtime_set_autosuspend_delay(ethosudev->base.dev, 50);
>  	pm_runtime_use_autosuspend(ethosudev->base.dev);
>  	ret = devm_pm_runtime_set_active_enabled(ethosudev->base.dev);
> -	if (ret)
> +	if (ret) {
> +		ethosu_device_suspend(ethosudev->base.dev);
>  		return ret;
> +	}
>  	pm_runtime_get_noresume(ethosudev->base.dev);
>  
>  	ethosudev->npu_info.id = id = readl_relaxed(ethosudev->regs + NPU_REG_ID);

[Severity: Medium]
This is a pre-existing issue, but should the return value of
ethosu_sram_init() be checked in ethosu_init()?

drivers/accel/ethosu/ethosu_drv.c:ethosu_init() {
...
	ethosu_sram_init(ethosudev);

	if (!ethosu_is_u65(ethosudev))
		ethosudev->pmu_regs += 0x1000;
...
}

If gen_pool_dma_alloc() fails in ethosu_sram_init(), it returns -ENOMEM.
However, ethosu_init() ignores this return value and continues initialization
with a 0-sized SRAM pool.

If SRAM is configured in the device tree but allocation fails, the error is
suppressed.

[Severity: High]
This is a pre-existing issue, but does this driver leak hardware clocks on
removal?

drivers/accel/ethosu/ethosu_drv.c:ethosu_remove() {
	struct ethosu_device *ethosudev = dev_get_drvdata(&pdev->dev);

	drm_dev_unregister(&ethosudev->base);
	ethosu_job_fini(ethosudev);
	if (ethosudev->sram)
		gen_pool_free(ethosudev->srampool, (unsigned long)ethosudev->sram,
			      ethosudev->npu_info.sram_size);
}

It appears the remove function completes without synchronously suspending the
device or flushing pending autosuspends before PM runtime is disabled.

If a job completes and releases its PM reference via
pm_runtime_put_autosuspend() (delay 50ms), and the device is unbound via
ethosu_remove() before the autosuspend timer expires, the devm teardown
sequence will call pm_runtime_disable().

This cancels pending autosuspend timers without suspending the hardware,
leaving the clocks permanently enabled.

As stated in the subsystem guidelines, synchronous suspend
(pm_runtime_force_suspend() or pm_runtime_put_sync()) is required before
pm_runtime_disable().

[Severity: High]
This is a pre-existing issue, but could destroying the DRM scheduler in
ethosu_remove() while userspace holds open file descriptors lead to a
use-after-free?

If userspace has an open file descriptor for the device and a privileged user
unbinds the driver via sysfs, ethosu_remove() calls ethosu_job_fini(), which
executes drm_sched_fini(&dev->sched).

This frees the scheduler's runqueues (sched->sched_rq[i]).

Later, when userspace closes the file descriptor, the DRM core invokes:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_close() {
	struct drm_sched_entity *entity = &ethosu_priv->sched_entity;

	drm_sched_entity_destroy(entity);
}

drm_sched_entity_destroy() attempts to flush the entity by accessing the freed
scheduler runqueues. This accesses entity->rq->sched, where entity->rq is a
dangling pointer to the freed runqueue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=3

[PATCH v2 04/19] accel: ethosu: Quiesce jobs before scheduler teardown

[Rob Herring (Arm)]

An NPU job can complete while driver removal tears down the scheduler. Its
IRQ handler could then access scheduler state after it has been
destroyed.

Stop scheduler submission and timeout work, reset the NPU, and
synchronize its IRQ before finalizing the scheduler. Add a cancel_job
callback so drm_sched_fini() signals queued jobs with -ECANCELED; their
runtime-PM references are then released during normal job cleanup.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_drv.c |  4 ++--
 drivers/accel/ethosu/ethosu_drv.h |  2 ++
 drivers/accel/ethosu/ethosu_job.c | 22 +++++++++++++++++++++-
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index 65b148e770e5..0918fd9b7041 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -216,7 +216,7 @@ static const struct drm_driver ethosu_drm_driver = {
 #define U85_MEM_ATTR0_CFG	0x00000000
 #define U85_MEM_ATTR2_CFG	0x000000b7
 
-static int ethosu_reset(struct ethosu_device *ethosudev)
+int ethosu_device_reset(struct ethosu_device *ethosudev)
 {
 	int ret;
 	u32 reg;
@@ -263,7 +263,7 @@ static int ethosu_device_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	ret = ethosu_reset(ethosudev);
+	ret = ethosu_device_reset(ethosudev);
 	if (!ret)
 		return 0;
 
diff --git a/drivers/accel/ethosu/ethosu_drv.h b/drivers/accel/ethosu/ethosu_drv.h
index 2193bc51d425..f59c845c758b 100644
--- a/drivers/accel/ethosu/ethosu_drv.h
+++ b/drivers/accel/ethosu/ethosu_drv.h
@@ -11,6 +11,8 @@ struct ethosu_device;
 struct drm_device;
 struct drm_file;
 
+int ethosu_device_reset(struct ethosu_device *ethosudev);
+
 struct ethosu_file_priv {
 	struct ethosu_device *edev;
 	struct drm_sched_entity sched_entity;
diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index ec65305e0cd7..74a1136635f8 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -189,6 +189,16 @@ static void ethosu_job_free(struct drm_sched_job *sched_job)
 	ethosu_job_put(job);
 }
 
+static void ethosu_job_cancel(struct drm_sched_job *sched_job)
+{
+	struct ethosu_job *job = to_ethosu_job(sched_job);
+
+	if (!dma_fence_is_signaled(job->done_fence)) {
+		dma_fence_set_error(job->done_fence, -ECANCELED);
+		dma_fence_signal(job->done_fence);
+	}
+}
+
 static void
 ethosu_switch_perfmon(struct ethosu_device *ethosu, struct ethosu_job *job)
 {
@@ -315,7 +325,8 @@ static enum drm_gpu_sched_stat ethosu_job_timedout(struct drm_sched_job *bad)
 static const struct drm_sched_backend_ops ethosu_sched_ops = {
 	.run_job = ethosu_job_run,
 	.timedout_job = ethosu_job_timedout,
-	.free_job = ethosu_job_free
+	.free_job = ethosu_job_free,
+	.cancel_job = ethosu_job_cancel,
 };
 
 int ethosu_job_init(struct ethosu_device *edev)
@@ -363,6 +374,15 @@ int ethosu_job_init(struct ethosu_device *edev)
 
 void ethosu_job_fini(struct ethosu_device *dev)
 {
+	drm_sched_wqueue_stop(&dev->sched);
+	cancel_delayed_work_sync(&dev->sched.work_tdr);
+
+	if (READ_ONCE(dev->in_flight_job)) {
+		WRITE_ONCE(dev->in_flight_job, NULL);
+		ethosu_device_reset(dev);
+	}
+
+	synchronize_irq(dev->irq);
 	drm_sched_fini(&dev->sched);
 }
 

-- 
2.53.0

Re: [PATCH v2 04/19] accel: ethosu: Quiesce jobs before scheduler teardown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect teardown ordering and missing disable_irq() allow the IRQ handler to re-arm the TDR timeout work after it was cancelled, leading to a Use-After-Free of scheduler runqueues.
- [High] drm_sched_entity_destroy() accesses freed scheduler runqueues when the device is unbound while DRM files remain open.
--

commit 02f285145ac0684a0a3a6e4b7752353026bebfae
Author: Rob Herring (Arm) <robh@kernel.org>
accel: ethosu: Quiesce jobs before scheduler teardown

This commit aims to fix a teardown race by stopping scheduler submission,
cancelling the timeout work, resetting the NPU, and synchronizing its IRQ
before destroying the scheduler. It also adds a cancel_job callback so
queued jobs are properly signalled with -ECANCELED.

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index ec65305e0cd7d..74a1136635f87 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c

[ ... ]

> @@ -363,6 +374,15 @@ int ethosu_job_init(struct ethosu_device *edev)
>  
>  void ethosu_job_fini(struct ethosu_device *dev)
>  {
> +	drm_sched_wqueue_stop(&dev->sched);
> +	cancel_delayed_work_sync(&dev->sched.work_tdr);
> +
> +	if (READ_ONCE(dev->in_flight_job)) {
> +		WRITE_ONCE(dev->in_flight_job, NULL);
> +		ethosu_device_reset(dev);
> +	}
> +
> +	synchronize_irq(dev->irq);

[Severity: High]
This is a pre-existing issue, but does using synchronize_irq() instead of
disable_irq() leave a race window open here?

If a hardware error IRQ fires concurrently, the IRQ thread could read the
error status before the device is reset but get preempted. Because the IRQ
is only synchronized and not disabled, the thread could resume, see the
error, and call drm_sched_fault(&dev->sched). This would re-arm
dev->sched.work_tdr after it was just cancelled.

When drm_sched_fini() is called immediately after this, it frees the
runqueues and then calls its own cancel_delayed_work_sync(&sched->work_tdr).
A re-armed work_tdr could execute concurrently in this window and access the
freed runqueues.

Should this use disable_irq() instead of synchronize_irq() to ensure the
timeout work cannot be re-armed during teardown?

[Severity: High]
This isn't a bug introduced by this patch, but does calling
ethosu_job_fini() directly from ethosu_remove() cause a use-after-free if
userspace still has the DRM file descriptor open?

If the driver is unbound (e.g., via sysfs rmmod/unbind) while userspace
holds an open file descriptor, ethosu_remove() will call ethosu_job_fini()
which in turn calls drm_sched_fini(), freeing the scheduler runqueues.

When the userspace process eventually closes the file descriptor,
ethosu_job_close() is called:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_close() {
    struct drm_sched_entity *entity = &ethosu_priv->sched_entity;

    drm_sched_entity_destroy(entity);
}

The drm_sched_entity_destroy() call will attempt to access the already
freed sched_rq spinlocks. Is there a way to defer the scheduler teardown to
the DRM device release callback instead of tearing it down in the remove
path?

>  	drm_sched_fini(&dev->sched);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=4

[PATCH v2 05/19] accel: ethosu: Move DMA mode to src/dst struct

[Rob Herring (Arm)]

The DMA mode setting is independent for source and destination, so it
should be part of the src/dst struct dma rather than the global DMA
state.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/accel/ethosu/ethosu_gem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 9afe2549ec84..408b93350dd7 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -100,6 +100,7 @@ int ethosu_gem_create_with_handle(struct drm_file *file,
 
 struct dma {
 	s8 region;
+	s8 mode;
 	u64 len;
 	u64 offset;
 	s64 stride[2];
@@ -108,7 +109,6 @@ struct dma {
 struct dma_state {
 	u16 size0;
 	u16 size1;
-	s8 mode;
 	struct dma src;
 	struct dma dst;
 };
@@ -161,7 +161,7 @@ static u64 cmd_to_addr(u32 *cmd)
 static u64 dma_length(struct ethosu_validated_cmdstream_info *info,
 		      struct dma_state *dma_st, struct dma *dma)
 {
-	s8 mode = dma_st->mode;
+	s8 mode = dma->mode;
 	u64 len = dma->len;
 
 	if (len == U64_MAX)
@@ -654,13 +654,14 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 				st.dma.src.region = -1;
 			else
 				st.dma.src.region = param & 0x7;
-			st.dma.mode = (param >> 9) & 0x3;
+			st.dma.src.mode = (param >> 9) & 0x3;
 			break;
 		case NPU_SET_DMA0_DST_REGION:
 			if (param & 0x100)
 				st.dma.dst.region = -1;
 			else
 				st.dma.dst.region = param & 0x7;
+			st.dma.dst.mode = (param >> 9) & 0x3;
 			break;
 		case NPU_SET_DMA0_SIZE0:
 			st.dma.size0 = param;

-- 
2.53.0

[PATCH v2 06/19] accel: ethosu: Track command stream register setup

[Rob Herring (Arm)]

The current method of tracking command stream state initializing state
tracking to illegal values and assuming unaccessed registers are 0 is
proving inadequate with additional validation. Instead, track all the
registers in a bitmap as the register address space is fairly small.

CMD1 opcodes overlap CMD0 after bit 14 is stripped, so maintain a
separate bitmap for each bank.

Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_gem.c | 269 +++++++++++++++++++++++++++++++++-----
 1 file changed, 234 insertions(+), 35 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 408b93350dd7..8a44a5d89bce 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only or MIT
 /* Copyright 2025 Arm, Ltd. */
 
+#include <linux/bitmap.h>
 #include <linux/err.h>
 #include <linux/overflow.h>
 #include <linux/slab.h>
@@ -138,7 +139,12 @@ struct feat_matrix {
 	u8 pad_right;
 };
 
+#define NPU_CMD0_REGS	0x200
+#define NPU_CMD1_REGS	0x100
+
 struct cmd_state {
+	DECLARE_BITMAP(cmd0, NPU_CMD0_REGS);
+	DECLARE_BITMAP(cmd1, NPU_CMD1_REGS);
 	struct dma_state dma;
 	struct buffer scale[2];
 	struct buffer weight[4];
@@ -149,8 +155,29 @@ struct cmd_state {
 
 static void cmd_state_init(struct cmd_state *st)
 {
-	/* Initialize to all 1s to detect missing setup */
-	memset(st, 0xff, sizeof(*st));
+	memset(st, 0, sizeof(*st));
+}
+
+static void cmd_state_set_reg(struct cmd_state *st, u16 cmd)
+{
+	u16 reg = cmd & ~BIT(14);
+
+	if (cmd & BIT(14)) {
+		if (reg < NPU_CMD1_REGS)
+			__set_bit(reg, st->cmd1);
+	} else if (reg < NPU_CMD0_REGS) {
+		__set_bit(reg, st->cmd0);
+	}
+}
+
+static bool cmd_state_reg_is_set(struct cmd_state *st, u16 cmd)
+{
+	u16 reg = cmd & ~BIT(14);
+
+	if (cmd & BIT(14))
+		return reg < NPU_CMD1_REGS && test_bit(reg, st->cmd1);
+
+	return reg < NPU_CMD0_REGS && test_bit(reg, st->cmd0);
 }
 
 static u64 cmd_to_addr(u32 *cmd)
@@ -158,13 +185,54 @@ static u64 cmd_to_addr(u32 *cmd)
 	return (((u64)cmd[0] & 0xff0000) << 16) | cmd[1];
 }
 
-static u64 dma_length(struct ethosu_validated_cmdstream_info *info,
-		      struct dma_state *dma_st, struct dma *dma)
+static bool dma_use_src_stride(struct ethosu_device *edev,
+			       const struct dma_state *dma_st, const struct dma *dma)
+{
+	return ethosu_is_u65(edev) || dma == &dma_st->src;
+}
+
+static bool dma_params_valid(struct ethosu_device *edev, struct cmd_state *st,
+			     const struct dma_state *dma_st,
+			     const struct dma *dma,
+			     u16 region_cmd, u16 addr_cmd)
+{
+	s8 mode = dma->mode;
+
+	if (!cmd_state_reg_is_set(st, region_cmd) ||
+	    !cmd_state_reg_is_set(st, addr_cmd) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_DMA0_LEN) || mode < 0 || mode > 2)
+		return false;
+
+	if (mode >= 1 &&
+	    !cmd_state_reg_is_set(st, dma_use_src_stride(edev, dma_st, dma) ?
+				  NPU_SET_DMA0_SRC_STRIDE0 :
+				  NPU_SET_DMA0_DST_STRIDE0))
+		return U64_MAX;
+	if (mode == 2 &&
+	    !cmd_state_reg_is_set(st, dma_use_src_stride(edev, dma_st, dma) ?
+				  NPU_SET_DMA0_SRC_STRIDE1 :
+				  NPU_SET_DMA0_DST_STRIDE1))
+		return U64_MAX;
+
+	if (mode >= 1 &&
+	    (!cmd_state_reg_is_set(st, NPU_SET_DMA0_SIZE0) || !dma_st->size0))
+		return false;
+	if (mode == 2 &&
+	    (!cmd_state_reg_is_set(st, NPU_SET_DMA0_SIZE1) || !dma_st->size1))
+		return false;
+
+	return true;
+}
+
+static u64 dma_length(struct ethosu_device *edev,
+		      struct ethosu_validated_cmdstream_info *info,
+		      struct cmd_state *st, struct dma_state *dma_st,
+		      struct dma *dma, u16 region_cmd, u16 addr_cmd)
 {
 	s8 mode = dma->mode;
 	u64 len = dma->len;
 
-	if (len == U64_MAX)
+	if (!dma_params_valid(edev, st, dma_st, dma, region_cmd, addr_cmd))
 		return U64_MAX;
 
 	if (mode >= 1) {
@@ -199,17 +267,98 @@ static bool feat_matrix_chained(struct ethosu_device *edev, struct feat_matrix *
 	return !ethosu_is_u65(edev) && storage == 2;
 }
 
+enum feat_matrix_type {
+	FEAT_MATRIX_IFM,
+	FEAT_MATRIX_OFM,
+	FEAT_MATRIX_IFM2,
+};
+
+static u16 feat_matrix_base_cmd(enum feat_matrix_type type)
+{
+	switch (type) {
+	case FEAT_MATRIX_IFM:
+		return NPU_SET_IFM_BASE0;
+	case FEAT_MATRIX_OFM:
+		return NPU_SET_OFM_BASE0;
+	case FEAT_MATRIX_IFM2:
+		return NPU_SET_IFM2_BASE0;
+	}
+
+	return 0;
+}
+
+static int feat_matrix_validate(struct ethosu_device *edev,
+				struct cmd_state *st, struct feat_matrix *fm,
+				enum feat_matrix_type type)
+{
+	u32 format;
+	u16 stride_cmd;
+
+	switch (type) {
+	case FEAT_MATRIX_IFM:
+		if (!cmd_state_reg_is_set(st, NPU_SET_IFM_REGION) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_PRECISION) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_DEPTH_M1))
+			return -EINVAL;
+		if (feat_matrix_chained(edev, fm))
+			return 0;
+		if (!cmd_state_reg_is_set(st, NPU_SET_IFM_WIDTH0_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_HEIGHT0_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_HEIGHT1_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_STRIDE_Y))
+			return -EINVAL;
+		break;
+	case FEAT_MATRIX_OFM:
+		if (!cmd_state_reg_is_set(st, NPU_SET_OFM_REGION) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_OFM_PRECISION) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_OFM_DEPTH_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_OFM_WIDTH_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_OFM_HEIGHT_M1))
+			return -EINVAL;
+		if (feat_matrix_chained(edev, fm))
+			return 0;
+		if (!cmd_state_reg_is_set(st, NPU_SET_OFM_WIDTH0_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_OFM_HEIGHT0_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_OFM_HEIGHT1_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_OFM_STRIDE_Y))
+			return -EINVAL;
+		break;
+	case FEAT_MATRIX_IFM2:
+		if (!cmd_state_reg_is_set(st, NPU_SET_IFM2_REGION) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM2_PRECISION))
+			return -EINVAL;
+		if (feat_matrix_chained(edev, fm))
+			return 0;
+		if (!cmd_state_reg_is_set(st, NPU_SET_IFM2_WIDTH0_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM2_HEIGHT0_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM2_HEIGHT1_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM2_STRIDE_Y))
+			return -EINVAL;
+		break;
+	}
+
+	format = (fm->precision >> 6) & 0x3;
+	stride_cmd = feat_matrix_base_cmd(type) + (format ? 6 : 4);
+	if (!cmd_state_reg_is_set(st, stride_cmd))
+		return -EINVAL;
+
+	return 0;
+}
 static u64 feat_matrix_length(struct ethosu_device *edev,
 			      struct ethosu_validated_cmdstream_info *info,
-			      struct feat_matrix *fm,
+			      struct cmd_state *st, struct feat_matrix *fm,
+			      enum feat_matrix_type type,
 			      u32 x, u32 y, u32 c, bool ofm)
 {
 	u32 element_size, storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14;
 	int tile = 0;
 	u64 addr;
+	u64 offset;
 
 	if (fm->region < 0)
 		return U64_MAX;
+	if (feat_matrix_validate(edev, st, fm, type))
+		return U64_MAX;
 
 	if (feat_matrix_chained(edev, fm))
 		return 0;
@@ -237,24 +386,39 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
 	default:
 		return U64_MAX;
 	}
-	if (fm->base[tile] == U64_MAX)
+	if (!cmd_state_reg_is_set(st, feat_matrix_base_cmd(type) + tile))
 		return U64_MAX;
 
-	addr = fm->base[tile] + y * fm->stride_y;
+	if (check_mul_overflow(y, (u64)fm->stride_y, &offset) ||
+	    check_add_overflow(fm->base[tile], offset, &addr))
+		return U64_MAX;
 
 	switch ((fm->precision >> 6) & 0x3) { // format
 	case 0: //nhwc:
 		element_size = BIT((fm->precision >> (ofm ? 1 : 2)) & 0x3);
-		addr += x * fm->stride_x + c * element_size;
+		if (check_mul_overflow(x, (u64)fm->stride_x, &offset) ||
+		    check_add_overflow(addr, offset, &addr) ||
+		    check_mul_overflow(c, element_size, &offset) ||
+		    check_add_overflow(addr, offset, &addr))
+			return U64_MAX;
 		break;
 	case 1: //nhcwb16:
 		element_size = BIT((fm->precision >> (ofm ? 1 : 2)) & 0x3);
 
-		addr += (c / 16) * fm->stride_c + (16 * x + (c & 0xf)) * element_size;
+		if (check_mul_overflow(c / 16, (u64)fm->stride_c, &offset) ||
+		    check_add_overflow(addr, offset, &addr) ||
+		    check_mul_overflow(16 * x + (c & 0xf), element_size, &offset) ||
+		    check_add_overflow(addr, offset, &addr))
+			return U64_MAX;
 		break;
+	default:
+		return U64_MAX;
 	}
 
-	info->region_size[fm->region] = max(info->region_size[fm->region], addr + 1);
+	if (check_add_overflow(addr, 1ULL, &offset))
+		return U64_MAX;
+
+	info->region_size[fm->region] = max(info->region_size[fm->region], offset);
 
 	return addr;
 }
@@ -268,7 +432,13 @@ static int calc_sizes(struct drm_device *ddev,
 	u64 len;
 
 	if (ifm) {
-		if (st->ifm.stride_kernel == U16_MAX)
+		if (!cmd_state_reg_is_set(st, NPU_SET_KERNEL_WIDTH_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_KERNEL_HEIGHT_M1) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_KERNEL_STRIDE) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_PAD_TOP) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_PAD_LEFT) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_PAD_RIGHT) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_IFM_PAD_BOTTOM))
 			return -EINVAL;
 		u32 stride_y = ((st->ifm.stride_kernel >> 8) & 0x2) +
 			((st->ifm.stride_kernel >> 1) & 0x1) + 1;
@@ -282,8 +452,9 @@ static int calc_sizes(struct drm_device *ddev,
 		if (ifm_height < 0 || ifm_width < 0)
 			return -EINVAL;
 
-		len = feat_matrix_length(edev, info, &st->ifm, ifm_width,
-					 ifm_height, st->ifm.depth, false);
+		len = feat_matrix_length(edev, info, st, &st->ifm,
+					 FEAT_MATRIX_IFM, ifm_width, ifm_height,
+					 st->ifm.depth, false);
 		dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n",
 			op, st->ifm.region, st->ifm.base[0], len);
 		if (len == U64_MAX)
@@ -291,8 +462,9 @@ static int calc_sizes(struct drm_device *ddev,
 	}
 
 	if (ifm2) {
-		len = feat_matrix_length(edev, info, &st->ifm2, st->ifm.depth,
-					 0, st->ofm.depth, false);
+		len = feat_matrix_length(edev, info, st, &st->ifm2,
+					 FEAT_MATRIX_IFM2, st->ifm.depth, 0,
+					 st->ofm.depth, false);
 		dev_dbg(ddev->dev, "op %d: IFM2:%d:0x%llx-0x%llx\n",
 			op, st->ifm2.region, st->ifm2.base[0], len);
 		if (len == U64_MAX)
@@ -303,8 +475,9 @@ static int calc_sizes(struct drm_device *ddev,
 		dev_dbg(ddev->dev, "op %d: W:%d:0x%llx-0x%llx\n",
 			op, st->weight[0].region, st->weight[0].base,
 			st->weight[0].base + st->weight[0].length - 1);
-		if (st->weight[0].region < 0 || st->weight[0].base == U64_MAX ||
-		    st->weight[0].length == U32_MAX)
+		if (!cmd_state_reg_is_set(st, NPU_SET_WEIGHT_REGION) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_WEIGHT_BASE) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_WEIGHT_LENGTH))
 			return -EINVAL;
 		info->region_size[st->weight[0].region] =
 			max(info->region_size[st->weight[0].region],
@@ -315,16 +488,18 @@ static int calc_sizes(struct drm_device *ddev,
 		dev_dbg(ddev->dev, "op %d: S:%d:0x%llx-0x%llx\n",
 			op, st->scale[0].region, st->scale[0].base,
 			st->scale[0].base + st->scale[0].length - 1);
-		if (st->scale[0].region < 0 || st->scale[0].base == U64_MAX ||
-		    st->scale[0].length == U32_MAX)
+		if (!cmd_state_reg_is_set(st, NPU_SET_SCALE_REGION) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_SCALE_BASE) ||
+		    !cmd_state_reg_is_set(st, NPU_SET_SCALE_LENGTH))
 			return -EINVAL;
 		info->region_size[st->scale[0].region] =
 			max(info->region_size[st->scale[0].region],
 			    st->scale[0].base + st->scale[0].length);
 	}
 
-	len = feat_matrix_length(edev, info, &st->ofm, st->ofm.width,
-				 st->ofm.height[2], st->ofm.depth, true);
+	len = feat_matrix_length(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,
+				 st->ofm.width, st->ofm.height[2], st->ofm.depth,
+				 true);
 	dev_dbg(ddev->dev, "op %d: OFM:%d:0x%llx-0x%llx\n",
 		op, st->ofm.region, st->ofm.base[0], len);
 	if (len == U64_MAX)
@@ -349,8 +524,8 @@ static int calc_sizes_elemwise(struct drm_device *ddev,
 		width = st->ifm.broadcast & 0x2 ? 0 : st->ofm.width;
 		depth = st->ifm.broadcast & 0x4 ? 0 : st->ofm.depth;
 
-		len = feat_matrix_length(edev, info, &st->ifm, width,
-					 height, depth, false);
+		len = feat_matrix_length(edev, info, st, &st->ifm,
+					 FEAT_MATRIX_IFM, width, height, depth, false);
 		dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n",
 			op, st->ifm.region, st->ifm.base[0], len);
 		if (len == U64_MAX)
@@ -362,16 +537,17 @@ static int calc_sizes_elemwise(struct drm_device *ddev,
 		width = st->ifm2.broadcast & 0x2 ? 0 : st->ofm.width;
 		depth = st->ifm2.broadcast & 0x4 ? 0 : st->ofm.depth;
 
-		len = feat_matrix_length(edev, info, &st->ifm2, width,
-					 height, depth, false);
+		len = feat_matrix_length(edev, info, st, &st->ifm2,
+					 FEAT_MATRIX_IFM2, width, height, depth, false);
 		dev_dbg(ddev->dev, "op %d: IFM2:%d:0x%llx-0x%llx\n",
 			op, st->ifm2.region, st->ifm2.base[0], len);
 		if (len == U64_MAX)
 			return -EINVAL;
 	}
 
-	len = feat_matrix_length(edev, info, &st->ofm, st->ofm.width,
-				 st->ofm.height[2], st->ofm.depth, true);
+	len = feat_matrix_length(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,
+				 st->ofm.width, st->ofm.height[2], st->ofm.depth,
+				 true);
 	dev_dbg(ddev->dev, "op %d: OFM:%d:0x%llx-0x%llx\n",
 		op, st->ofm.region, st->ofm.base[0], len);
 	if (len == U64_MAX)
@@ -426,6 +602,8 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			addr = cmd_to_addr(cmds);
 		}
 
+		cmd_state_set_reg(&st, cmd);
+
 		switch (cmd) {
 		case NPU_OP_STOP:
 			if (i != size / 4 - 1)
@@ -433,8 +611,10 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			ends_with_stop = true;
 			break;
 		case NPU_OP_DMA_START:
-			srclen = dma_length(info, &st.dma, &st.dma.src);
-			dstlen = dma_length(info, &st.dma, &st.dma.dst);
+			srclen = dma_length(edev, info, &st, &st.dma, &st.dma.src,
+					    NPU_SET_DMA0_SRC_REGION, NPU_SET_DMA0_SRC);
+			dstlen = dma_length(edev, info, &st, &st.dma, &st.dma.dst,
+					    NPU_SET_DMA0_DST_REGION, NPU_SET_DMA0_DST);
 			if (srclen == U64_MAX || dstlen == U64_MAX)
 				return -EINVAL;
 
@@ -445,16 +625,28 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 				st.dma.dst.region, st.dma.dst.offset, dstlen);
 			break;
 		case NPU_OP_CONV:
-		case NPU_OP_DEPTHWISE:
 			use_ifm2 = param & 0x1;  // weights_ifm2
+			if (!cmd_state_reg_is_set(&st, NPU_SET_OFM_PRECISION))
+				return -EINVAL;
 			use_scale = !(st.ofm.precision & 0x100);
 			ret = calc_sizes(ddev, info, cmd, &st, true, use_ifm2,
 					 !use_ifm2, use_scale);
 			if (ret)
 				return ret;
 			break;
+		case NPU_OP_DEPTHWISE:
+			if (!cmd_state_reg_is_set(&st, NPU_SET_OFM_PRECISION))
+				return -EINVAL;
+			use_scale = !(st.ofm.precision & 0x100);
+			ret = calc_sizes(ddev, info, cmd, &st, true, false, true,
+					 use_scale);
+			if (ret)
+				return ret;
+			break;
 		case NPU_OP_POOL:
 			use_ifm = param != 0x4;  // pooling mode
+			if (!cmd_state_reg_is_set(&st, NPU_SET_OFM_PRECISION))
+				return -EINVAL;
 			use_scale = !(st.ofm.precision & 0x100);
 			ret = calc_sizes(ddev, info, cmd, &st, use_ifm, false,
 					 false, use_scale);
@@ -462,11 +654,18 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 				return ret;
 			break;
 		case NPU_OP_ELEMENTWISE:
-			use_scale = ethosu_is_u65(edev) ?
+			if (!ethosu_is_u65(edev) &&
+			    !cmd_state_reg_is_set(&st, NPU_SET_IFM_BROADCAST))
+				return -EINVAL;
+			use_ifm2 = (param != 5) && (param != 6) &&
+				(param != 7) && (param != 0x24);
+			if (use_ifm2 &&
+			    !cmd_state_reg_is_set(&st, NPU_SET_IFM2_BROADCAST))
+				return -EINVAL;
+			use_scale = use_ifm2 && (ethosu_is_u65(edev) ?
 				    (st.ifm2.broadcast & 0x80) :
-				    (st.ifm2.broadcast == 8);
-			use_ifm2 = !(use_scale || (param == 5) ||
-				(param == 6) || (param == 7) || (param == 0x24));
+				    (st.ifm2.broadcast == 8));
+			use_ifm2 = use_ifm2 && !use_scale;
 			use_ifm = st.ifm.broadcast != 8;
 			ret = calc_sizes_elemwise(ddev, info, cmd, &st, use_ifm, use_ifm2);
 			if (ret)

-- 
2.53.0

Re: [PATCH v2 06/19] accel: ethosu: Track command stream register setup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dma_params_valid() incorrectly returns U64_MAX in a boolean function, which evaluates to true and bypasses missing DMA parameter validation.
--

commit 6f2462930489d04d8fd80b48b9dfe3672578205e
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Track command stream register setup

The current method of tracking command stream state initializing state
tracking to illegal values and assuming unaccessed registers are 0 is
proving inadequate with additional validation. Instead, track all the
registers in a bitmap as the register address space is fairly small.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 408b93350dd79..8a44a5d89bce6 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -158,13 +185,54 @@ static u64 cmd_to_addr(u32 *cmd)
[ ... ]
> +static bool dma_params_valid(struct ethosu_device *edev, struct cmd_state *st,
> +			     const struct dma_state *dma_st,
> +			     const struct dma *dma,
> +			     u16 region_cmd, u16 addr_cmd)
> +{
> +	s8 mode = dma->mode;
> +
> +	if (!cmd_state_reg_is_set(st, region_cmd) ||
> +	    !cmd_state_reg_is_set(st, addr_cmd) ||
> +	    !cmd_state_reg_is_set(st, NPU_SET_DMA0_LEN) || mode < 0 || mode > 2)
> +		return false;
> +
> +	if (mode >= 1 &&
> +	    !cmd_state_reg_is_set(st, dma_use_src_stride(edev, dma_st, dma) ?
> +				  NPU_SET_DMA0_SRC_STRIDE0 :
> +				  NPU_SET_DMA0_DST_STRIDE0))
> +		return U64_MAX;

[Severity: High]
Since dma_params_valid() returns a bool, does returning U64_MAX here get
converted to true?

If an attacker intentionally omits the stride command in a malicious stream,
could this evaluate as true and inadvertently bypass the validation check,
potentially leading to out-of-bounds DMA accesses using retained hardware
stride values?

> +	if (mode == 2 &&
> +	    !cmd_state_reg_is_set(st, dma_use_src_stride(edev, dma_st, dma) ?
> +				  NPU_SET_DMA0_SRC_STRIDE1 :
> +				  NPU_SET_DMA0_DST_STRIDE1))
> +		return U64_MAX;

[Severity: High]
Does returning U64_MAX here also inadvertently evaluate to true?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=6

[PATCH v2 07/19] accel: ethosu: Factor buffer bounds checks

[Rob Herring (Arm)]

Move the repeated command-stream buffer range validation into a
helper in preparation for validating all weight and scale streams.

Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - Adjust due to previous patch
---
 drivers/accel/ethosu/ethosu_gem.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 8a44a5d89bce..0b1edf02d48e 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -423,6 +423,25 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
 	return addr;
 }
 
+static int buffer_size(struct ethosu_validated_cmdstream_info *info,
+		       struct cmd_state *st, struct buffer *buf, s8 region,
+		       u16 region_cmd, u16 base_cmd, u16 length_cmd)
+{
+	u64 end;
+
+	if (region < 0 || !cmd_state_reg_is_set(st, region_cmd) ||
+	    !cmd_state_reg_is_set(st, base_cmd) ||
+	    !cmd_state_reg_is_set(st, length_cmd))
+		return -EINVAL;
+
+	if (check_add_overflow(buf->base, (u64)buf->length, &end))
+		return -EINVAL;
+
+	info->region_size[region] = max(info->region_size[region], end);
+
+	return 0;
+}
+
 static int calc_sizes(struct drm_device *ddev,
 		      struct ethosu_validated_cmdstream_info *info,
 		      u16 op, struct cmd_state *st,
@@ -475,26 +494,20 @@ static int calc_sizes(struct drm_device *ddev,
 		dev_dbg(ddev->dev, "op %d: W:%d:0x%llx-0x%llx\n",
 			op, st->weight[0].region, st->weight[0].base,
 			st->weight[0].base + st->weight[0].length - 1);
-		if (!cmd_state_reg_is_set(st, NPU_SET_WEIGHT_REGION) ||
-		    !cmd_state_reg_is_set(st, NPU_SET_WEIGHT_BASE) ||
-		    !cmd_state_reg_is_set(st, NPU_SET_WEIGHT_LENGTH))
+		if (buffer_size(info, st, &st->weight[0], st->weight[0].region,
+				NPU_SET_WEIGHT_REGION, NPU_SET_WEIGHT_BASE,
+				NPU_SET_WEIGHT_LENGTH))
 			return -EINVAL;
-		info->region_size[st->weight[0].region] =
-			max(info->region_size[st->weight[0].region],
-			    st->weight[0].base + st->weight[0].length);
 	}
 
 	if (scale) {
 		dev_dbg(ddev->dev, "op %d: S:%d:0x%llx-0x%llx\n",
 			op, st->scale[0].region, st->scale[0].base,
 			st->scale[0].base + st->scale[0].length - 1);
-		if (!cmd_state_reg_is_set(st, NPU_SET_SCALE_REGION) ||
-		    !cmd_state_reg_is_set(st, NPU_SET_SCALE_BASE) ||
-		    !cmd_state_reg_is_set(st, NPU_SET_SCALE_LENGTH))
+		if (buffer_size(info, st, &st->scale[0], st->scale[0].region,
+				NPU_SET_SCALE_REGION, NPU_SET_SCALE_BASE,
+				NPU_SET_SCALE_LENGTH))
 			return -EINVAL;
-		info->region_size[st->scale[0].region] =
-			max(info->region_size[st->scale[0].region],
-			    st->scale[0].base + st->scale[0].length);
 	}
 
 	len = feat_matrix_length(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,

-- 
2.53.0

[PATCH v2 08/19] accel: ethosu: Validate secondary streams

[Rob Herring (Arm)]

The command-stream validator records the additional U65 scale and
weight stream addresses and the U85 weight decoder addresses, but only
checked stream 0 against its region buffer.

Check every configured secondary stream against the matching weight or
scale region before accepting a kernel operation.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - Adjust for register state tracking
---
 drivers/accel/ethosu/ethosu_gem.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 0b1edf02d48e..ec2832eb9a07 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -425,13 +425,17 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
 
 static int buffer_size(struct ethosu_validated_cmdstream_info *info,
 		       struct cmd_state *st, struct buffer *buf, s8 region,
-		       u16 region_cmd, u16 base_cmd, u16 length_cmd)
+		       u16 region_cmd, u16 base_cmd, u16 length_cmd, bool optional)
 {
 	u64 end;
+	bool base_set = cmd_state_reg_is_set(st, base_cmd);
+	bool length_set = cmd_state_reg_is_set(st, length_cmd);
+
+	if (optional && !base_set && !length_set)
+		return 0;
 
 	if (region < 0 || !cmd_state_reg_is_set(st, region_cmd) ||
-	    !cmd_state_reg_is_set(st, base_cmd) ||
-	    !cmd_state_reg_is_set(st, length_cmd))
+	    !base_set || !length_set)
 		return -EINVAL;
 
 	if (check_add_overflow(buf->base, (u64)buf->length, &end))
@@ -496,7 +500,20 @@ static int calc_sizes(struct drm_device *ddev,
 			st->weight[0].base + st->weight[0].length - 1);
 		if (buffer_size(info, st, &st->weight[0], st->weight[0].region,
 				NPU_SET_WEIGHT_REGION, NPU_SET_WEIGHT_BASE,
-				NPU_SET_WEIGHT_LENGTH))
+				NPU_SET_WEIGHT_LENGTH, false))
+			return -EINVAL;
+
+		if (buffer_size(info, st, &st->weight[1], st->weight[0].region,
+				NPU_SET_WEIGHT_REGION, NPU_SET_WEIGHT1_BASE,
+				NPU_SET_WEIGHT1_LENGTH, true) ||
+		    buffer_size(info, st, &st->weight[3], st->weight[0].region,
+				NPU_SET_WEIGHT_REGION, NPU_SET_WEIGHT3_BASE,
+				NPU_SET_WEIGHT3_LENGTH, true))
+			return -EINVAL;
+		if (!ethosu_is_u65(edev) &&
+		    buffer_size(info, st, &st->weight[2], st->weight[0].region,
+				NPU_SET_WEIGHT_REGION, NPU_SET_WEIGHT2_BASE,
+				NPU_SET_WEIGHT2_LENGTH, true))
 			return -EINVAL;
 	}
 
@@ -506,7 +523,13 @@ static int calc_sizes(struct drm_device *ddev,
 			st->scale[0].base + st->scale[0].length - 1);
 		if (buffer_size(info, st, &st->scale[0], st->scale[0].region,
 				NPU_SET_SCALE_REGION, NPU_SET_SCALE_BASE,
-				NPU_SET_SCALE_LENGTH))
+				NPU_SET_SCALE_LENGTH, false))
+			return -EINVAL;
+
+		if (ethosu_is_u65(edev) &&
+		    buffer_size(info, st, &st->scale[1], st->scale[0].region,
+				NPU_SET_SCALE_REGION, NPU_SET_SCALE1_BASE,
+				NPU_SET_SCALE1_LENGTH, true))
 			return -EINVAL;
 	}
 

-- 
2.53.0

[PATCH v2 09/19] accel: ethosu: Reject unsupported commands

[Rob Herring (Arm)]

The command-stream validator does not model U85 branches, indexed DMA,
or OFM transposes. A branch can bypass the linear validation state,
indexed DMA accesses an unchecked index buffer, and a transpose changes
the feature-map address calculation.

Reject those commands and configurations, as well as the reserved DMA
stride mode and feature-map formats. Reject command-stream IRQs because
they can signal job completion before later commands finish.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - no changes
---
 drivers/accel/ethosu/ethosu_device.h |  4 ++++
 drivers/accel/ethosu/ethosu_gem.c    | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index 1eca8590e68d..c330048dbcca 100644
--- a/drivers/accel/ethosu/ethosu_device.h
+++ b/drivers/accel/ethosu/ethosu_device.h
@@ -86,14 +86,18 @@ struct gen_pool;
 #define PMU_EV_TYPE_CYCLES	0x11
 #define PMU_EV_TYPE_IDLE	0x20
 
+#define NPU_DMA_REGION_INDEX_MODE	BIT(11)
+
 enum ethosu_cmds {
 	NPU_OP_STOP = 0x0,
+	NPU_OP_IRQ = 0x1,
 	NPU_OP_CONV = 0x2,
 	NPU_OP_DEPTHWISE = 0x3,
 	NPU_OP_POOL = 0x5,
 	NPU_OP_ELEMENTWISE = 0x6,
 	NPU_OP_RESIZE = 0x7,	// U85 only
 	NPU_OP_DMA_START = 0x10,
+	NPU_OP_BRANCH = 0x4100,	// U85 only
 	NPU_SET_IFM_PAD_TOP = 0x100,
 	NPU_SET_IFM_PAD_LEFT = 0x101,
 	NPU_SET_IFM_PAD_RIGHT = 0x102,
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index ec2832eb9a07..c913c95e48ae 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -641,6 +641,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		cmd_state_set_reg(&st, cmd);
 
 		switch (cmd) {
+		case NPU_OP_BRANCH:
+		case NPU_OP_IRQ:
+			return -EINVAL;
 		case NPU_OP_STOP:
 			if (i != size / 4 - 1)
 				return -EINVAL;
@@ -734,6 +737,8 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			st.ifm.depth = param;
 			break;
 		case NPU_SET_IFM_PRECISION:
+			if (((param >> 6) & 0x3) > 1)
+				return -EINVAL;
 			st.ifm.precision = param;
 			break;
 		case NPU_SET_IFM_BROADCAST:
@@ -777,6 +782,10 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			st.ofm.depth = param;
 			break;
 		case NPU_SET_OFM_PRECISION:
+			if (((param >> 6) & 0x3) > 1)
+				return -EINVAL;
+			if (!ethosu_is_u65(edev) && (param & GENMASK(13, 11)))
+				return -EINVAL;
 			st.ofm.precision = param;
 			break;
 		case NPU_SET_OFM_REGION:
@@ -811,6 +820,8 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			st.ifm2.broadcast = param;
 			break;
 		case NPU_SET_IFM2_PRECISION:
+			if (((param >> 6) & 0x3) > 1)
+				return -EINVAL;
 			st.ifm2.precision = param;
 			break;
 		case NPU_SET_IFM2_REGION:
@@ -885,18 +896,26 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			break;
 
 		case NPU_SET_DMA0_SRC_REGION:
+			if (param & NPU_DMA_REGION_INDEX_MODE)
+				return -EINVAL;
 			if (param & 0x100)
 				st.dma.src.region = -1;
 			else
 				st.dma.src.region = param & 0x7;
 			st.dma.src.mode = (param >> 9) & 0x3;
+			if (st.dma.src.mode == 3)
+				return -EINVAL;
 			break;
 		case NPU_SET_DMA0_DST_REGION:
+			if (param & NPU_DMA_REGION_INDEX_MODE)
+				return -EINVAL;
 			if (param & 0x100)
 				st.dma.dst.region = -1;
 			else
 				st.dma.dst.region = param & 0x7;
 			st.dma.dst.mode = (param >> 9) & 0x3;
+			if (st.dma.dst.mode == 3)
+				return -EINVAL;
 			break;
 		case NPU_SET_DMA0_SIZE0:
 			st.dma.size0 = param;

-- 
2.53.0

Re: [PATCH v2 09/19] accel: ethosu: Reject unsupported commands

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Time-of-Check to Time-of-Use (TOCTOU) vulnerability due to DMA-BUF export of command stream BO
- [Medium] Sandbox escape vulnerability due to default-allow validation of unknown commands
--

commit d6039ab4982bea39258f6865972e83f60eb4978c
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Reject unsupported commands

The command-stream validator does not model U85 branches, indexed DMA,
or OFM transposes. A branch can bypass the linear validation state,
indexed DMA accesses an unchecked index buffer, and a transpose changes
the feature-map address calculation.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index ec2832eb9a07a..c913c95e48ae0 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -885,18 +896,26 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			break;
>  
>  		case NPU_SET_DMA0_SRC_REGION:
> +			if (param & NPU_DMA_REGION_INDEX_MODE)
> +				return -EINVAL;
[ ... ]

[Severity: Medium]
This is a pre-existing issue, but I noticed a potential sandbox escape
vulnerability in this switch statement due to default-allow validation
of unknown commands.

In ethosu_gem_cmdstream_copy_and_validate(), if a future NPU hardware
revision introduces new opcodes that perform memory accesses, the current
driver will blindly allow them without calculating their memory bounds
because of the default case:

        case NPU_SET_DMA0_LEN:
            st.dma.src.len = st.dma.dst.len = addr;
            break;
        default:
            break;
        }

Can this allow the NPU to execute unvalidated commands and access memory
outside the validated BO regions if such opcodes are introduced?

[Severity: High]
This is also a pre-existing issue, but could this driver be vulnerable to a
Time-of-Check to Time-of-Use (TOCTOU) attack via DMA-BUF export?

In drivers/accel/ethosu/ethosu_gem.c:ethosu_gem_funcs, the .get_sg_table
callback is exposed without overriding the .export callback:

static const struct drm_gem_object_funcs ethosu_gem_funcs = {
    .free = ethosu_gem_free_object,
    .print_info = drm_gem_dma_object_print_info,
    .get_sg_table = drm_gem_dma_object_get_sg_table,
    ...
};

This permits the command stream BO to be exported as a DMA-BUF and mapped
by other devices.

Meanwhile, in drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job(),
the job submission blindly trusts the pre-calculated cmd_info (including
region_size) without preventing concurrent modifications to the buffer
contents:

        /* Verify the command stream doesn't have accesses outside the BO */
        if (cmd_info->region_size[i] > gem->size) {
            dev_err(dev->dev,
                "cmd stream region %d size greater than BO size (%llu > %zu)\n",
                i, cmd_info->region_size[i], gem->size);
            ret = -EOVERFLOW;
            goto out_cleanup_job;
        }

Could a malicious userspace application create a command stream, export it
as a DMA-BUF to another device, and overwrite the command stream with
malicious instructions using the secondary device's DMA capabilities after
validation but before execution?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=9

[PATCH v2 10/19] accel: ethosu: Validate all feature map tiles

[Rob Herring (Arm)]

The command-stream validator checked only the final feature-map
coordinate. For tiled tensors, this can leave an earlier tile base
address unchecked even though the operation accesses it.

Check the final coordinate of every tile touched by an operation. Also
treat U65 feature maps as 2x2 tiled: its precision rounding bits are not
the U85 storage encoding.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - no changes
---
 drivers/accel/ethosu/ethosu_gem.c | 125 +++++++++++++++++++++++++++++---------
 1 file changed, 97 insertions(+), 28 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index c913c95e48ae..c54496fa08f4 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -423,6 +423,74 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
 	return addr;
 }
 
+static int feat_matrix_check_location(struct ethosu_device *edev,
+				      struct ethosu_validated_cmdstream_info *info,
+				      struct cmd_state *st, struct feat_matrix *fm,
+				      enum feat_matrix_type type, u32 x, u32 y,
+				      u32 c, bool ofm, u64 *max_len)
+{
+	u64 len;
+
+	len = feat_matrix_length(edev, info, st, fm, type, x, y, c, ofm);
+	if (len == U64_MAX)
+		return -EINVAL;
+
+	*max_len = max(*max_len, len);
+	return 0;
+}
+
+static int feat_matrix_size(struct ethosu_device *edev,
+			    struct ethosu_validated_cmdstream_info *info,
+			    struct cmd_state *st, struct feat_matrix *fm,
+			    enum feat_matrix_type type,
+			    u32 x, u32 y, u32 c, bool ofm, u64 *max_len)
+{
+	u32 storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14;
+	int ret;
+
+	*max_len = 0;
+
+	if (ethosu_is_u65(edev) || storage == 0) {
+		for (int xi = 0; xi < 2; xi++) {
+			for (int yi = 0; yi < 2; yi++) {
+				ret = feat_matrix_check_location(edev, info, st, fm, type,
+								 xi ? x : 0,
+								 yi ? y : 0, c, ofm,
+								 max_len);
+				if (ret)
+					return ret;
+			}
+		}
+		return 0;
+	}
+
+	if (storage == 1) {
+		ret = feat_matrix_check_location(edev, info, st, fm, type, x, 0, c,
+						 ofm, max_len);
+		if (ret)
+			return ret;
+		if (fm->height[0] < fm->height[1] && fm->height[1] <= y) {
+			ret = feat_matrix_check_location(edev, info, st, fm, type, x,
+							 fm->height[1], c, ofm,
+							 max_len);
+			if (ret)
+				return ret;
+		}
+		if (fm->height[1] < y) {
+			ret = feat_matrix_check_location(edev, info, st, fm, type, x,
+							 fm->height[1] + 1, c, ofm,
+							 max_len);
+			if (ret)
+				return ret;
+		}
+		return feat_matrix_check_location(edev, info, st, fm, type, x, y, c,
+						  ofm, max_len);
+	}
+
+	return feat_matrix_check_location(edev, info, st, fm, type, x, y, c, ofm,
+					  max_len);
+}
+
 static int buffer_size(struct ethosu_validated_cmdstream_info *info,
 		       struct cmd_state *st, struct buffer *buf, s8 region,
 		       u16 region_cmd, u16 base_cmd, u16 length_cmd, bool optional)
@@ -453,6 +521,7 @@ static int calc_sizes(struct drm_device *ddev,
 {
 	struct ethosu_device *edev = to_ethosu_device(ddev);
 	u64 len;
+	int ret;
 
 	if (ifm) {
 		if (!cmd_state_reg_is_set(st, NPU_SET_KERNEL_WIDTH_M1) ||
@@ -475,23 +544,22 @@ static int calc_sizes(struct drm_device *ddev,
 		if (ifm_height < 0 || ifm_width < 0)
 			return -EINVAL;
 
-		len = feat_matrix_length(edev, info, st, &st->ifm,
-					 FEAT_MATRIX_IFM, ifm_width, ifm_height,
-					 st->ifm.depth, false);
+		ret = feat_matrix_size(edev, info, st, &st->ifm, FEAT_MATRIX_IFM,
+				       ifm_width, ifm_height, st->ifm.depth, false,
+				       &len);
 		dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n",
 			op, st->ifm.region, st->ifm.base[0], len);
-		if (len == U64_MAX)
-			return -EINVAL;
+		if (ret)
+			return ret;
 	}
 
 	if (ifm2) {
-		len = feat_matrix_length(edev, info, st, &st->ifm2,
-					 FEAT_MATRIX_IFM2, st->ifm.depth, 0,
-					 st->ofm.depth, false);
+		ret = feat_matrix_size(edev, info, st, &st->ifm2, FEAT_MATRIX_IFM2,
+				       st->ifm.depth, 0, st->ofm.depth, false, &len);
 		dev_dbg(ddev->dev, "op %d: IFM2:%d:0x%llx-0x%llx\n",
 			op, st->ifm2.region, st->ifm2.base[0], len);
-		if (len == U64_MAX)
-			return -EINVAL;
+		if (ret)
+			return ret;
 	}
 
 	if (weight) {
@@ -533,13 +601,13 @@ static int calc_sizes(struct drm_device *ddev,
 			return -EINVAL;
 	}
 
-	len = feat_matrix_length(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,
-				 st->ofm.width, st->ofm.height[2], st->ofm.depth,
-				 true);
+	ret = feat_matrix_size(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,
+			       st->ofm.width, st->ofm.height[2], st->ofm.depth,
+			       true, &len);
 	dev_dbg(ddev->dev, "op %d: OFM:%d:0x%llx-0x%llx\n",
 		op, st->ofm.region, st->ofm.base[0], len);
-	if (len == U64_MAX)
-		return -EINVAL;
+	if (ret)
+		return ret;
 	if (!feat_matrix_chained(edev, &st->ofm))
 		info->output_region[st->ofm.region] = true;
 
@@ -554,18 +622,19 @@ static int calc_sizes_elemwise(struct drm_device *ddev,
 	struct ethosu_device *edev = to_ethosu_device(ddev);
 	u32 height, width, depth;
 	u64 len;
+	int ret;
 
 	if (ifm) {
 		height = st->ifm.broadcast & 0x1 ? 0 : st->ofm.height[2];
 		width = st->ifm.broadcast & 0x2 ? 0 : st->ofm.width;
 		depth = st->ifm.broadcast & 0x4 ? 0 : st->ofm.depth;
 
-		len = feat_matrix_length(edev, info, st, &st->ifm,
-					 FEAT_MATRIX_IFM, width, height, depth, false);
+		ret = feat_matrix_size(edev, info, st, &st->ifm, FEAT_MATRIX_IFM,
+				       width, height, depth, false, &len);
 		dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n",
 			op, st->ifm.region, st->ifm.base[0], len);
-		if (len == U64_MAX)
-			return -EINVAL;
+		if (ret)
+			return ret;
 	}
 
 	if (ifm2) {
@@ -573,21 +642,21 @@ static int calc_sizes_elemwise(struct drm_device *ddev,
 		width = st->ifm2.broadcast & 0x2 ? 0 : st->ofm.width;
 		depth = st->ifm2.broadcast & 0x4 ? 0 : st->ofm.depth;
 
-		len = feat_matrix_length(edev, info, st, &st->ifm2,
-					 FEAT_MATRIX_IFM2, width, height, depth, false);
+		ret = feat_matrix_size(edev, info, st, &st->ifm2, FEAT_MATRIX_IFM2,
+				       width, height, depth, false, &len);
 		dev_dbg(ddev->dev, "op %d: IFM2:%d:0x%llx-0x%llx\n",
 			op, st->ifm2.region, st->ifm2.base[0], len);
-		if (len == U64_MAX)
-			return -EINVAL;
+		if (ret)
+			return ret;
 	}
 
-	len = feat_matrix_length(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,
-				 st->ofm.width, st->ofm.height[2], st->ofm.depth,
-				 true);
+	ret = feat_matrix_size(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,
+			       st->ofm.width, st->ofm.height[2], st->ofm.depth,
+			       true, &len);
 	dev_dbg(ddev->dev, "op %d: OFM:%d:0x%llx-0x%llx\n",
 		op, st->ofm.region, st->ofm.base[0], len);
-	if (len == U64_MAX)
-		return -EINVAL;
+	if (ret)
+		return ret;
 	if (!feat_matrix_chained(edev, &st->ofm))
 		info->output_region[st->ofm.region] = true;
 

-- 
2.53.0

Re: [PATCH v2 10/19] accel: ethosu: Validate all feature map tiles

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incomplete bounds validation for multi-tile feature maps allows NPU out-of-bounds memory access (stride overflow).
--

commit ed82a852cb38061cea72ccfbb8b9844fe471cc0f
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Validate all feature map tiles

The commit modifies the command-stream validator to check the final
coordinate of every tile touched by an operation, rather than just the
final feature-map coordinate. It also updates the treatment of U65 feature
maps to be checked as 2x2 tiled.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index c913c95e48ae0..c54496fa08f4c 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -423,6 +423,74 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
[ ... ]
> +static int feat_matrix_size(struct ethosu_device *edev,
> +			    struct ethosu_validated_cmdstream_info *info,
> +			    struct cmd_state *st, struct feat_matrix *fm,
> +			    enum feat_matrix_type type,
> +			    u32 x, u32 y, u32 c, bool ofm, u64 *max_len)
> +{
> +	u32 storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14;
> +	int ret;
> +
> +	*max_len = 0;
> +
> +	if (ethosu_is_u65(edev) || storage == 0) {
> +		for (int xi = 0; xi < 2; xi++) {
> +			for (int yi = 0; yi < 2; yi++) {
> +				ret = feat_matrix_check_location(edev, info, st, fm, type,
> +								 xi ? x : 0,
> +								 yi ? y : 0, c, ofm,
> +								 max_len);

[Severity: Critical]
This isn't a bug introduced by this patch, but it appears the validation
logic might still be incomplete for intermediate tiles. In feat_matrix_size(),
when checking storage == 0, the loop evaluates to 0 (the relative minimum
coordinate) for intermediate tiles rather than checking the tile's maximum
extent. 

Because feat_matrix_length() calculates offset by multiplying strides by the
passed coordinates, passing 0 skips validating the maximum memory offset
for tile 0 (which should be evaluated at fm->width0 and fm->height[0]).
Could an ioctl command stream from unprivileged userspace, carrying a valid
base address but artificially large strides, bypass bounds checking here and
cause the NPU to access out-of-bounds memory?

> +				if (ret)
> +					return ret;
> +			}
> +		}
> +		return 0;
> +	}
> +
> +	if (storage == 1) {
> +		ret = feat_matrix_check_location(edev, info, st, fm, type, x, 0, c,
> +						 ofm, max_len);
> +		if (ret)
> +			return ret;
> +		if (fm->height[0] < fm->height[1] && fm->height[1] <= y) {
> +			ret = feat_matrix_check_location(edev, info, st, fm, type, x,
> +							 fm->height[1], c, ofm,
> +							 max_len);

[Severity: Critical]
This is also a pre-existing issue, but similar to the loop above, does this
storage == 1 logic skip validating the maximum memory bound of tile 0?

The code explicitly checks the boundaries at y = 0 and y = fm->height[1], but
it seems to skip verifying the maximum extent of tile 0 at y = fm->height[0].
Would this allow unprivileged userspace to trigger out-of-bounds accesses
during intermediate tile processing if the vertical stride is maliciously
large?

> +			if (ret)
> +				return ret;
> +		}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=10

[PATCH v2 11/19] accel: ethosu: Account for feature map element size

[Rob Herring (Arm)]

The feature map bounds calculation currently accounts for only one byte
of the final element. This can leave the tail of B16, B32, and B64
elements outside the validated buffer.

Use the feature map element size when calculating the final byte.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 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 c54496fa08f4..bca68fab0527 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -415,7 +415,7 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
 		return U64_MAX;
 	}
 
-	if (check_add_overflow(addr, 1ULL, &offset))
+	if (check_add_overflow(addr, (u64)element_size, &offset))
 		return U64_MAX;
 
 	info->region_size[fm->region] = max(info->region_size[fm->region], offset);

-- 
2.53.0

Re: [PATCH v2 11/19] accel: ethosu: Account for feature map element size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incomplete bounds validation for multi-tile feature maps in `feat_matrix_size` allows the NPU to access memory outside the GEM buffer.
- [High] The `feat_matrix_length` function does not account for the 16-channel hardware brick alignment for NHCWB16 formats, underestimating the buffer size.
--

commit 5efa0071e22603105740c81590f076bdcb4fef2a
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Account for feature map element size

The feature map bounds calculation currently accounts for only one byte
of the final element. This can leave the tail of B16, B32, and B64
elements outside the validated buffer, so the commit uses the feature map
element size when calculating the final byte.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index c54496fa08f4c..bca68fab05275 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -415,7 +415,7 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
>  		return U64_MAX;
>  	}
>  
> -	if (check_add_overflow(addr, 1ULL, &offset))
> +	if (check_add_overflow(addr, (u64)element_size, &offset))

[Severity: High]
This isn't a bug introduced by this patch, but while the patch adds a missing
single element_size to the calculation, is it sufficient for NHCWB16 formats?

Looking slightly above in feat_matrix_length():

	case 1: //nhcwb16:
		element_size = BIT((fm->precision >> (ofm ? 1 : 2)) & 0x3);

		if (check_mul_overflow(c / 16, (u64)fm->stride_c, &offset) ||
		    check_add_overflow(addr, offset, &addr) ||
		    check_mul_overflow(16 * x + (c & 0xf), element_size, &offset) ||
		    check_add_overflow(addr, offset, &addr))
			return U64_MAX;
		break;

The Ethos-U NPU inherently processes NHCWB16 data in 16-channel bricks. If
the channel depth modulo 16 is less than 15, the hardware will still read
or write the full 16-element brick.

Because the validation calculation stops precisely after the specified channel
and doesn't appear to round up the offset to the 16-element brick boundary,
could it underestimate the required buffer size by up to 15 * element_size
bytes, leading to out-of-bounds hardware memory accesses?

>  		return U64_MAX;
>  
>  	info->region_size[fm->region] = max(info->region_size[fm->region], offset);


[Severity: Critical]
This is a pre-existing issue, but incomplete bounds validation for multi-tile
feature maps in feat_matrix_size() might allow the NPU to access memory
outside the GEM buffer.

In drivers/accel/ethosu/ethosu_gem.c:feat_matrix_size():

	if (ethosu_is_u65(edev) || storage == 0) {
		for (int xi = 0; xi < 2; xi++) {
			for (int yi = 0; yi < 2; yi++) {
				ret = feat_matrix_check_location(edev, info, st, fm, type,
								 xi ? x : 0,
								 yi ? y : 0, c, ofm,
								 max_len);
				if (ret)
					return ret;
			}
		}
		return 0;
	}

The loop evaluates the minimum and global maximum coordinates but skips the
internal boundary coordinates (like fm->width0) of intermediate tiles.

Similarly, for vertically split tiles (storage == 1):

	if (storage == 1) {
		ret = feat_matrix_check_location(edev, info, st, fm, type, x, 0, c,
						 ofm, max_len);

The maximum Y coordinate within the first tile (fm->height[0]) is completely
omitted from bounds validation.

If a multi-tile feature map is submitted, and tile 0's base address is
configured near the end of the GEM allocation, with a massive internal
width0 and stride_x for tile 0 (while properly bounding subsequent tiles
so the global maximum falls into tile 1), could this artificially bypass
validation for the rest of tile 0 and allow unconstrained memory accesses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=11

[PATCH v2 12/19] accel: ethosu: Validate convolution parameter

[Rob Herring (Arm)]

The U65 NPU_OP_CONV command has no parameter fields, but the
validator interpreted bit zero as the U85 weights_ifm2 field. A crafted
U65 stream could consequently make validation skip the weight buffer
that hardware accesses.

Require a zero parameter on U65 and reject the reserved U85 parameter
bits.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_device.h | 1 +
 drivers/accel/ethosu/ethosu_gem.c    | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index c330048dbcca..d7e1e3c8ca12 100644
--- a/drivers/accel/ethosu/ethosu_device.h
+++ b/drivers/accel/ethosu/ethosu_device.h
@@ -87,6 +87,7 @@ struct gen_pool;
 #define PMU_EV_TYPE_IDLE	0x20
 
 #define NPU_DMA_REGION_INDEX_MODE	BIT(11)
+#define NPU_OP_CONV_WEIGHTS_IFM2	BIT(0)
 
 enum ethosu_cmds {
 	NPU_OP_STOP = 0x0,
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index bca68fab0527..d60a2a453302 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -733,7 +733,10 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 				st.dma.dst.region, st.dma.dst.offset, dstlen);
 			break;
 		case NPU_OP_CONV:
-			use_ifm2 = param & 0x1;  // weights_ifm2
+			if ((ethosu_is_u65(edev) && param) || (param & ~NPU_OP_CONV_WEIGHTS_IFM2))
+				return -EINVAL;
+
+			use_ifm2 = param & NPU_OP_CONV_WEIGHTS_IFM2;
 			if (!cmd_state_reg_is_set(&st, NPU_SET_OFM_PRECISION))
 				return -EINVAL;
 			use_scale = !(st.ofm.precision & 0x100);

-- 
2.53.0

Re: [PATCH v2 12/19] accel: ethosu: Validate convolution parameter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The command validator fails to validate the `scale` buffer bounds for `NPU_OP_ELEMENTWISE` operations, potentially allowing out-of-bounds NPU memory reads.
--

commit ce7a8243218e8f4759fe093516845ea9fe6bb544
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Validate convolution parameter

The commit fixes a validation issue with the U65 NPU_OP_CONV command where a
crafted stream could skip the weight buffer validation. It requires a zero
parameter on U65 and rejects reserved U85 parameter bits.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index bca68fab05275..d60a2a453302b 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -733,7 +733,10 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  				st.dma.dst.region, st.dma.dst.offset, dstlen);
>  			break;
>  		case NPU_OP_CONV:
> -			use_ifm2 = param & 0x1;  // weights_ifm2
> +			if ((ethosu_is_u65(edev) && param) || (param & ~NPU_OP_CONV_WEIGHTS_IFM2))
> +				return -EINVAL;
> +
> +			use_ifm2 = param & NPU_OP_CONV_WEIGHTS_IFM2;
>  			if (!cmd_state_reg_is_set(&st, NPU_SET_OFM_PRECISION))
>  				return -EINVAL;
>  			use_scale = !(st.ofm.precision & 0x100);

[ ... ]

>  		case NPU_OP_ELEMENTWISE:
>  			if (!ethosu_is_u65(edev) &&
>  			    !cmd_state_reg_is_set(&st, NPU_SET_IFM_BROADCAST))
>  				return -EINVAL;
>  			use_ifm2 = (param != 5) && (param != 6) &&
>  				(param != 7) && (param != 0x24);
>  			if (use_ifm2 &&
>  			    !cmd_state_reg_is_set(&st, NPU_SET_IFM2_BROADCAST))
>  				return -EINVAL;
>  			use_scale = use_ifm2 && (ethosu_is_u65(edev) ?
>  				    (st.ifm2.broadcast & 0x80) :
>  				    (st.ifm2.broadcast == 8));
>  			use_ifm2 = use_ifm2 && !use_scale;
>  			use_ifm = st.ifm.broadcast != 8;
>  			ret = calc_sizes_elemwise(ddev, info, cmd, &st, use_ifm, use_ifm2);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the changes to
the validation in ethosu_gem_cmdstream_copy_and_validate(), I noticed that
the scale buffer bounds don't seem to be validated for NPU_OP_ELEMENTWISE
operations.

For NPU_OP_ELEMENTWISE commands, the driver parses the broadcast
configuration and sets use_scale = true. However, use_scale is never
passed to calc_sizes_elemwise() and the scale buffer bounds are never
validated.

Could this allow an out-of-bounds NPU memory read if a userspace application
submits a crafted command stream containing an NPU_OP_ELEMENTWISE command
with a broadcast configuration that uses the scale buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=12

[PATCH v2 13/19] accel: ethosu: Account for kernel dilation in IFM size

[Rob Herring (Arm)]

Kernel dilation increases the input feature-map area accessed by a
kernel operation. Include the x and y dilation settings when
calculating the required IFM bounds.

Without this, a command stream using x2 dilation can access IFM
memory past the range validated by the driver.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_device.h |  2 ++
 drivers/accel/ethosu/ethosu_gem.c    | 10 ++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index d7e1e3c8ca12..8e23fdbf7f8a 100644
--- a/drivers/accel/ethosu/ethosu_device.h
+++ b/drivers/accel/ethosu/ethosu_device.h
@@ -88,6 +88,8 @@ struct gen_pool;
 
 #define NPU_DMA_REGION_INDEX_MODE	BIT(11)
 #define NPU_OP_CONV_WEIGHTS_IFM2	BIT(0)
+#define NPU_KERNEL_DILATION_X		BIT(3)
+#define NPU_KERNEL_DILATION_Y		BIT(4)
 
 enum ethosu_cmds {
 	NPU_OP_STOP = 0x0,
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index d60a2a453302..63dd07791f31 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -536,10 +536,16 @@ static int calc_sizes(struct drm_device *ddev,
 			((st->ifm.stride_kernel >> 1) & 0x1) + 1;
 		u32 stride_x = ((st->ifm.stride_kernel >> 5) & 0x2) +
 			(st->ifm.stride_kernel & 0x1) + 1;
+		u32 dilation_y = 1 + !!(st->ifm.stride_kernel &
+					 NPU_KERNEL_DILATION_Y);
+		u32 dilation_x = 1 + !!(st->ifm.stride_kernel &
+					 NPU_KERNEL_DILATION_X);
 		s32 ifm_height = st->ofm.height[2] * stride_y +
-			st->ifm.height[2] - (st->ifm.pad_top + st->ifm.pad_bottom);
+			st->ifm.height[2] * dilation_y -
+			(st->ifm.pad_top + st->ifm.pad_bottom);
 		s32 ifm_width = st->ofm.width * stride_x +
-			st->ifm.width - (st->ifm.pad_left + st->ifm.pad_right);
+			st->ifm.width * dilation_x -
+			(st->ifm.pad_left + st->ifm.pad_right);
 
 		if (ifm_height < 0 || ifm_width < 0)
 			return -EINVAL;

-- 
2.53.0

Re: [PATCH v2 13/19] accel: ethosu: Account for kernel dilation in IFM size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The command stream validation in `feat_matrix_size` fails to check the maximum accessed address within intermediate tiles, allowing an attacker to bypass bounds checks and cause the NPU to access out-of-bounds memory.
--

commit eae7ad151b58329b8dda3932561e68aa3827908a
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Account for kernel dilation in IFM size

Kernel dilation increases the input feature-map area accessed by a kernel
operation. Include the x and y dilation settings when calculating the
required IFM bounds. Without this, a command stream using x2 dilation can
access IFM memory past the range validated by the driver.

> diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
> index d7e1e3c8ca12d..8e23fdbf7f8ab 100644
> --- a/drivers/accel/ethosu/ethosu_device.h
> +++ b/drivers/accel/ethosu/ethosu_device.h
[ ... ]
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index d60a2a453302b..63dd07791f314 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -536,10 +536,16 @@ static int calc_sizes(struct drm_device *ddev,
>  			((st->ifm.stride_kernel >> 1) & 0x1) + 1;
>  		u32 stride_x = ((st->ifm.stride_kernel >> 5) & 0x2) +
>  			(st->ifm.stride_kernel & 0x1) + 1;
> +		u32 dilation_y = 1 + !!(st->ifm.stride_kernel &
> +					 NPU_KERNEL_DILATION_Y);
> +		u32 dilation_x = 1 + !!(st->ifm.stride_kernel &
> +					 NPU_KERNEL_DILATION_X);
>  		s32 ifm_height = st->ofm.height[2] * stride_y +
> -			st->ifm.height[2] - (st->ifm.pad_top + st->ifm.pad_bottom);
> +			st->ifm.height[2] * dilation_y -
> +			(st->ifm.pad_top + st->ifm.pad_bottom);
>  		s32 ifm_width = st->ofm.width * stride_x +
> -			st->ifm.width - (st->ifm.pad_left + st->ifm.pad_right);
> +			st->ifm.width * dilation_x -
> +			(st->ifm.pad_left + st->ifm.pad_right);
>  
>  		if (ifm_height < 0 || ifm_width < 0)
>  			return -EINVAL;

[Severity: Critical]
This isn't a regression introduced by this patch, but does the command stream
validation in feat_matrix_size() fail to check the maximum accessed address
within intermediate tiles?

Looking at drivers/accel/ethosu/ethosu_gem.c in feat_matrix_size():

	if (ethosu_is_u65(edev) || storage == 0) {
		for (int xi = 0; xi < 2; xi++) {
			for (int yi = 0; yi < 2; yi++) {
				ret = feat_matrix_check_location(edev, info, st, fm, type,
								 xi ? x : 0,
								 yi ? y : 0, c, ofm,
								 max_len);

The validation loop only checks the memory bounds by testing the origin
of the first tile (xi = 0, yi = 0) and the extremities of the last tile
(xi = 1, yi = 1), but skips checking the inner boundaries of
intermediate tiles.

If a user-space application constructs a command stream
defining a feature map that spans multiple tiles (e.g., storage == 0),
could it set base[0] near the end of the valid GEM buffer and set a
large width0 for the first tile, while setting base[1] to the beginning
of the buffer?

Because the check for x = 0 (Tile 0) uses base[0] + 0, and the check
for x = x_max (Tile 1) uses base[1] + offset, both could pass
validation. However, the maximum offset within Tile 0
(base[0] + width0 * stride_x) is never verified by the logic.

Could this allow the NPU to execute the command stream and access
out-of-bounds memory up to this unchecked boundary?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=13

[PATCH v2 14/19] accel: ethosu: Reject reserved command encodings

[Rob Herring (Arm)]

The command stream contains a 10-bit opcode and a two-bit command
control field. Reject reserved opcode and control encodings in the
switch default case so they cannot be interpreted differently by the
validator and hardware.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_device.h |  3 +++
 drivers/accel/ethosu/ethosu_gem.c    | 12 +++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index 8e23fdbf7f8a..68e2969b6f79 100644
--- a/drivers/accel/ethosu/ethosu_device.h
+++ b/drivers/accel/ethosu/ethosu_device.h
@@ -91,6 +91,9 @@ struct gen_pool;
 #define NPU_KERNEL_DILATION_X		BIT(3)
 #define NPU_KERNEL_DILATION_Y		BIT(4)
 
+#define NPU_CMD_CTRL_CMD1		BIT(14)
+#define NPU_CMD_RESERVED_MASK		(BIT(15) | GENMASK(13, 10))
+
 enum ethosu_cmds {
 	NPU_OP_STOP = 0x0,
 	NPU_OP_IRQ = 0x1,
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 63dd07791f31..d5c3a2c530dc 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -160,9 +160,9 @@ static void cmd_state_init(struct cmd_state *st)
 
 static void cmd_state_set_reg(struct cmd_state *st, u16 cmd)
 {
-	u16 reg = cmd & ~BIT(14);
+	u16 reg = cmd & ~NPU_CMD_CTRL_CMD1;
 
-	if (cmd & BIT(14)) {
+	if (cmd & NPU_CMD_CTRL_CMD1) {
 		if (reg < NPU_CMD1_REGS)
 			__set_bit(reg, st->cmd1);
 	} else if (reg < NPU_CMD0_REGS) {
@@ -172,9 +172,9 @@ static void cmd_state_set_reg(struct cmd_state *st, u16 cmd)
 
 static bool cmd_state_reg_is_set(struct cmd_state *st, u16 cmd)
 {
-	u16 reg = cmd & ~BIT(14);
+	u16 reg = cmd & ~NPU_CMD_CTRL_CMD1;
 
-	if (cmd & BIT(14))
+	if (cmd & NPU_CMD_CTRL_CMD1)
 		return reg < NPU_CMD1_REGS && test_bit(reg, st->cmd1);
 
 	return reg < NPU_CMD0_REGS && test_bit(reg, st->cmd0);
@@ -702,7 +702,7 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		cmd = cmds[0];
 		param = cmds[0] >> 16;
 
-		if (cmd & 0x4000) {
+		if (cmd & NPU_CMD_CTRL_CMD1) {
 			if (get_user(cmds[1], ucmds++))
 				return -EFAULT;
 
@@ -1023,6 +1023,8 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			st.dma.src.len = st.dma.dst.len = addr;
 			break;
 		default:
+			if (cmd & NPU_CMD_RESERVED_MASK)
+				return -EINVAL;
 			break;
 		}
 	}

-- 
2.53.0

[PATCH v2 15/19] accel: ethosu: Validate accumulator input

[Rob Herring (Arm)]

The U85 ACC_FORMAT command can select IFM2 as the accumulator
input. This is used by null-pool operations and can also be used
by convolution. Track this selection and validate the IFM2 feature
map against the OFM extent before submitting the operation.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_device.h |  4 ++++
 drivers/accel/ethosu/ethosu_gem.c    | 42 ++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index 68e2969b6f79..6b9d093d73e6 100644
--- a/drivers/accel/ethosu/ethosu_device.h
+++ b/drivers/accel/ethosu/ethosu_device.h
@@ -126,6 +126,7 @@ enum ethosu_cmds {
 	NPU_SET_KERNEL_WIDTH_M1 = 0x120,
 	NPU_SET_KERNEL_HEIGHT_M1 = 0x121,
 	NPU_SET_KERNEL_STRIDE = 0x122,
+	NPU_SET_ACC_FORMAT = 0x124,
 	NPU_SET_WEIGHT_REGION = 0x128,
 	NPU_SET_SCALE_REGION = 0x129,
 	NPU_SET_DMA0_SRC_REGION = 0x130,
@@ -180,6 +181,9 @@ enum ethosu_cmds {
 	NPU_SET_WEIGHT3_LENGTH = 0x4095,
 };
 
+#define NPU_ACC_FORMAT_INPUT_MASK	GENMASK(5, 4)
+#define NPU_ACC_INPUT_IFM2		2
+
 #define ETHOSU_SRAM_REGION	2	/* Matching Vela compiler */
 
 struct ethosu_perfmon;
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index d5c3a2c530dc..05a3cc04e7d1 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -145,6 +145,7 @@ struct feat_matrix {
 struct cmd_state {
 	DECLARE_BITMAP(cmd0, NPU_CMD0_REGS);
 	DECLARE_BITMAP(cmd1, NPU_CMD1_REGS);
+	bool acc_input_ifm2;
 	struct dma_state dma;
 	struct buffer scale[2];
 	struct buffer weight[4];
@@ -491,6 +492,32 @@ static int feat_matrix_size(struct ethosu_device *edev,
 					  max_len);
 }
 
+static int
+calc_acc_input_size(struct drm_device *ddev,
+		    struct ethosu_validated_cmdstream_info *info,
+		    struct cmd_state *st)
+{
+	struct ethosu_device *edev = to_ethosu_device(ddev);
+	u64 len;
+	int ret;
+
+	if (!ethosu_is_u65(edev) &&
+	    !cmd_state_reg_is_set(st, NPU_SET_ACC_FORMAT))
+		return -EINVAL;
+
+	if (!st->acc_input_ifm2)
+		return 0;
+
+	/* The accumulator has one input value for each OFM element. */
+	ret = feat_matrix_size(edev, info, st, &st->ifm2,
+			       FEAT_MATRIX_IFM2, st->ofm.width,
+			       st->ofm.height[2], st->ofm.depth, false, &len);
+	dev_dbg(ddev->dev, "ACC IFM2:%d:0x%llx-0x%llx\n",
+		st->ifm2.region, st->ifm2.base[0], len);
+
+	return ret;
+}
+
 static int buffer_size(struct ethosu_validated_cmdstream_info *info,
 		       struct cmd_state *st, struct buffer *buf, s8 region,
 		       u16 region_cmd, u16 base_cmd, u16 length_cmd, bool optional)
@@ -612,6 +639,9 @@ static int calc_sizes(struct drm_device *ddev,
 			       true, &len);
 	dev_dbg(ddev->dev, "op %d: OFM:%d:0x%llx-0x%llx\n",
 		op, st->ofm.region, st->ofm.base[0], len);
+	if (ret)
+		return ret;
+	ret = calc_acc_input_size(ddev, info, st);
 	if (ret)
 		return ret;
 	if (!feat_matrix_chained(edev, &st->ofm))
@@ -661,6 +691,9 @@ static int calc_sizes_elemwise(struct drm_device *ddev,
 			       true, &len);
 	dev_dbg(ddev->dev, "op %d: OFM:%d:0x%llx-0x%llx\n",
 		op, st->ofm.region, st->ofm.base[0], len);
+	if (ret)
+		return ret;
+	ret = calc_acc_input_size(ddev, info, st);
 	if (ret)
 		return ret;
 	if (!feat_matrix_chained(edev, &st->ofm))
@@ -799,6 +832,15 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		case NPU_SET_KERNEL_STRIDE:
 			st.ifm.stride_kernel = param;
 			break;
+		case NPU_SET_ACC_FORMAT:
+			if (!ethosu_is_u65(edev)) {
+				u32 acc_input = FIELD_GET(NPU_ACC_FORMAT_INPUT_MASK, param);
+
+				if (acc_input > NPU_ACC_INPUT_IFM2)
+					return -EINVAL;
+				st.acc_input_ifm2 = acc_input == NPU_ACC_INPUT_IFM2;
+			}
+			break;
 		case NPU_SET_IFM_PAD_TOP:
 			st.ifm.pad_top = param & 0x7f;
 			break;

-- 
2.53.0

[PATCH v2 16/19] accel: ethosu: Restrict dynamic IFM2 weights

[Rob Herring (Arm)]

The validator bounds dynamic IFM2 weights as a 1x1 weight matrix, but
did not enforce the corresponding kernel shape. A larger crafted kernel
could make the NPU access beyond the validated IFM2 feature map.

Reject dynamic-weight convolutions whose kernel is not 1x1.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 05a3cc04e7d1..c3dad3f80c34 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -559,6 +559,9 @@ static int calc_sizes(struct drm_device *ddev,
 		    !cmd_state_reg_is_set(st, NPU_SET_IFM_PAD_RIGHT) ||
 		    !cmd_state_reg_is_set(st, NPU_SET_IFM_PAD_BOTTOM))
 			return -EINVAL;
+		/* Dynamic IFM2 weights are only supported for 1x1 convolutions. */
+		if (ifm2 && (st->ifm.width || st->ifm.height[2]))
+			return -EINVAL;
 		u32 stride_y = ((st->ifm.stride_kernel >> 8) & 0x2) +
 			((st->ifm.stride_kernel >> 1) & 0x1) + 1;
 		u32 stride_x = ((st->ifm.stride_kernel >> 5) & 0x2) +

-- 
2.53.0

[PATCH v2 17/19] accel: ethosu: Split U65 and U85 DMA length validation

[Rob Herring (Arm)]

Ethos-U65 and Ethos-U85 have slightly different DMA programming models.
The U65 has skip values added on to the size whereas U85 has signed
stride values. The U65 shares the skip values for source and destination
whereas the U85 has independent settings for source and destination.

The current validation only correctly handles U65 constraints. Split the
shared DMA length calculation into U65 and U85 specific versions.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_gem.c | 110 ++++++++++++++++++++++++++++++--------
 1 file changed, 87 insertions(+), 23 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index c3dad3f80c34..559fbf55f12d 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -225,40 +225,92 @@ static bool dma_params_valid(struct ethosu_device *edev, struct cmd_state *st,
 	return true;
 }
 
-static u64 dma_length(struct ethosu_device *edev,
-		      struct ethosu_validated_cmdstream_info *info,
-		      struct cmd_state *st, struct dma_state *dma_st,
-		      struct dma *dma, u16 region_cmd, u16 addr_cmd)
+static u64 dma_length_finish(struct ethosu_validated_cmdstream_info *info,
+			     const struct dma *dma, u64 len)
+{
+	if (dma->region >= 0) {
+		u64 end;
+
+		if (check_add_overflow(len, dma->offset, &end))
+			return U64_MAX;
+		info->region_size[dma->region] =
+			max(info->region_size[dma->region], end);
+	}
+
+	return len;
+}
+
+static u64 dma_length_u65(struct ethosu_validated_cmdstream_info *info,
+			  struct dma_state *dma_st,
+			  struct dma *dma)
 {
 	s8 mode = dma->mode;
 	u64 len = dma->len;
 
-	if (!dma_params_valid(edev, st, dma_st, dma, region_cmd, addr_cmd))
-		return U64_MAX;
+	if (mode >= 1) {
+		if (check_add_overflow(len, (u64)dma->stride[0], &len) ||
+		    check_mul_overflow(len, (u64)dma_st->size0, &len))
+			return U64_MAX;
+	}
+	if (mode == 2) {
+		if (check_add_overflow(len, (u64)dma->stride[1], &len) ||
+		    check_mul_overflow(len, (u64)dma_st->size1, &len))
+			return U64_MAX;
+	}
+
+	return dma_length_finish(info, dma, len);
+}
+
+static u64 dma_length_u85(struct ethosu_validated_cmdstream_info *info,
+			  struct dma_state *dma_st,
+			  struct dma *dma)
+{
+	s8 mode = dma->mode;
+	s64 min = 0;
+	u64 max = dma->len;
+	s64 stride;
 
 	if (mode >= 1) {
-		if (dma->stride[0] < 0 && (u64)(-dma->stride[0]) > len)
+		if (check_mul_overflow(dma->stride[0],
+				       (s64)dma_st->size0, &stride))
 			return U64_MAX;
-		len += dma->stride[0];
-		if (check_mul_overflow(len, (u64)dma_st->size0, &len))
+		if (stride < 0) {
+			if (check_add_overflow(min, stride, &min))
+				return U64_MAX;
+		} else if (check_add_overflow(max, (u64)stride, &max)) {
 			return U64_MAX;
+		}
 	}
 	if (mode == 2) {
-		if (dma->stride[1] < 0 && (u64)(-dma->stride[1]) > len)
+		if (check_mul_overflow(dma->stride[1],
+				       (s64)dma_st->size1, &stride))
 			return U64_MAX;
-		len += dma->stride[1];
-		if (check_mul_overflow(len, (u64)dma_st->size1, &len))
+		if (stride < 0) {
+			if (check_add_overflow(min, stride, &min))
+				return U64_MAX;
+		} else if (check_add_overflow(max, (u64)stride, &max)) {
 			return U64_MAX;
+		}
 	}
-	if (dma->region >= 0) {
-		u64 end;
 
-		if (check_add_overflow(len, dma->offset, &end))
-			return U64_MAX;
-		info->region_size[dma->region] = max(info->region_size[dma->region], end);
-	}
+	if (min < 0 && -(u64)min > dma->offset)
+		return U64_MAX;
 
-	return len;
+	return dma_length_finish(info, dma, max);
+}
+
+static u64 dma_length(struct ethosu_device *edev,
+		      struct ethosu_validated_cmdstream_info *info,
+		      struct cmd_state *st, struct dma_state *dma_st,
+		      struct dma *dma, u16 region_cmd, u16 addr_cmd)
+{
+	if (!dma_params_valid(edev, st, dma_st, dma, region_cmd, addr_cmd))
+		return U64_MAX;
+
+	if (ethosu_is_u65(edev))
+		return dma_length_u65(info, dma_st, dma);
+
+	return dma_length_u85(info, dma_st, dma);
 }
 
 static bool feat_matrix_chained(struct ethosu_device *edev, struct feat_matrix *fm)
@@ -1047,16 +1099,28 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			st.dma.size1 = param;
 			break;
 		case NPU_SET_DMA0_SRC_STRIDE0:
-			st.dma.src.stride[0] = ((s64)addr << 24) >> 24;
+			if (ethosu_is_u65(edev))
+				st.dma.dst.stride[0] = addr;
+			else
+				st.dma.src.stride[0] = sign_extend64(addr, 39);
 			break;
 		case NPU_SET_DMA0_SRC_STRIDE1:
-			st.dma.src.stride[1] = ((s64)addr << 24) >> 24;
+			if (ethosu_is_u65(edev))
+				st.dma.dst.stride[1] = addr;
+			else
+				st.dma.src.stride[1] = sign_extend64(addr, 39);
 			break;
 		case NPU_SET_DMA0_DST_STRIDE0:
-			st.dma.dst.stride[0] = ((s64)addr << 24) >> 24;
+			if (!ethosu_is_u65(edev))
+				st.dma.dst.stride[0] = sign_extend64(addr, 39);
+			else
+				return -EINVAL;
 			break;
 		case NPU_SET_DMA0_DST_STRIDE1:
-			st.dma.dst.stride[1] = ((s64)addr << 24) >> 24;
+			if (!ethosu_is_u65(edev))
+				st.dma.dst.stride[1] = sign_extend64(addr, 39);
+			else
+				return -EINVAL;
 			break;
 		case NPU_SET_DMA0_SRC:
 			st.dma.src.offset = addr;

-- 
2.53.0

Re: [PATCH v2 17/19] accel: ethosu: Split U65 and U85 DMA length validation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] For Ethos-U65, the shared DMA skip values are parsed and assigned only to the destination stride, leaving the source stride uninitialized (zero). This causes the driver to severely underestimate the required source buffer size, allowing userspace to bypass bounds validation and trigger out-of-bounds memory accesses.
--

commit f80309886975bd6af6358046bb0e792268687173
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Split U65 and U85 DMA length validation

This commit splits the DMA length calculation in the Ethos-U driver into
separate versions for U65 and U85 to handle their slightly different
programming models. The U65 uses skip values added to the size, while
the U85 uses independent signed stride values.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index c3dad3f80c34b..559fbf55f12d5 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -225,40 +225,92 @@ static bool dma_params_valid(struct ethosu_device *edev, struct cmd_state *st,
[ ... ]
> +static u64 dma_length_u65(struct ethosu_validated_cmdstream_info *info,
> +			  struct dma_state *dma_st,
> +			  struct dma *dma)
>  {
>  	s8 mode = dma->mode;
>  	u64 len = dma->len;
>  
> -	if (!dma_params_valid(edev, st, dma_st, dma, region_cmd, addr_cmd))
> -		return U64_MAX;
> +	if (mode >= 1) {
> +		if (check_add_overflow(len, (u64)dma->stride[0], &len) ||
> +		    check_mul_overflow(len, (u64)dma_st->size0, &len))
> +			return U64_MAX;
> +	}

[Severity: Critical]
Does this code underestimate the bounding memory range for the source buffer?

When evaluating the source DMA length for U65, dma->stride[0] will be 0
because ethosu_gem_cmdstream_copy_and_validate() only assigns the skip
value to the destination stride. This bypasses the skip value expansion and
could allow userspace to bypass bounds validation, potentially triggering
out-of-bounds memory accesses.

[ ... ]
> @@ -1047,16 +1099,28 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			st.dma.size1 = param;
>  			break;
>  		case NPU_SET_DMA0_SRC_STRIDE0:
> -			st.dma.src.stride[0] = ((s64)addr << 24) >> 24;
> +			if (ethosu_is_u65(edev))
> +				st.dma.dst.stride[0] = addr;

[Severity: Critical]
Is it intentional that the user-provided shared stride is only assigned to
the destination stride here?

This leaves the source stride, st.dma.src.stride[0], uninitialized (0)
for U65, leading to the regression in dma_length_u65() where length bounds
are underestimated. Should this also be assigned to st.dma.src.stride[0]
for U65?

> +			else
> +				st.dma.src.stride[0] = sign_extend64(addr, 39);
>  			break;
>  		case NPU_SET_DMA0_SRC_STRIDE1:
> -			st.dma.src.stride[1] = ((s64)addr << 24) >> 24;
> +			if (ethosu_is_u65(edev))
> +				st.dma.dst.stride[1] = addr;

[Severity: Critical]
Does this same regression occur for stride 1?

It appears this also leaves st.dma.src.stride[1] uninitialized for U65.

> +			else
> +				st.dma.src.stride[1] = sign_extend64(addr, 39);
>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=17

[PATCH v2 18/19] accel: ethosu: Validate OFM transpose

[Rob Herring (Arm)]

U85 OFM dimensions are specified before transposition, while
tile bases and strides address the transposed feature map. Permute
the output endpoint before validating its tile and stride accesses.

Allow the defined U85 transpose encodings and reject the two
reserved encodings.

Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - no change
---
 drivers/accel/ethosu/ethosu_device.h |  1 +
 drivers/accel/ethosu/ethosu_gem.c    | 58 ++++++++++++++++++++++++++++++++++--
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index 6b9d093d73e6..3f1fa0a36bd9 100644
--- a/drivers/accel/ethosu/ethosu_device.h
+++ b/drivers/accel/ethosu/ethosu_device.h
@@ -90,6 +90,7 @@ struct gen_pool;
 #define NPU_OP_CONV_WEIGHTS_IFM2	BIT(0)
 #define NPU_KERNEL_DILATION_X		BIT(3)
 #define NPU_KERNEL_DILATION_Y		BIT(4)
+#define NPU_OFM_TRANSPOSE_MASK		GENMASK(13, 11)
 
 #define NPU_CMD_CTRL_CMD1		BIT(14)
 #define NPU_CMD_RESERVED_MASK		(BIT(15) | GENMASK(13, 10))
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 559fbf55f12d..8114447891b2 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -397,6 +397,52 @@ static int feat_matrix_validate(struct ethosu_device *edev,
 
 	return 0;
 }
+
+static int feat_matrix_permute(struct ethosu_device *edev,
+			       struct feat_matrix *fm, u32 *x, u32 *y,
+			       u32 *c, bool ofm)
+{
+	u32 width = *x;
+	u32 height = *y;
+	u32 depth = *c;
+	u32 transpose;
+
+	if (ethosu_is_u65(edev) || !ofm)
+		return 0;
+
+	transpose = FIELD_GET(NPU_OFM_TRANSPOSE_MASK, fm->precision);
+
+	switch (transpose) {
+	case 0: /* HWC */
+		break;
+	case 1: /* WHC */
+		*x = height;
+		*y = width;
+		break;
+	case 2: /* HCW */
+		*x = depth;
+		*c = width;
+		break;
+	case 3: /* WCH */
+		*x = depth;
+		*y = width;
+		*c = height;
+		break;
+	case 6: /* CHW */
+		*x = height;
+		*y = depth;
+		*c = width;
+		break;
+	case 7: /* CWH */
+		*y = depth;
+		*c = height;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
 static u64 feat_matrix_length(struct ethosu_device *edev,
 			      struct ethosu_validated_cmdstream_info *info,
 			      struct cmd_state *st, struct feat_matrix *fm,
@@ -502,6 +548,9 @@ static int feat_matrix_size(struct ethosu_device *edev,
 	int ret;
 
 	*max_len = 0;
+	ret = feat_matrix_permute(edev, fm, &x, &y, &c, ofm);
+	if (ret)
+		return ret;
 
 	if (ethosu_is_u65(edev) || storage == 0) {
 		for (int xi = 0; xi < 2; xi++) {
@@ -959,8 +1008,13 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		case NPU_SET_OFM_PRECISION:
 			if (((param >> 6) & 0x3) > 1)
 				return -EINVAL;
-			if (!ethosu_is_u65(edev) && (param & GENMASK(13, 11)))
-				return -EINVAL;
+			if (!ethosu_is_u65(edev)) {
+				switch (FIELD_GET(NPU_OFM_TRANSPOSE_MASK, param)) {
+				case 4:
+				case 5:
+					return -EINVAL;
+				}
+			}
 			st.ofm.precision = param;
 			break;
 		case NPU_SET_OFM_REGION:

-- 
2.53.0

Re: [PATCH v2 18/19] accel: ethosu: Validate OFM transpose

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] feat_matrix_size completely fails to evaluate intermediate tile boundaries, allowing arbitrary out-of-bounds DMA access
- [High] feat_matrix_length assumes monotonic address growth for nhcwb16 format, underestimating buffer bounds by up to 60 bytes
--

commit 5cfa238ea5bc9f37e1c0be661f6a282b416aafb4
Author: Rob Herring (Arm) <robh@kernel.org>
Subject: accel: ethosu: Validate OFM transpose

U85 OFM dimensions are specified before transposition, while
tile bases and strides address the transposed feature map. Permute
the output endpoint before validating its tile and stride accesses.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 559fbf55f12d5..8114447891b25 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -397,6 +397,52 @@ static int feat_matrix_validate(struct ethosu_device *edev,
>  
>  	return 0;
>  }
> +
> +static int feat_matrix_permute(struct ethosu_device *edev,
> +			       struct feat_matrix *fm, u32 *x, u32 *y,
> +			       u32 *c, bool ofm)
> +{

[ ... ]

> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
>  static u64 feat_matrix_length(struct ethosu_device *edev,
>  			      struct ethosu_validated_cmdstream_info *info,
>  			      struct cmd_state *st, struct feat_matrix *fm,

[Severity: High]
This isn't a bug introduced by this patch, but feat_matrix_length() assumes
monotonic address growth for the nhcwb16 format, underestimating buffer
bounds by up to 60 bytes.

For the nhcwb16 format, the address offset calculation:

    check_mul_overflow(c / 16, (u64)fm->stride_c, &offset) ||
    ...
    check_mul_overflow(16 * x + (c & 0xf), element_size, &offset)

is not monotonic with respect to c if stride_c is smaller than
15 * element_size (which userspace fully controls).

Because the validator only checks the offset at the maximum depth,
malicious userspace can set stride_c to 0 and c_max to 16. It then
evaluates only at c=16, which yields 16 * x * element_size.

However, the hardware will also access c=15, resulting in a larger offset
of 16 * x * element_size + 15 * element_size. This allows out-of-bounds DMA
access into adjacent kernel memory.

Can this logic be updated to evaluate the maximum offset accurately by
accounting for intermediate values of c?

> @@ -502,6 +548,9 @@ static int feat_matrix_size(struct ethosu_device *edev,
>  	int ret;
>  
>  	*max_len = 0;
> +	ret = feat_matrix_permute(edev, fm, &x, &y, &c, ofm);
> +	if (ret)
> +		return ret;
>  
>  	if (ethosu_is_u65(edev) || storage == 0) {
>  		for (int xi = 0; xi < 2; xi++) {
>  			for (int yi = 0; yi < 2; yi++) {
>  				ret = feat_matrix_check_location(edev, info, st, fm, type,
>  								 xi ? x : 0,
>  								 yi ? y : 0, c, ofm,
>  								 max_len);

[Severity: Critical]
This is a pre-existing issue, but feat_matrix_size() completely fails to
evaluate intermediate tile boundaries, allowing arbitrary out-of-bounds DMA
access.

For tiled matrices (storage == 0 or storage == 1), each tile has an
independent base address. The maximum memory offset within tile 0 occurs
at x = min(x, fm->width0) and y = min(y, fm->height[0]).

The validator never evaluates these intermediate maxima:

For storage == 0, it only evaluates x at 0 and max_x in the loop:

    for (int xi = 0; xi < 2; xi++) {
        for (int yi = 0; yi < 2; yi++) {
            ret = feat_matrix_check_location(..., xi ? x : 0, yi ? y : 0, ...);

This misses the local maximum x offset inside tile 0 (fm->width0).

For storage == 1, it evaluates y at 0 and fm->height[1]:

    if (fm->height[0] < fm->height[1] && fm->height[1] <= y) {
        ret = feat_matrix_check_location(..., x, fm->height[1], ...);

This completely skips the end of tile 0 (fm->height[0]), leaving the max
offset for tile 0 unvalidated.

Userspace can bypass validation by setting base[0] near the end of the
buffer while base[0] + offset(x, fm->height[0]) massively overflows it.

Could this be corrected to evaluate the local maxima for tile boundaries?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=18

[PATCH v2 19/19] accel: ethosu: Validate resize operations

[Rob Herring (Arm)]

Resize input coordinates are controlled by the scale, offset, and step
registers. Require those values to be explicitly programmed, validate
the scale and step relationships, and use a conservative coordinate
bound when validating the input feature map.

This prevents retained or malformed resize state from accessing beyond
the validated input feature-map buffer.

Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
 - new patch
---
 drivers/accel/ethosu/ethosu_device.h |   8 ++
 drivers/accel/ethosu/ethosu_gem.c    | 159 ++++++++++++++++++++++++++++++++++-
 2 files changed, 166 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index 3f1fa0a36bd9..2974173bbc40 100644
--- a/drivers/accel/ethosu/ethosu_device.h
+++ b/drivers/accel/ethosu/ethosu_device.h
@@ -120,6 +120,8 @@ enum ethosu_cmds {
 	NPU_SET_OFM_HEIGHT_M1 = 0x112,
 	NPU_SET_OFM_DEPTH_M1 = 0x113,
 	NPU_SET_OFM_PRECISION = 0x114,
+	NPU_SET_OFM_BLK_WIDTH_M1 = 0x115,
+	NPU_SET_OFM_BLK_HEIGHT_M1 = 0x116,
 	NPU_SET_OFM_WIDTH0_M1 = 0x11a,
 	NPU_SET_OFM_HEIGHT0_M1 = 0x11b,
 	NPU_SET_OFM_HEIGHT1_M1 = 0x11c,
@@ -130,6 +132,10 @@ enum ethosu_cmds {
 	NPU_SET_ACC_FORMAT = 0x124,
 	NPU_SET_WEIGHT_REGION = 0x128,
 	NPU_SET_SCALE_REGION = 0x129,
+	NPU_SET_RESIZE_X_SCALE_N_M1 = 0x12a,
+	NPU_SET_RESIZE_Y_SCALE_N_M1 = 0x12b,
+	NPU_SET_RESIZE_X_OFFSET = 0x12c,
+	NPU_SET_RESIZE_Y_OFFSET = 0x12d,
 	NPU_SET_DMA0_SRC_REGION = 0x130,
 	NPU_SET_DMA0_DST_REGION = 0x131,
 	NPU_SET_DMA0_SIZE0 = 0x132,
@@ -180,6 +186,8 @@ enum ethosu_cmds {
 	NPU_SET_WEIGHT2_LENGTH = 0x4093,
 	NPU_SET_WEIGHT3_BASE = 0x4094,
 	NPU_SET_WEIGHT3_LENGTH = 0x4095,
+	NPU_SET_RESIZE_X = 0x4096,
+	NPU_SET_RESIZE_Y = 0x4097,
 };
 
 #define NPU_ACC_FORMAT_INPUT_MASK	GENMASK(5, 4)
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 8114447891b2..67bee9612934 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -139,6 +139,15 @@ struct feat_matrix {
 	u8 pad_right;
 };
 
+struct resize_axis {
+	u16 scale_n;
+	s16 offset;
+	u16 one_step_int;
+	u16 one_step_mod;
+	u16 blk_step_int;
+	u16 blk_step_mod;
+};
+
 #define NPU_CMD0_REGS	0x200
 #define NPU_CMD1_REGS	0x100
 
@@ -152,6 +161,9 @@ struct cmd_state {
 	struct feat_matrix ofm;
 	struct feat_matrix ifm;
 	struct feat_matrix ifm2;
+	u16 ofm_blk_width;
+	u16 ofm_blk_height;
+	struct resize_axis resize[2];
 };
 
 static void cmd_state_init(struct cmd_state *st)
@@ -619,6 +631,96 @@ calc_acc_input_size(struct drm_device *ddev,
 	return ret;
 }
 
+static int resize_axis_size(struct cmd_state *st, int axis, u16 ofm_size,
+			    u16 ofm_blk_size, u32 *size)
+{
+	struct resize_axis *resize = &st->resize[axis];
+	u64 one_step, blk_step, coord;
+
+	if (resize->offset < -(s16)resize->scale_n ||
+	    resize->offset >= resize->scale_n ||
+	    resize->one_step_mod >= resize->scale_n ||
+	    resize->blk_step_mod >= resize->scale_n)
+		return -EINVAL;
+
+	one_step = resize->one_step_int * resize->scale_n +
+		resize->one_step_mod;
+	blk_step = resize->blk_step_int * resize->scale_n +
+		resize->blk_step_mod;
+	if (check_mul_overflow((u64)ofm_blk_size, one_step, &coord) ||
+	    blk_step != coord)
+		return -EINVAL;
+
+	if (check_mul_overflow((u64)ofm_size, one_step, &coord) ||
+	    check_add_overflow(coord, (u64)resize->scale_n - 1, &coord))
+		return -EINVAL;
+
+	coord = div_u64(coord, resize->scale_n);
+	if (coord >= U32_MAX)
+		return -EINVAL;
+
+	*size = coord + 1;
+	return 0;
+}
+
+
+static int calc_sizes_resize(struct drm_device *ddev,
+			     struct ethosu_validated_cmdstream_info *info,
+			     struct cmd_state *st)
+{
+	struct ethosu_device *edev = to_ethosu_device(ddev);
+	u32 ifm_width, ifm_height;
+	u64 len;
+	int ret;
+
+	if (!cmd_state_reg_is_set(st, NPU_SET_KERNEL_WIDTH_M1) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_KERNEL_HEIGHT_M1) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_OFM_BLK_WIDTH_M1) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_OFM_BLK_HEIGHT_M1) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_RESIZE_X_SCALE_N_M1) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_RESIZE_Y_SCALE_N_M1) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_RESIZE_X_OFFSET) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_RESIZE_Y_OFFSET) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_RESIZE_X) ||
+	    !cmd_state_reg_is_set(st, NPU_SET_RESIZE_Y))
+		return -EINVAL;
+
+	ret = resize_axis_size(st, 0, st->ofm.width, st->ofm_blk_width,
+			       &ifm_width);
+	if (ret)
+		return ret;
+	ret = resize_axis_size(st, 1, st->ofm.height[2], st->ofm_blk_height,
+			       &ifm_height);
+	if (ret)
+		return ret;
+
+	ret = feat_matrix_size(edev, info, st, &st->ifm, FEAT_MATRIX_IFM,
+			       max(ifm_width, (u32)st->ifm.width),
+			       max(ifm_height, (u32)st->ifm.height[2]), st->ifm.depth,
+			       false, &len);
+	dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n", NPU_OP_RESIZE,
+		st->ifm.region, st->ifm.base[0], len);
+	if (ret)
+		return ret;
+
+	ret = feat_matrix_size(edev, info, st, &st->ofm, FEAT_MATRIX_OFM,
+			       st->ofm.width, st->ofm.height[2], st->ofm.depth,
+			       true, &len);
+	dev_dbg(ddev->dev, "op %d: OFM:%d:0x%llx-0x%llx\n", NPU_OP_RESIZE,
+		st->ofm.region, st->ofm.base[0], len);
+	if (ret)
+		return ret;
+
+	ret = calc_acc_input_size(ddev, info, st);
+	if (ret)
+		return ret;
+
+	if (!feat_matrix_chained(edev, &st->ofm))
+		info->output_region[st->ofm.region] = true;
+
+	return 0;
+}
+
 static int buffer_size(struct ethosu_validated_cmdstream_info *info,
 		       struct cmd_state *st, struct buffer *buf, s8 region,
 		       u16 region_cmd, u16 base_cmd, u16 length_cmd, bool optional)
@@ -926,7 +1028,12 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 				return ret;
 			break;
 		case NPU_OP_RESIZE: // U85 only
-			return -EINVAL;
+			if (ethosu_is_u65(edev) || param > 2)
+				return -EINVAL;
+			ret = calc_sizes_resize(ddev, info, &st);
+			if (ret)
+				return ret;
+			break;
 		case NPU_SET_KERNEL_WIDTH_M1:
 			st.ifm.width = param;
 			break;
@@ -1017,6 +1124,12 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 			}
 			st.ofm.precision = param;
 			break;
+		case NPU_SET_OFM_BLK_WIDTH_M1:
+			st.ofm_blk_width = param & 0x7f;
+			break;
+		case NPU_SET_OFM_BLK_HEIGHT_M1:
+			st.ofm_blk_height = param & 0x7f;
+			break;
 		case NPU_SET_OFM_REGION:
 			st.ofm.region = param & 0x7;
 			break;
@@ -1087,6 +1200,34 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		case NPU_SET_SCALE_REGION:
 			st.scale[0].region = param & 0x7;
 			break;
+		case NPU_SET_RESIZE_X_SCALE_N_M1:
+			if (ethosu_is_u65(edev))
+				break;
+			if (param & GENMASK(15, 11))
+				return -EINVAL;
+			st.resize[0].scale_n = param + 1;
+			break;
+		case NPU_SET_RESIZE_Y_SCALE_N_M1:
+			if (ethosu_is_u65(edev))
+				break;
+			if (param & GENMASK(15, 11))
+				return -EINVAL;
+			st.resize[1].scale_n = param + 1;
+			break;
+		case NPU_SET_RESIZE_X_OFFSET:
+			if (ethosu_is_u65(edev))
+				break;
+			if (param & GENMASK(15, 12))
+				return -EINVAL;
+			st.resize[0].offset = sign_extend32(param, 11);
+			break;
+		case NPU_SET_RESIZE_Y_OFFSET:
+			if (ethosu_is_u65(edev))
+				break;
+			if (param & GENMASK(15, 12))
+				return -EINVAL;
+			st.resize[1].offset = sign_extend32(param, 11);
+			break;
 		case NPU_SET_WEIGHT_BASE:
 			st.weight[0].base = addr;
 			break;
@@ -1123,6 +1264,22 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		case NPU_SET_WEIGHT3_LENGTH:
 			st.weight[3].length = cmds[1];
 			break;
+		case NPU_SET_RESIZE_X:
+		case NPU_SET_RESIZE_Y:
+			if (ethosu_is_u65(edev))
+				break;
+			if ((cmds[0] & BIT(31)) ||
+			    (cmds[1] & (GENMASK(31, 27) | GENMASK(15, 11))))
+				return -EINVAL;
+			st.resize[cmd - NPU_SET_RESIZE_X].one_step_int =
+				FIELD_GET(GENMASK(19, 16), cmds[0]);
+			st.resize[cmd - NPU_SET_RESIZE_X].blk_step_int =
+				FIELD_GET(GENMASK(30, 20), cmds[0]);
+			st.resize[cmd - NPU_SET_RESIZE_X].one_step_mod =
+				FIELD_GET(GENMASK(10, 0), cmds[1]);
+			st.resize[cmd - NPU_SET_RESIZE_X].blk_step_mod =
+				FIELD_GET(GENMASK(26, 16), cmds[1]);
+			break;
 
 		case NPU_SET_DMA0_SRC_REGION:
 			if (param & NPU_DMA_REGION_INDEX_MODE)

-- 
2.53.0

Re: [PATCH v2 19/19] accel: ethosu: Validate resize operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Flawed buffer bounds calculation for RESIZE operations ignores the kernel footprint, allowing the NPU to read out of bounds.
--

commit ea6b05d137a756ad8605387afc8b28c1b9b10e2e
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Validate resize operations

This commit adds validation for resize operations by explicitly requiring and
verifying the scale, offset, and step registers. It also computes a
conservative coordinate bound when validating the input feature map.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 8114447891b25..67bee96129341 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -619,6 +631,96 @@ calc_acc_input_size(struct drm_device *ddev,
[ ... ]
> +static int calc_sizes_resize(struct drm_device *ddev,
> +			     struct ethosu_validated_cmdstream_info *info,
> +			     struct cmd_state *st)
> +{
> +	struct ethosu_device *edev = to_ethosu_device(ddev);
> +	u32 ifm_width, ifm_height;
> +	u64 len;
> +	int ret;
[ ... ]
> +	ret = resize_axis_size(st, 0, st->ofm.width, st->ofm_blk_width,
> +			       &ifm_width);
> +	if (ret)
> +		return ret;
> +	ret = resize_axis_size(st, 1, st->ofm.height[2], st->ofm_blk_height,
> +			       &ifm_height);
> +	if (ret)
> +		return ret;
> +
> +	ret = feat_matrix_size(edev, info, st, &st->ifm, FEAT_MATRIX_IFM,
> +			       max(ifm_width, (u32)st->ifm.width),
> +			       max(ifm_height, (u32)st->ifm.height[2]), st->ifm.depth,
                               ^^^^

[Severity: High]
Does this calculation underestimate the required buffer size by ignoring the
kernel footprint?

In calc_sizes_resize(), the starting input feature map (IFM) coordinate
for the last output pixel is calculated by resize_axis_size() as ifm_width.
Because the NPU applies a kernel of size st->ifm.width + 1 at this
coordinate, the true maximum IFM coordinate accessed should be
ifm_width + st->ifm.width.

By combining the starting coordinate and kernel width with max() instead of
addition, the bound checks max(ifm_width, st->ifm.width). This allows a
malicious command stream with a large kernel width to read memory past the
validated input feature map buffer.

Could we use addition here to correctly account for the kernel footprint?

> +			       false, &len);
> +	dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n", NPU_OP_RESIZE,
> +		st->ifm.region, st->ifm.base[0], len);
> +	if (ret)
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=19