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

[PATCH 00/11] 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 series adds rejecting OFM tranpose in cmd stream, but then the 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>
---
Rob Herring (Arm) (11):
      accel: ethosu: Fix ethosu_job_open() return value
      accel: ethosu: Drop IRQF_SHARED flag
      accel: ethosu: Ensure cmd stream ends with a stop op
      accel: ethosu: Ensure SRAM size is 0 on mapping failure
      accel: ethosu: Ensure SRAM region size matches job
      accel: ethosu: Fix probe error cleanup
      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: Validate OFM transpose

 drivers/accel/ethosu/ethosu_device.h |   6 +
 drivers/accel/ethosu/ethosu_drv.c    |  17 ++-
 drivers/accel/ethosu/ethosu_gem.c    | 240 ++++++++++++++++++++++++++++++-----
 drivers/accel/ethosu/ethosu_job.c    |  22 ++--
 4 files changed, 233 insertions(+), 52 deletions(-)
---
base-commit: f7e3f4d9f425cf4c5577cd84a096e6e618488083
change-id: 20260827-ethosu-fixes-ee58386e9000

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

[PATCH 01/11] accel: ethosu: Fix ethosu_job_open() return value

[Rob Herring (Arm)]

A WARN_ON() returns a 0 or 1, not the original negative errno. Just drop
the WARN_ON() as the FD open will pass the return code to userspace and
there's only one possible source of the error (drm_sched_entity_init()).

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

diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index 6a038c0384cc..99c8812691e5 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -368,12 +368,10 @@ int ethosu_job_open(struct ethosu_file_priv *ethosu_priv)
 {
 	struct ethosu_device *dev = ethosu_priv->edev;
 	struct drm_gpu_scheduler *sched = &dev->sched;
-	int ret;
 
-	ret = drm_sched_entity_init(&ethosu_priv->sched_entity,
-				    DRM_SCHED_PRIORITY_NORMAL,
-				    &sched, 1, NULL);
-	return WARN_ON(ret);
+	return drm_sched_entity_init(&ethosu_priv->sched_entity,
+				     DRM_SCHED_PRIORITY_NORMAL,
+				     &sched, 1, NULL);
 }
 
 void ethosu_job_close(struct ethosu_file_priv *ethosu_priv)

-- 
2.53.0

Re: [PATCH 01/11] accel: ethosu: Fix ethosu_job_open() return value

[sashiko-bot]

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

Pre-existing issues:
- [High] The dynamically allocated `done_fence` is leaked on error paths during job submission in `ethosu_ioctl_submit_job`.
- [High] NULL pointer dereference in `ethosu_ioctl_submit_job` error path due to premature cleanup of an uninitialized scheduler job.
--

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

    accel: ethosu: Fix ethosu_job_open() return value
    
    A WARN_ON() returns a 0 or 1, not the original negative errno. Just drop
    the WARN_ON() as the FD open will pass the return code to userspace and
    there's only one possible source of the error (drm_sched_entity_init()).

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index 6a038c0384ccd..99c8812691e55 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c

[ ... ]

While reviewing this file, I noticed a couple of pre-existing issues in the
job submission error paths.

If the ejob->done_fence allocation fails, it bypasses job initialization but
still attempts cleanup:

drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job() {
    ...
	ejob->done_fence = kzalloc_obj(*ejob->done_fence);
	if (!ejob->done_fence) {
		ret = -ENOMEM;
		goto out_cleanup_job;
	}

	ret = drm_sched_job_init(&ejob->base,
				 &file_priv->sched_entity,
				 1, NULL, file->client_id);
    ...
out_cleanup_job:
	if (ret)
		drm_sched_job_cleanup(&ejob->base);
out_put_job:
	ethosu_job_err_cleanup(ejob);

	return ret;
}

[Severity: High]
This isn't a bug introduced by this patch, but does calling
drm_sched_job_cleanup(&ejob->base) cause a NULL pointer dereference here?

Since drm_sched_job_init() hasn't been called yet, ejob->base.s_fence is
completely zero-initialized. drm_sched_job_cleanup() unconditionally
dereferences job->s_fence when checking the reference count, which would
trigger a panic on this memory allocation failure path.

Additionally, looking at the cleanup function called further down that same
error path:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_err_cleanup() {
	unsigned int i;

	ethosu_perfmon_put(job->perfmon);

	for (i = 0; i < job->region_cnt; i++)
		drm_gem_object_put(job->region_bo[i]);

	drm_gem_object_put(job->cmd_bo);

	kfree(job);
}

[Severity: High]
This is a pre-existing issue, but will this leak ejob->done_fence?

If ethosu_ioctl_submit_job() fails later in the function (for example, if
drm_gem_object_lookup() returns an error), it jumps to out_cleanup_job and
eventually calls ethosu_job_err_cleanup(). The cleanup function frees the
job container and BOs, but appears to miss freeing the dynamically allocated
ejob->done_fence pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=1

Re: [PATCH 01/11] accel: ethosu: Fix ethosu_job_open() return value

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:00PM -0500, Rob Herring (Arm) wrote:
> A WARN_ON() returns a 0 or 1, not the original negative errno. Just drop
> the WARN_ON() as the FD open will pass the return code to userspace and
> there's only one possible source of the error (drm_sched_entity_init()).
>
> Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_job.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index 6a038c0384cc..99c8812691e5 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -368,12 +368,10 @@ int ethosu_job_open(struct ethosu_file_priv *ethosu_priv)
>  {
>  	struct ethosu_device *dev = ethosu_priv->edev;
>  	struct drm_gpu_scheduler *sched = &dev->sched;
> -	int ret;
>
> -	ret = drm_sched_entity_init(&ethosu_priv->sched_entity,
> -				    DRM_SCHED_PRIORITY_NORMAL,
> -				    &sched, 1, NULL);
> -	return WARN_ON(ret);
> +	return drm_sched_entity_init(&ethosu_priv->sched_entity,
> +				     DRM_SCHED_PRIORITY_NORMAL,
> +				     &sched, 1, NULL);
>  }
>
>  void ethosu_job_close(struct ethosu_file_priv *ethosu_priv)
>
> --
> 2.53.0
>

[PATCH 02/11] accel: ethosu: Drop IRQF_SHARED flag

[Rob Herring (Arm)]

The IRQF_SHARED flag doesn't work with runtime-pm as the IRQ handler
could run without resuming the device. This could also be fixed with
runtime-pm calls in the IRQ handler, but there is no known need for a
shared IRQ.

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

diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index 99c8812691e5..c33f2877b385 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -337,7 +337,7 @@ int ethosu_job_init(struct ethosu_device *edev)
 	ret = devm_request_threaded_irq(dev, edev->irq,
 					ethosu_job_irq_handler,
 					ethosu_job_irq_handler_thread,
-					IRQF_SHARED, KBUILD_MODNAME,
+					0, KBUILD_MODNAME,
 					edev);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");

-- 
2.53.0

Re: [PATCH 02/11] accel: ethosu: Drop IRQF_SHARED flag

[sashiko-bot]

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

Pre-existing issues:
- [High] Crash due to unconditional register access in IRQ handler while clocks are disabled
- [High] Null pointer dereference due to calling drm_sched_job_cleanup() on an uninitialized structure
- [High] Permanent memory leak of done_fence on job submission error paths
- [High] Use-After-Free of DRM scheduler due to missing drm_dev_enter() protection
- [High] Asymmetric teardown allows device interrupts to trigger a Use-After-Free on the destroyed DRM scheduler
- [High] Race condition leading to Use-After-Free of job and done_fence between IRQ handler and timeout worker
- [High] Missing read-back / flush after clearing the interrupt via writel_relaxed causes an IRQ storm
--

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

accel: ethosu: Drop IRQF_SHARED flag

The IRQF_SHARED flag doesn't work with runtime-pm as the IRQ handler
could run without resuming the device. This could also be fixed with
runtime-pm calls in the IRQ handler, but there is no known need for a
shared IRQ.

Cc: stable@vger.kernel.org

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index 99c8812691e55..c33f2877b3850 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -337,7 +337,7 @@ int ethosu_job_init(struct ethosu_device *edev)
>  	ret = devm_request_threaded_irq(dev, edev->irq,
>  					ethosu_job_irq_handler,
>  					ethosu_job_irq_handler_thread,
> -					IRQF_SHARED, KBUILD_MODNAME,
> +					0, KBUILD_MODNAME,
>  					edev);

[Severity: High]
This is a pre-existing issue, but I see the commit message says "This could
also be fixed with runtime-pm calls in the IRQ handler". Does this change
completely address the issue with clocks being disabled when the IRQ handler
runs?

For instance, in ethosu_probe(), the hardware IRQ is requested via
ethosu_job_init() before the device is resumed and clocks are enabled in
ethosu_init():

drivers/accel/ethosu/ethosu_drv.c:ethosu_probe() {
    ...
    ret = ethosu_job_init(ethosudev);
    if (ret)
        return ret;

    ret = ethosu_init(ethosudev);
    ...
}

If a pending interrupt fires immediately, ethosu_job_irq_handler() will access
registers while clocks are gated, which can trigger a bus fault:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_irq_handler() {
    ...
    u32 status = readl_relaxed(dev->regs + NPU_REG_STATUS);
    ...
}

Similarly, in ethosu_job_timedout(), clocks are forcibly disabled without
synchronizing or disabling the IRQ:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_timedout() {
    ...
    pm_runtime_force_suspend(dev->base.dev);
    ...
}

Can a concurrent or delayed interrupt still crash the system here?


[Severity: High]
This is a pre-existing issue, but can returning IRQ_WAKE_THREAD immediately
after a relaxed write lead to an IRQ storm?

drivers/accel/ethosu/ethosu_job.c:ethosu_job_irq_handler() {
    ...
    writel_relaxed(CMD_CLEAR_IRQ, dev->regs + NPU_REG_CMD);
    return IRQ_WAKE_THREAD;
}

Since the relaxed write is posted asynchronously and the interrupt is not
requested with IRQF_ONESHOT, the generic IRQ core will unmask the interrupt
line immediately.

If the write hasn't reached the hardware to deassert the line, won't the CPU
immediately take the interrupt again? Would adding a dummy register read-back
ensure the interrupt is cleared before returning?


[Severity: High]
This is a pre-existing issue, but is it safe to bind the IRQ lifetime to
devres while manually destroying the DRM scheduler in ethosu_remove()?

drivers/accel/ethosu/ethosu_drv.c:ethosu_remove() {
    ...
    ethosu_job_fini(ethosudev);
    ...
}

Since the IRQ remains active until the devres teardown phase, if an interrupt
fires after ethosu_job_fini(), ethosu_job_handle_irq() could call
drm_sched_fault() on the already destroyed scheduler:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_handle_irq() {
    ...
    drm_sched_fault(&dev->sched);
    ...
}

Could this lead to a use-after-free during device unbind?


[Severity: High]
This is a pre-existing issue, but can the threaded IRQ handler race with the
timeout worker causing a use-after-free?

The threaded IRQ handler reads dev->in_flight_job and can be preempted:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_handle_irq() {
    ...
    job = READ_ONCE(dev->in_flight_job);
    if (job) {
        WRITE_ONCE(dev->in_flight_job, NULL);
        dma_fence_signal(job->done_fence);
    }
    ...
}

If ethosu_job_timedout() executes concurrently, it stops the scheduler and
sets dev->in_flight_job to NULL, freeing the job independently:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_timedout() {
    ...
    drm_sched_stop(&dev->sched, bad);
    ...
    WRITE_ONCE(dev->in_flight_job, NULL);
    ...
}

When the asynchronous IRQ handler resumes, will it call dma_fence_signal()
using the stale job pointer and its nested fence?


[Severity: High]
This is a pre-existing issue, but can a failed allocation in
ethosu_ioctl_submit_job() lead to a NULL pointer dereference?

drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job() {
    ...
    ejob->done_fence = kzalloc_obj(*ejob->done_fence);
    if (!ejob->done_fence) {
        ...
        goto out_cleanup_job;
    }
    ...
out_cleanup_job:
    drm_sched_job_cleanup(&ejob->base);
    ...
}

Because drm_sched_job_init() hasn't been called yet, ejob->base.s_fence is
NULL. Won't drm_sched_job_cleanup() blindly dereference this pointer during
the error path?


[Severity: High]
This is a pre-existing issue, but does ethosu_job_err_cleanup() leak the
done_fence allocation?

In ethosu_ioctl_submit_job(), done_fence is dynamically allocated:

drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job() {
    ...
    ejob->done_fence = kzalloc_obj(*ejob->done_fence);
    ...
}

If validation fails later (e.g., invalid BO handles), the code falls through
to ethosu_job_err_cleanup(), which frees the job object but doesn't appear
to free done_fence:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_err_cleanup() {
    ...
    kfree(job);
}

Could this lead to a memory leak when users submit invalid jobs?


[Severity: High]
This is a pre-existing issue, but is the ETHOSU_SUBMIT ioctl handler missing
drm_dev_enter() / drm_dev_exit() protection?

drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit() {
    int ethosu_ioctl_submit(struct drm_device *dev, void *data,
                            struct drm_file *file)
    ...
}

If the device is unbound (e.g., via sysfs or hot-unplug), ethosu_remove()
tears down the DRM scheduler:

drivers/accel/ethosu/ethosu_drv.c:ethosu_remove() {
    ...
    ethosu_job_fini(ethosudev);
    ...
}

If a concurrent job submission from a process holding the DRM fd continues
executing, could it access the destroyed scheduler and cause a use-after-free?

>  	if (ret) {
>  		dev_err(dev, "failed to request irq\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=2

Re: [PATCH 02/11] accel: ethosu: Drop IRQF_SHARED flag

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:01PM -0500, Rob Herring (Arm) wrote:
> The IRQF_SHARED flag doesn't work with runtime-pm as the IRQ handler
> could run without resuming the device. This could also be fixed with
> runtime-pm calls in the IRQ handler, but there is no known need for a
> shared IRQ.
>
> Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_job.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index 99c8812691e5..c33f2877b385 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -337,7 +337,7 @@ int ethosu_job_init(struct ethosu_device *edev)
>  	ret = devm_request_threaded_irq(dev, edev->irq,
>  					ethosu_job_irq_handler,
>  					ethosu_job_irq_handler_thread,
> -					IRQF_SHARED, KBUILD_MODNAME,
> +					0, KBUILD_MODNAME,
>  					edev);
>  	if (ret) {
>  		dev_err(dev, "failed to request irq\n");
>
> --
> 2.53.0
>

[PATCH 03/11] accel: ethosu: Ensure cmd stream ends with a stop op

[Rob Herring (Arm)]

While the QSIZE register setting should prevent an out of bounds access
of the command stream, it is not clear whether the h/w generates an
interrupt in this case as is required (to prevent a timeout). As a stop op
is expected end of the command stream, let's just ensure it is present. A
stop op in the middle of the command stream also makes no sense.

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

diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
index d4458eac8447..1eca8590e68d 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
 
 enum ethosu_cmds {
+	NPU_OP_STOP = 0x0,
 	NPU_OP_CONV = 0x2,
 	NPU_OP_DEPTHWISE = 0x3,
 	NPU_OP_POOL = 0x5,
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index d50fed64d4d9..eda9f42239be 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -390,6 +390,7 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 	struct ethosu_validated_cmdstream_info __free(kfree) *info = kzalloc_obj(*info);
 	struct ethosu_device *edev = to_ethosu_device(ddev);
 	u32 *bocmds = bo->base.vaddr;
+	bool ends_with_stop = false;
 	struct cmd_state st;
 	int i, ret;
 
@@ -426,6 +427,11 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		}
 
 		switch (cmd) {
+		case NPU_OP_STOP:
+			if (i != size / 4 - 1)
+				return -EINVAL;
+			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);
@@ -688,6 +694,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		}
 	}
 
+	if (!ends_with_stop)
+		return -EINVAL;
+
 	for (i = 0; i < NPU_BASEP_REGION_MAX; i++) {
 		if (!info->region_size[i])
 			continue;

-- 
2.53.0

Re: [PATCH 03/11] accel: ethosu: Ensure cmd stream ends with a stop op

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:02PM -0500, Rob Herring (Arm) wrote:
> While the QSIZE register setting should prevent an out of bounds access
> of the command stream, it is not clear whether the h/w generates an
> interrupt in this case as is required (to prevent a timeout). As a stop op
> is expected end of the command stream, let's just ensure it is present. A
> stop op in the middle of the command stream also makes no sense.
>
> Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_device.h | 1 +
>  drivers/accel/ethosu/ethosu_gem.c    | 9 +++++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
> index d4458eac8447..1eca8590e68d 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
>
>  enum ethosu_cmds {
> +	NPU_OP_STOP = 0x0,
>  	NPU_OP_CONV = 0x2,
>  	NPU_OP_DEPTHWISE = 0x3,
>  	NPU_OP_POOL = 0x5,
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index d50fed64d4d9..eda9f42239be 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -390,6 +390,7 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  	struct ethosu_validated_cmdstream_info __free(kfree) *info = kzalloc_obj(*info);
>  	struct ethosu_device *edev = to_ethosu_device(ddev);
>  	u32 *bocmds = bo->base.vaddr;
> +	bool ends_with_stop = false;
>  	struct cmd_state st;
>  	int i, ret;
>
> @@ -426,6 +427,11 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  		}
>
>  		switch (cmd) {
> +		case NPU_OP_STOP:
> +			if (i != size / 4 - 1)
> +				return -EINVAL;
> +			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);
> @@ -688,6 +694,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  		}
>  	}
>
> +	if (!ends_with_stop)
> +		return -EINVAL;
> +
>  	for (i = 0; i < NPU_BASEP_REGION_MAX; i++) {
>  		if (!info->region_size[i])
>  			continue;
>
> --
> 2.53.0
>

[PATCH 04/11] accel: ethosu: Ensure SRAM size is 0 on mapping failure

[Rob Herring (Arm)]

On a mapping failure of the SRAM, the SRAM size is left as non-zero. The
probe will succeed as the error return is not checked since having SRAM is
not a hard requirement. The non-zero size allows jobs to access SRAM which
is left pointing to physical base address 0x0.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/accel/ethosu/ethosu_drv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index d121fb0d7732..f1af7b3ea038 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -281,8 +281,6 @@ static int ethosu_device_suspend(struct device *dev)
 
 static int ethosu_sram_init(struct ethosu_device *ethosudev)
 {
-	ethosudev->npu_info.sram_size = 0;
-
 	ethosudev->srampool = of_gen_pool_get(ethosudev->base.dev->of_node, "sram", 0);
 	if (!ethosudev->srampool)
 		return 0;
@@ -293,6 +291,7 @@ static int ethosu_sram_init(struct ethosu_device *ethosudev)
 							     ethosudev->npu_info.sram_size,
 							     &ethosudev->sramphys);
 	if (!ethosudev->sram) {
+		ethosudev->npu_info.sram_size = 0;
 		dev_err(ethosudev->base.dev, "failed to allocate from SRAM pool\n");
 		return -ENOMEM;
 	}

-- 
2.53.0

Re: [PATCH 04/11] accel: ethosu: Ensure SRAM size is 0 on mapping failure

[sashiko-bot]

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

Pre-existing issues:
- [High] ejob->done_fence is leaked on all error paths, and a NULL pointer dereference occurs if its initial allocation fails.
- [High] Hardware is not stopped and in-flight jobs are not synchronized in driver removal, leading to Use-After-Free.
--

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

accel: ethosu: Ensure SRAM size is 0 on mapping failure

On a mapping failure of the SRAM, the SRAM size is left as non-zero. The
probe will succeed as the error return is not checked since having SRAM is
not a hard requirement. The non-zero size allows jobs to access SRAM which
is left pointing to physical base address 0x0.

> diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> index d121fb0d77328..f1af7b3ea0385 100644
> --- a/drivers/accel/ethosu/ethosu_drv.c
> +++ b/drivers/accel/ethosu/ethosu_drv.c
> @@ -281,8 +281,6 @@ static int ethosu_device_suspend(struct device *dev)
>  
>  static int ethosu_sram_init(struct ethosu_device *ethosudev)
>  {
> -	ethosudev->npu_info.sram_size = 0;
> -
>  	ethosudev->srampool = of_gen_pool_get(ethosudev->base.dev->of_node, "sram", 0);
>  	if (!ethosudev->srampool)
>  		return 0;
> @@ -293,6 +291,7 @@ static int ethosu_sram_init(struct ethosu_device *ethosudev)
>  							     ethosudev->npu_info.sram_size,
>  							     &ethosudev->sramphys);
>  	if (!ethosudev->sram) {
> +		ethosudev->npu_info.sram_size = 0;
>  		dev_err(ethosudev->base.dev, "failed to allocate from SRAM pool\n");
>  		return -ENOMEM;
>  	}

While reviewing this patch, I noticed a few pre-existing issues in this driver.

>  static void ethosu_remove(struct platform_device *pdev)
>  {
>  	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);
>  }

[Severity: High]
This is a pre-existing issue, but is it safe to tear down the device this
way in ethosu_remove()?

It appears that ethosu_remove() tears down the DRM scheduler via
ethosu_job_fini() and frees the SRAM, but does not stop the hardware or wait
for the active in_flight_job to complete. If a user unbinds the driver via
sysfs while a job is running, could the hardware continue executing and access
SRAM or DMA buffers that have already been freed, leading to memory corruption?

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c

[ ... ]

>  static void ethosu_job_err_cleanup(struct ethosu_job *job)
>  {
>  	unsigned int i;
>  
>  	ethosu_perfmon_put(job->perfmon);
>  
>  	for (i = 0; i < job->region_cnt; i++)
>  		drm_gem_object_put(job->region_bo[i]);
>  
>  	drm_gem_object_put(job->cmd_bo);
>  
>  	kfree(job);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but can ethosu_job_err_cleanup()
leak job->done_fence?

It frees the job object itself, but I don't see job->done_fence being freed,
which might cause a memory leak on every job submission failure path.

[ ... ]

>  static int ethosu_ioctl_submit_job(struct drm_device *dev, struct drm_file *file,
>  				   struct drm_ethosu_job *job,
>  				   int perfmon_id)
>  {
[ ... ]
>  	ejob->done_fence = kzalloc_obj(*ejob->done_fence);
>  	if (!ejob->done_fence) {
>  		ret = -ENOMEM;
>  		goto out_cleanup_job;
>  	}
>  
>  	ret = drm_sched_job_init(&ejob->base,
>  				 &file_priv->sched_entity,
>  				 1, NULL, file->client_id);
>  	if (ret)
>  		goto out_put_job;
[ ... ]
>  out_cleanup_job:
>  	if (ret)
>  		drm_sched_job_cleanup(&ejob->base);
>  out_put_job:
>  	ethosu_job_err_cleanup(ejob);
>  
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but could jumping to out_cleanup_job when
allocating ejob->done_fence fails cause a NULL pointer dereference?

If allocating ejob->done_fence fails, the code jumps to out_cleanup_job,
which calls drm_sched_job_cleanup(&ejob->base). Because drm_sched_job_init()
hasn't run yet, ejob->base.s_fence is NULL, which might lead to a crash when it
is dereferenced during cleanup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=4

Re: [PATCH 04/11] accel: ethosu: Ensure SRAM size is 0 on mapping failure

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:03PM -0500, Rob Herring (Arm) wrote:
> On a mapping failure of the SRAM, the SRAM size is left as non-zero. The
> probe will succeed as the error return is not checked since having SRAM is
> not a hard requirement. The non-zero size allows jobs to access SRAM which
> is left pointing to physical base address 0x0.
>
> Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_drv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> index d121fb0d7732..f1af7b3ea038 100644
> --- a/drivers/accel/ethosu/ethosu_drv.c
> +++ b/drivers/accel/ethosu/ethosu_drv.c
> @@ -281,8 +281,6 @@ static int ethosu_device_suspend(struct device *dev)
>
>  static int ethosu_sram_init(struct ethosu_device *ethosudev)
>  {
> -	ethosudev->npu_info.sram_size = 0;
> -
>  	ethosudev->srampool = of_gen_pool_get(ethosudev->base.dev->of_node, "sram", 0);
>  	if (!ethosudev->srampool)
>  		return 0;
> @@ -293,6 +291,7 @@ static int ethosu_sram_init(struct ethosu_device *ethosudev)
>  							     ethosudev->npu_info.sram_size,
>  							     &ethosudev->sramphys);
>  	if (!ethosudev->sram) {
> +		ethosudev->npu_info.sram_size = 0;
>  		dev_err(ethosudev->base.dev, "failed to allocate from SRAM pool\n");
>  		return -ENOMEM;
>  	}
>
> --
> 2.53.0
>

[PATCH 05/11] accel: ethosu: Ensure SRAM region size matches job

[Rob Herring (Arm)]

It is possible for userspace to set the job SRAM size to 0, but then still
have SRAM accesses in the command stream. When the job SRAM size is 0,
setting the region base register is skipped and a stale base address from
a prior job is used.

Check the region size against the job's SRAM size instead of just the size
of the SRAM. The job's SRAM size was already checked against the total SRAM
size.

Fixes: 9cff90774872 ("accel: ethosu: Validate SRAM size on submit")
Cc: stable@vger.kernel.org
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/accel/ethosu/ethosu_job.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index c33f2877b385..0982722a9195 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -441,13 +441,13 @@ static int ethosu_ioctl_submit_job(struct drm_device *dev, struct drm_file *file
 			if (!cmd_info->region_size[i])
 				continue;
 			if (i == ETHOSU_SRAM_REGION) {
-				if (cmd_info->region_size[i] <= edev->npu_info.sram_size)
+				if (cmd_info->region_size[i] <= ejob->sram_size)
 					continue;
 
 				dev_err(dev->dev,
-					"cmd stream region %d size greater than SRAM size (%llu > %u)\n",
+					"cmd stream region %d size greater than job SRAM size (%llu > %u)\n",
 					i, cmd_info->region_size[i],
-					edev->npu_info.sram_size);
+					ejob->sram_size);
 				ret = -EINVAL;
 				goto out_cleanup_job;
 			}

-- 
2.53.0

Re: [PATCH 05/11] accel: ethosu: Ensure SRAM region size matches job

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Kernel panic due to NULL pointer dereference in `drm_sched_job_cleanup` on uninitialized job.
- [High] Memory leak of `done_fence` on error paths controllable by userspace.
- [High] Lockless TOCTOU race in IRQ handler causing premature signaling of unrelated jobs.
--

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

    accel: ethosu: Ensure SRAM region size matches job
    
    It is possible for userspace to set the job SRAM size to 0, but then still
    have SRAM accesses in the command stream. When the job SRAM size is 0,
    setting the region base register is skipped and a stale base address from
    a prior job is used.
    
    Check the region size against the job's SRAM size instead of just the size
    of the SRAM. The job's SRAM size was already checked against the total SRAM
    size.

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index c33f2877b3850..0982722a9195a 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -441,13 +441,13 @@ static int ethosu_ioctl_submit_job(struct drm_device *dev, struct drm_file *file
>  			if (!cmd_info->region_size[i])
>  				continue;
>  			if (i == ETHOSU_SRAM_REGION) {
> -				if (cmd_info->region_size[i] <= edev->npu_info.sram_size)
> +				if (cmd_info->region_size[i] <= ejob->sram_size)
>  					continue;
>  
>  				dev_err(dev->dev,
> -					"cmd stream region %d size greater than SRAM size (%llu > %u)\n",
> +					"cmd stream region %d size greater than job SRAM size (%llu > %u)\n",
>  					i, cmd_info->region_size[i],
> -					edev->npu_info.sram_size);
> +					ejob->sram_size);
>  				ret = -EINVAL;
>  				goto out_cleanup_job;

[Severity: High]
This isn't a bug introduced by this patch, but this newly modified error path
can trigger a memory leak. If the region size validation fails here, we jump
to out_cleanup_job. Does this leak the dynamically allocated
ejob->done_fence?

In ethosu_job_err_cleanup(), the job is freed:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_err_cleanup() {
    ...
	drm_gem_object_put(job->cmd_bo);

	kfree(job);
}

But it doesn't appear to free job->done_fence, allowing unprivileged
userspace to trigger a memory leak by intentionally submitting jobs with
mismatched SRAM region sizes.

>  			}

[Severity: Critical]
This is a pre-existing issue, but there is a potential NULL pointer
dereference in the error handling path for this function.

If kzalloc_obj for ejob->done_fence fails earlier in
ethosu_ioctl_submit_job(), the code jumps to out_cleanup_job without
initializing the scheduler job:

drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job() {
    ...
	ejob->done_fence = kzalloc_obj(*ejob->done_fence);
	if (!ejob->done_fence) {
		ret = -ENOMEM;
		goto out_cleanup_job;
	}
    ...
out_cleanup_job:
	if (ret)
		drm_sched_job_cleanup(&ejob->base);
}

Because drm_sched_job_init() was never called, ejob->base.s_fence is NULL.
Will this cause a NULL pointer dereference when drm_sched_job_cleanup()
attempts to read the refcount?

drivers/gpu/drm/scheduler/sched_main.c:drm_sched_job_cleanup() {
    ...
	if (kref_read(&job->s_fence->finished.refcount)) {
    ...
}


[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a lockless
TOCTOU race in the IRQ handler that could cause premature signaling of
unrelated jobs.

In ethosu_job_handle_irq(), the in_flight_job is read and signaled locklessly:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_handle_irq() {
    ...
	job = READ_ONCE(dev->in_flight_job);
	if (job) {
		WRITE_ONCE(dev->in_flight_job, NULL);
		dma_fence_signal(job->done_fence);
	}
    ...
}

If the threaded IRQ handler is delayed and a hardware timeout occurs
concurrently, ethosu_job_timedout() clears in_flight_job and the scheduler
starts the next job.

When the delayed IRQ thread resumes, could it read the new in_flight_job,
clear it, and erroneously signal the new job's fence before it has finished
executing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=5

Re: [PATCH 05/11] accel: ethosu: Ensure SRAM region size matches job

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:04PM -0500, Rob Herring (Arm) wrote:
> It is possible for userspace to set the job SRAM size to 0, but then still
> have SRAM accesses in the command stream. When the job SRAM size is 0,
> setting the region base register is skipped and a stale base address from
> a prior job is used.
>
> Check the region size against the job's SRAM size instead of just the size
> of the SRAM. The job's SRAM size was already checked against the total SRAM
> size.
>
> Fixes: 9cff90774872 ("accel: ethosu: Validate SRAM size on submit")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_job.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index c33f2877b385..0982722a9195 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -441,13 +441,13 @@ static int ethosu_ioctl_submit_job(struct drm_device *dev, struct drm_file *file
>  			if (!cmd_info->region_size[i])
>  				continue;
>  			if (i == ETHOSU_SRAM_REGION) {
> -				if (cmd_info->region_size[i] <= edev->npu_info.sram_size)
> +				if (cmd_info->region_size[i] <= ejob->sram_size)
>  					continue;
>
>  				dev_err(dev->dev,
> -					"cmd stream region %d size greater than SRAM size (%llu > %u)\n",
> +					"cmd stream region %d size greater than job SRAM size (%llu > %u)\n",
>  					i, cmd_info->region_size[i],
> -					edev->npu_info.sram_size);
> +					ejob->sram_size);
>  				ret = -EINVAL;
>  				goto out_cleanup_job;
>  			}
>
> --
> 2.53.0
>

[PATCH 06/11] 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>
---
 drivers/accel/ethosu/ethosu_drv.c | 14 ++++++++++++--
 drivers/accel/ethosu/ethosu_job.c |  6 +-----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index f1af7b3ea038..41ecfc623d42 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -371,13 +371,23 @@ 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)
-		pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
+		goto err_pm_runtime;
+
+	pm_runtime_put_autosuspend(ethosudev->base.dev);
+	return 0;
 
+err_pm_runtime:
+	pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
 	pm_runtime_put_autosuspend(ethosudev->base.dev);
+	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 0982722a9195..7cadd75ad0ba 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -349,14 +349,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 06/11] accel: ethosu: Fix probe error cleanup

[sashiko-bot]

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

Pre-existing issues:
- [High] Asynchronous Runtime PM put on probe error path causes device to remain powered on.
- [High] IRQ remains active while the DRM scheduler is destroyed on probe error and unbind, risking Use-After-Free.
- [High] IRQ handler accesses hardware registers without PM runtime protection, causing bus faults.
- [High] Memory leak of `done_fence` via user-controlled ioctl error path.
--

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

accel: ethosu: Fix probe error cleanup

This commit unwinds initialized resources on probe error paths to prevent
leaks and leaves the hardware cleanly. It also prevents calling
drm_sched_fini() on a failed scheduler initialization.

> diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> index f1af7b3ea0385..41ecfc623d425 100644
> --- a/drivers/accel/ethosu/ethosu_drv.c
> +++ b/drivers/accel/ethosu/ethosu_drv.c
> @@ -371,13 +371,23 @@ static int ethosu_probe(struct platform_device *pdev)
>  
>  	ret = ethosu_init(ethosudev);
>  	if (ret)
> -		return ret;
> +		goto err_job_fini;

[Severity: High]
This isn't a bug introduced by this patch, but does the initialization order
risk a bus fault if a spurious interrupt arrives early?

In ethosu_probe(), ethosu_job_init() requests and unmasks the IRQ before
ethosu_init() is called to enable the device clocks.

If a spurious interrupt fires while the device is still powered off (or in
autosuspend), the handler executes unconditionally:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_irq_handler() {
    ...
    u32 status = readl_relaxed(dev->regs + NPU_REG_STATUS);
    ...
}

Can reading these unclocked memory-mapped registers cause a synchronous
external abort?

>  
>  	ret = drm_dev_register(&ethosudev->base, 0);
>  	if (ret)
> -		pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
> +		goto err_pm_runtime;
> +
> +	pm_runtime_put_autosuspend(ethosudev->base.dev);
> +	return 0;
>  
> +err_pm_runtime:
> +	pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
>  	pm_runtime_put_autosuspend(ethosudev->base.dev);

[Severity: High]
This isn't a bug introduced by this patch, but does using
pm_runtime_put_autosuspend() here cause a problem with devres cleanup?

When the probe function fails and returns, devres will immediately call
pm_runtime_disable().

According to the PM subsystem guidelines, using an asynchronous
pm_runtime_put() like pm_runtime_put_autosuspend() when
pm_runtime_disable() follows immediately causes the pending suspend work
to be cancelled.

Can this leave the NPU hardware powered on and in an incorrect PM state?

> +	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]
This isn't a bug introduced by this patch, but could this error path lead to
a use-after-free regression due to the interrupt handler remaining active?

In ethosu_job_init(), the hardware interrupt is requested via
devm_request_threaded_irq().

Because it is devres-managed, the IRQ is not disabled until after the probe
function completely finishes and devres runs.

However, ethosu_job_fini() synchronously destroys the DRM scheduler here.

If a spurious or delayed interrupt fires in the window between
drm_sched_fini() and devres cleanup:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_irq_handler() {
    ...
    drm_sched_fault(&edev->sched);
    ...
}

Will the IRQ handler access the freed scheduler structures?

>  	return ret;
>  }


[Severity: High]
This is a pre-existing issue, but does the error path in
ethosu_ioctl_submit_job() leak the done_fence allocation?

In ethosu_ioctl_submit_job(), ejob->done_fence is dynamically allocated:

drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job() {
    ...
    ejob->done_fence = kzalloc_obj(*ejob->done_fence);
    ...
}

If a subsequent step fails (such as drm_gem_object_lookup failing due to an
invalid command stream buffer handle), the code jumps to out_cleanup_job. 

The cleanup function ethosu_job_err_cleanup() frees the job struct and BO
references, but it doesn't appear to call kfree() on ejob->done_fence.

Could unprivileged userspace trigger this repeatedly to drain kernel memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=6

Re: [PATCH 06/11] accel: ethosu: Fix probe error cleanup

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:05PM -0500, Rob Herring (Arm) wrote:
> 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>
> ---
>  drivers/accel/ethosu/ethosu_drv.c | 14 ++++++++++++--
>  drivers/accel/ethosu/ethosu_job.c |  6 +-----
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> index f1af7b3ea038..41ecfc623d42 100644
> --- a/drivers/accel/ethosu/ethosu_drv.c
> +++ b/drivers/accel/ethosu/ethosu_drv.c
> @@ -371,13 +371,23 @@ 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);

Does this involve any hardware access by callbck? I suppose
pm_runtime_put_autosuspend() should be end of ethosu_init().

>  	if (ret)
> -		pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
> +		goto err_pm_runtime;
> +
> +	pm_runtime_put_autosuspend(ethosudev->base.dev);
> +	return 0;
>
> +err_pm_runtime:
> +	pm_runtime_dont_use_autosuspend(ethosudev->base.dev);

it should be call by pm_runtime_disable_action().

Frank
>  	pm_runtime_put_autosuspend(ethosudev->base.dev);
> +	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 0982722a9195..7cadd75ad0ba 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -349,14 +349,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
>

[PATCH 07/11] 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.

Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/accel/ethosu/ethosu_gem.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index eda9f42239be..9fce7caeeb9a 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -259,6 +259,22 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
 	return addr;
 }
 
+static int buffer_size(struct ethosu_validated_cmdstream_info *info,
+		       struct buffer *buf, s8 region)
+{
+	u64 end;
+
+	if (region < 0 || buf->base == U64_MAX || buf->length == U32_MAX)
+		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,
@@ -303,24 +319,16 @@ 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 (buffer_size(info, &st->weight[0], st->weight[0].region))
 			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 (st->scale[0].region < 0 || st->scale[0].base == U64_MAX ||
-		    st->scale[0].length == U32_MAX)
+		if (buffer_size(info, &st->scale[0], st->scale[0].region))
 			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,

-- 
2.53.0

Re: [PATCH 07/11] accel: ethosu: Factor buffer bounds checks

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing buffer range validation for scale buffers in elementwise operations.
--

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

accel: ethosu: Factor buffer bounds checks

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

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index eda9f42239beb..9fce7caeeb9af 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -303,24 +319,16 @@ static int calc_sizes(struct drm_device *ddev,
[ ... ]
>  	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 (st->scale[0].region < 0 || st->scale[0].base == U64_MAX ||
> -		    st->scale[0].length == U32_MAX)
> +		if (buffer_size(info, &st->scale[0], st->scale[0].region))
>  			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);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while calc_sizes() validates
the scale buffer, does calc_sizes_elemwise() miss this validation for
elementwise operations?

In ethosu_gem_cmdstream_copy_and_validate(), NPU_OP_ELEMENTWISE calculates
use_scale, but it never passes it to calc_sizes_elemwise():

drivers/accel/ethosu/ethosu_gem.c:ethosu_gem_cmdstream_copy_and_validate() {
		...
		case NPU_OP_ELEMENTWISE:
			use_scale = ethosu_is_u65(edev) ?
				    (st.ifm2.broadcast & 0x80) :
				    (st.ifm2.broadcast == 8);
			use_ifm2 = !(use_scale || (param == 5) ||
				(param == 6) || (param == 7) || (param == 0x24));
			use_ifm = st.ifm.broadcast != 8;
			ret = calc_sizes_elemwise(ddev, info, cmd, &st, use_ifm, use_ifm2);
			if (ret)
				return ret;
		...
}

Because calc_sizes_elemwise() only checks ifm, ifm2, and ofm, could a
malicious command stream with an undersized scale buffer bypass validation
here?

This would allow an out-of-bounds memory read by the NPU when the
elementwise operation uses a scale stream.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=7

Re: [PATCH 07/11] accel: ethosu: Factor buffer bounds checks

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:06PM -0500, Rob Herring (Arm) wrote:
> Move the repeated command-stream buffer range validation into a
> helper in preparation for validating all weight and scale streams.
>
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_gem.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index eda9f42239be..9fce7caeeb9a 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -259,6 +259,22 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
>  	return addr;
>  }
>
> +static int buffer_size(struct ethosu_validated_cmdstream_info *info,
> +		       struct buffer *buf, s8 region)
> +{
> +	u64 end;
> +
> +	if (region < 0 || buf->base == U64_MAX || buf->length == U32_MAX)
> +		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,
> @@ -303,24 +319,16 @@ 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 (buffer_size(info, &st->weight[0], st->weight[0].region))
>  			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 (st->scale[0].region < 0 || st->scale[0].base == U64_MAX ||
> -		    st->scale[0].length == U32_MAX)
> +		if (buffer_size(info, &st->scale[0], st->scale[0].region))
>  			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,
>
> --
> 2.53.0
>

[PATCH 08/11] 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>
---
 drivers/accel/ethosu/ethosu_gem.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 9fce7caeeb9a..3d1f4121db4f 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -321,6 +321,15 @@ static int calc_sizes(struct drm_device *ddev,
 			st->weight[0].base + st->weight[0].length - 1);
 		if (buffer_size(info, &st->weight[0], st->weight[0].region))
 			return -EINVAL;
+
+		for (int i = 1; i < ARRAY_SIZE(st->weight); i++) {
+			if (st->weight[i].base == U64_MAX &&
+			    st->weight[i].length == U32_MAX)
+				continue;
+
+			if (buffer_size(info, &st->weight[i], st->weight[0].region))
+				return -EINVAL;
+		}
 	}
 
 	if (scale) {
@@ -329,6 +338,12 @@ static int calc_sizes(struct drm_device *ddev,
 			st->scale[0].base + st->scale[0].length - 1);
 		if (buffer_size(info, &st->scale[0], st->scale[0].region))
 			return -EINVAL;
+
+		if (ethosu_is_u65(edev) &&
+		    (st->scale[1].base != U64_MAX ||
+		     st->scale[1].length != U32_MAX) &&
+		    buffer_size(info, &st->scale[1], st->scale[0].region))
+			return -EINVAL;
 	}
 
 	len = feat_matrix_length(edev, info, &st->ofm, st->ofm.width,

-- 
2.53.0

Re: [PATCH 08/11] accel: ethosu: Validate secondary streams

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:07PM -0500, Rob Herring (Arm) wrote:
> 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>
> ---
>  drivers/accel/ethosu/ethosu_gem.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 9fce7caeeb9a..3d1f4121db4f 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -321,6 +321,15 @@ static int calc_sizes(struct drm_device *ddev,
>  			st->weight[0].base + st->weight[0].length - 1);
>  		if (buffer_size(info, &st->weight[0], st->weight[0].region))
>  			return -EINVAL;
> +
> +		for (int i = 1; i < ARRAY_SIZE(st->weight); i++) {

why not also put 0 into this loop?

> +			if (st->weight[i].base == U64_MAX &&
> +			    st->weight[i].length == U32_MAX)
> +				continue;
> +
> +			if (buffer_size(info, &st->weight[i], st->weight[0].region))

Just make sure 0 of st->weight[0].region is not typo, all weight share
one region?

Frank

> +				return -EINVAL;
> +		}
>  	}
>
>  	if (scale) {
> @@ -329,6 +338,12 @@ static int calc_sizes(struct drm_device *ddev,
>  			st->scale[0].base + st->scale[0].length - 1);
>  		if (buffer_size(info, &st->scale[0], st->scale[0].region))
>  			return -EINVAL;
> +
> +		if (ethosu_is_u65(edev) &&
> +		    (st->scale[1].base != U64_MAX ||
> +		     st->scale[1].length != U32_MAX) &&
> +		    buffer_size(info, &st->scale[1], st->scale[0].region))
> +			return -EINVAL;
>  	}
>
>  	len = feat_matrix_length(edev, info, &st->ofm, st->ofm.width,
>
> --
> 2.53.0
>

[PATCH 09/11] 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>
---
 drivers/accel/ethosu/ethosu_device.h |  4 ++++
 drivers/accel/ethosu/ethosu_gem.c    | 17 +++++++++++++++++
 2 files changed, 21 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 3d1f4121db4f..2aafbfe95a8c 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -450,6 +450,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
 		}
 
 		switch (cmd) {
+		case NPU_OP_BRANCH:
+		case NPU_OP_IRQ:
+			return -EINVAL;
 		case NPU_OP_STOP:
 			if (i != size / 4 - 1)
 				return -EINVAL;
@@ -522,6 +525,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:
@@ -565,6 +570,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:
@@ -599,6 +608,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:
@@ -673,13 +684,19 @@ 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.mode = (param >> 9) & 0x3;
+			if (st.dma.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

-- 
2.53.0

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The hardware's state registers and BASEP (base pointer) registers are not reset between jobs, allowing an attacker to omit configuration commands and hijack memory regions from previously executed jobs.
- [Critical] The `dma_length()` function incorrectly calculates memory bounds for negative DMA strides, drastically underestimating the maximum address and failing to check for buffer underflows.
- [Medium] The command-stream validator employs a default-allow policy for unknown commands, passing unrecognized NPU commands directly to the hardware without any safety checks.
--

commit 3012502d7ce6f9c86b531a92e4564ed74d340366
Author: Rob Herring (Arm) <robh@kernel.org>
accel: ethosu: Reject unsupported commands

This commit updates the command-stream validator to reject unsupported commands
like branches, IRQs, and certain DMA configurations. It prevents commands that
bypass validation state or access unchecked index buffers.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 3d1f4121db4f4..2aafbfe95a8c2 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -450,6 +450,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  		}
>  
>  		switch (cmd) {
> +		case NPU_OP_BRANCH:
> +		case NPU_OP_IRQ:
> +			return -EINVAL;

[Severity: Medium]
This is a pre-existing issue, but is the validator using a default-allow
policy for unknown commands?

If a user submits a command stream containing unrecognized opcodes (such as
undocumented commands or those from a minor hardware revision), the validator
hits the default case:

drivers/accel/ethosu/ethosu_gem.c:ethosu_gem_cmdstream_copy_and_validate() {
		...
		switch (cmd) {
		...
		default:
			break;
		}
		...
}

This ignores the command without calculating memory bounds for it, allowing it
to be copied to the execution buffer. The hardware could then execute it,
potentially accessing physical memory outside the validated GEM bounds. Should
the validator fail closed by returning -EINVAL for unrecognized commands?

[ ... ]
> @@ -673,13 +684,19 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			break;
>  
>  		case NPU_SET_DMA0_SRC_REGION:

[Severity: Critical]
This isn't a bug introduced by this patch, but what happens if an attacker
intentionally omits these configuration commands?

If a command stream contains NPU_OP_DMA_START but omits
NPU_SET_DMA0_SRC_REGION and NPU_SET_DMA0_DST_REGION, the regions default to
-1 because cmd_state_init() initializes the state with 0xff:

drivers/accel/ethosu/ethosu_gem.c:cmd_state_init() {
	memset(st, 0xff, sizeof(*st));
}

In dma_length(), the upper bound update is bypassed because of the region check:

drivers/accel/ethosu/ethosu_gem.c:dma_length() {
	if (dma->region >= 0) {
		...
		info->region_size[dma->region] = max(...);
	}
}

Because region_size remains 0, the job can be submitted without a valid
region_bo_handle. When ethosu_job_hw_submit() runs, it only programs NPU_REG_BASEP
for explicitly provided regions:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_hw_submit() {
	...
	writel_relaxed(lower_32_bits(bo->dma_addr), dev->regs + NPU_REG_BASEP(region));
	...
}

This leaves other BASEP registers retaining physical addresses from the previously
executed job. The NPU hardware retains internal state across jobs, so it could
use the un-overridden regions from the previous job, leading to cross-process
memory leaks or physical memory corruption.

[Severity: Critical]
This is also a pre-existing issue, but how are negative DMA strides handled
during length calculation?

drivers/accel/ethosu/ethosu_gem.c:dma_length() {
	...
	len += dma->stride[0];
	if (check_mul_overflow(len, (u64)dma_st->size0, &len))
		return U64_MAX;
	...
}

If a negative stride is provided (e.g., stride[0] = -len and offset = 0), the
logic incorrectly subtracts it before multiplication. This results in len = 0,
making the maximum address calculation 0 and bypassing the upper bound check.

Since the hardware DMA engine iterates backwards, it would access physical memory
below BASEP. The validator never calculates the minimum address accessed nor
checks for this lower bound underflow, which can allow out-of-bounds physical
memory access.

> +			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.mode = (param >> 9) & 0x3;
> +			if (st.dma.mode == 3)
> +				return -EINVAL;
>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=9

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

[Frank Li]

On Thu, Aug 27, 2026 at 03:33:08PM -0500, Rob Herring (Arm) wrote:
> 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>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/accel/ethosu/ethosu_device.h |  4 ++++
>  drivers/accel/ethosu/ethosu_gem.c    | 17 +++++++++++++++++
>  2 files changed, 21 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 3d1f4121db4f..2aafbfe95a8c 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -450,6 +450,9 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  		}
>
>  		switch (cmd) {
> +		case NPU_OP_BRANCH:
> +		case NPU_OP_IRQ:
> +			return -EINVAL;
>  		case NPU_OP_STOP:
>  			if (i != size / 4 - 1)
>  				return -EINVAL;
> @@ -522,6 +525,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:
> @@ -565,6 +570,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:
> @@ -599,6 +608,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:
> @@ -673,13 +684,19 @@ 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.mode = (param >> 9) & 0x3;
> +			if (st.dma.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
>
> --
> 2.53.0
>

[PATCH 10/11] 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>
---
 drivers/accel/ethosu/ethosu_gem.c | 117 ++++++++++++++++++++++++++++++--------
 1 file changed, 93 insertions(+), 24 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index 2aafbfe95a8c..a042e650f626 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -259,6 +259,72 @@ 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 feat_matrix *fm, u32 x, u32 y, u32 c,
+				      bool ofm, u64 *max_len)
+{
+	u64 len;
+
+	len = feat_matrix_length(edev, info, fm, 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 feat_matrix *fm,
+			    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, fm,
+								 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, fm, 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, fm, x,
+							 fm->height[1], c, ofm,
+							 max_len);
+			if (ret)
+				return ret;
+		}
+		if (fm->height[1] < y) {
+			ret = feat_matrix_check_location(edev, info, fm, x,
+							 fm->height[1] + 1, c, ofm,
+							 max_len);
+			if (ret)
+				return ret;
+		}
+		return feat_matrix_check_location(edev, info, fm, x, y, c,
+						  ofm, max_len);
+	}
+
+	return feat_matrix_check_location(edev, info, fm, x, y, c, ofm,
+					  max_len);
+}
+
 static int buffer_size(struct ethosu_validated_cmdstream_info *info,
 		       struct buffer *buf, s8 region)
 {
@@ -282,6 +348,7 @@ static int calc_sizes(struct drm_device *ddev,
 {
 	struct ethosu_device *edev = to_ethosu_device(ddev);
 	u64 len;
+	int ret;
 
 	if (ifm) {
 		if (st->ifm.stride_kernel == U16_MAX)
@@ -298,21 +365,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->ifm, ifm_width,
-					 ifm_height, st->ifm.depth, false);
+		ret = feat_matrix_size(edev, info, &st->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->ifm2, st->ifm.depth,
-					 0, st->ofm.depth, false);
+		ret = feat_matrix_size(edev, info, &st->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) {
@@ -346,12 +414,12 @@ static int calc_sizes(struct drm_device *ddev,
 			return -EINVAL;
 	}
 
-	len = feat_matrix_length(edev, info, &st->ofm, st->ofm.width,
-				 st->ofm.height[2], st->ofm.depth, true);
+	ret = feat_matrix_size(edev, info, &st->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;
 
@@ -366,18 +434,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->ifm, width,
-					 height, depth, false);
+		ret = feat_matrix_size(edev, info, &st->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) {
@@ -385,20 +454,20 @@ 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);
+		ret = feat_matrix_size(edev, info, &st->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->ofm, st->ofm.width,
-				 st->ofm.height[2], st->ofm.depth, true);
+	ret = feat_matrix_size(edev, info, &st->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 10/11] accel: ethosu: Validate all feature map tiles

[sashiko-bot]

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

New issues:
- [Critical] Incomplete U65 storage encoding override causes inconsistent tile address calculation and broken validation.

Pre-existing issues:
- [Critical] Insufficient bounds checking of intermediate feature map tiles due to evaluating start coordinates instead of maximum accessed coordinates.
--

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

accel: ethosu: Validate all feature map tiles

This commit updates the command-stream validator to check the final 
coordinate of every tile touched by an operation instead of just the 
final feature-map coordinate. It also treats U65 feature maps as 2x2 
tiled.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 2aafbfe95a8c2..a042e650f626a 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -259,6 +259,72 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
>  	return addr;
>  }
>  
> +static int feat_matrix_check_location(struct ethosu_device *edev,
[ ... ]
> +static int feat_matrix_size(struct ethosu_device *edev,
> +			    struct ethosu_validated_cmdstream_info *info,
> +			    struct feat_matrix *fm,
> +			    u32 x, u32 y, u32 c, bool ofm, u64 *max_len)
> +{
> +	u32 storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14;

[Severity: Critical]
Does this U65 storage encoding override create an inconsistency with how
feat_matrix_length() calculates the storage?

In feat_matrix_size(), storage is forced to 0 for U65. However, looking at
feat_matrix_length(), it still decodes storage using the precision bits:

static u64 feat_matrix_length(...)
{
        u32 element_size, storage = fm->precision >> 14;
        ...
}

Could a manipulated command stream use this discrepancy to pass validation on
one tile (using storage = 0) while the hardware accesses a different,
unvalidated tile base address (using storage = 1)?

> +	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, fm,
> +								 xi ? x : 0,
> +								 yi ? y : 0, c, ofm,
> +								 max_len);

[Severity: Critical]
This is a pre-existing issue, but does this loop evaluate the absolute
coordinate 0 instead of the maximum valid coordinate for the intermediate
tile?

By checking (0, y) and (x, y), it appears we skip the maximum local
coordinate accessed in the intermediate tile (such as fm->width0). Could
this allow a command stream with an inflated stride to pass validation
here but cause the hardware to perform an out-of-bounds DMA access?

> +				if (ret)
> +					return ret;
> +			}
> +		}
> +		return 0;
> +	}
> +
> +	if (storage == 1) {
> +		ret = feat_matrix_check_location(edev, info, fm, x, 0, c,
> +						 ofm, max_len);

[Severity: Critical]
This is a pre-existing issue, but similar to the 2x2 case above, is tile 0
checked at y=0 instead of its maximum accessed extent (fm->height[0])?

This might leave its size bounds not fully validated.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=10

[PATCH 11/11] 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>
---
 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 c330048dbcca..1731c43aa045 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_OFM_TRANSPOSE_MASK		GENMASK(13, 11)
 
 enum ethosu_cmds {
 	NPU_OP_STOP = 0x0,
diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
index a042e650f626..ad36fb8b3b30 100644
--- a/drivers/accel/ethosu/ethosu_gem.c
+++ b/drivers/accel/ethosu/ethosu_gem.c
@@ -199,6 +199,52 @@ static bool feat_matrix_chained(struct ethosu_device *edev, struct feat_matrix *
 	return !ethosu_is_u65(edev) && storage == 2;
 }
 
+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 feat_matrix *fm,
@@ -283,6 +329,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++) {
@@ -641,8 +690,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 11/11] accel: ethosu: Validate OFM transpose

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Inconsistent extraction of the 'storage' configuration bitfield between `feat_matrix_size` and `feat_matrix_length` allows an attacker to bypass bounds checking for `base[3]` on U65 devices, leading to arbitrary out-of-bounds memory accesses by the NPU.
--

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

accel: ethosu: Validate OFM transpose

This commit permutes the output endpoint before validating its tile and
stride accesses on U85 devices, while rejecting reserved transpose encodings.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index a042e650f626a..ad36fb8b3b302 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -283,6 +329,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) {

[Severity: Critical]
This isn't a bug introduced by this patch, but does this inconsistent
extraction of the storage bitfield lead to missing bounds checks for the
4th tile on U65 devices?

In feat_matrix_size(), storage is correctly forced to 0 for U65 hardware:

drivers/accel/ethosu/ethosu_gem.c:feat_matrix_size() {
	u32 storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14;
    ...
}

This ensures the loop checks all 4 possible tile coordinates. However,
feat_matrix_length() extracts it directly from the precision bits without
checking if the hardware is U65:

drivers/accel/ethosu/ethosu_gem.c:feat_matrix_length() {
	u32 element_size, storage = fm->precision >> 14;
    ...
}

If a user provides a command stream with bits 14-15 set to 1 (indicating
storage = 1) and sets a large, out-of-bounds offset for base[3],
feat_matrix_size() forces storage = 0 and delegates address computation to
feat_matrix_length().

Because feat_matrix_length() uses the unmasked storage = 1 value, its tile
computation logic will only ever produce tile = 1 or tile = 2. As a result,
the 4th tile (base[3]) is never visited or verified against the GEM buffer
size.

The previous commit "accel: ethosu: Validate all feature map tiles"
attempted to fix U65 tile validation, but left the direct bitwise extraction
in feat_matrix_length() intact.

Since U65 hardware natively uses 2x2 tiling and ignores the storage bits,
could this allow the NPU DMA engine to access arbitrary physical memory
outside the provided GEM buffers when evaluating the true coordinates?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-ethosu-fixes-v1-0-346f9ea8791c@kernel.org?part=11