* [PATCH 0/3] accel/ethosu: fix probe and job cleanup errors
@ 2026-07-16 8:25 zhaoguohan
2026-07-16 8:25 ` [PATCH 1/3] accel/ethosu: clean up resources on probe failure zhaoguohan
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: zhaoguohan @ 2026-07-16 8:25 UTC (permalink / raw)
To: Rob Herring (Arm), Tomeu Vizoso, dri-devel
Cc: Oded Gabbay, Frank Li, Thomas Zimmermann, linux-kernel, stable
From: GuoHan Zhao <zhaoguohan@kylinos.cn>
Fix error handling and cleanup issues in the Arm Ethos-U driver.
Sashiko reported these issues while reviewing the Ethos-U MMIO mapping fix.
The report is available at:
https://sashiko.dev/#/patchset/20260716065219.931088-1-zhaoguohan@kylinos.cn?part=1
The first two patches unwind scheduler, runtime PM, and SRAM state when
probe or SRAM setup fails. The final patch fixes completion fence ownership
across submit failures and scheduler early exits.
GuoHan Zhao (3):
accel/ethosu: clean up resources on probe failure
accel/ethosu: propagate SRAM initialization errors
accel/ethosu: fix job completion fence cleanup
drivers/accel/ethosu/ethosu_drv.c | 39 +++++++++++++++++++++++++------
drivers/accel/ethosu/ethosu_job.c | 8 ++++++-
2 files changed, 39 insertions(+), 8 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] accel/ethosu: clean up resources on probe failure
2026-07-16 8:25 [PATCH 0/3] accel/ethosu: fix probe and job cleanup errors zhaoguohan
@ 2026-07-16 8:25 ` zhaoguohan
2026-07-16 8:25 ` [PATCH 2/3] accel/ethosu: propagate SRAM initialization errors zhaoguohan
2026-07-16 8:25 ` [PATCH 3/3] accel/ethosu: fix job completion fence cleanup zhaoguohan
2 siblings, 0 replies; 4+ messages in thread
From: zhaoguohan @ 2026-07-16 8:25 UTC (permalink / raw)
To: Rob Herring (Arm), Tomeu Vizoso, dri-devel
Cc: Oded Gabbay, Frank Li, Thomas Zimmermann, linux-kernel, stable
From: GuoHan Zhao <zhaoguohan@kylinos.cn>
ethosu_job_init() creates a scheduler before ethosu_init() and
drm_dev_register(). Errors from either later step return directly, leaving
the scheduler workqueue allocated. A drm_dev_register() failure also leaves
the optional SRAM allocation behind.
Unwind runtime PM, SRAM, and scheduler state in reverse order on probe
failure. Disable the clocks when runtime PM setup fails as well.
Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260716065219.931088-1-zhaoguohan@kylinos.cn?part=1
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
Sashiko report:
> This is a pre-existing issue, but was not introduced by this patch.
>
> If ethosu_init() or drm_dev_register() fail, the function returns
> directly without calling ethosu_job_fini() or freeing the sram pool.
>
> Because the job scheduler is embedded in the devm-allocated
> ethosu_device, it will be freed when devm cleans up. However, the drm
> scheduler kthreads and timers remain active and will access the freed
> memory.
>
> Can this cause a use-after-free regression leading to a kernel panic?
drivers/accel/ethosu/ethosu_drv.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index ed9c748a54ad..d1153b15ca3e 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -293,6 +293,14 @@ static int ethosu_sram_init(struct ethosu_device *ethosudev)
return 0;
}
+static void ethosu_sram_fini(struct ethosu_device *ethosudev)
+{
+ if (ethosudev->sram)
+ gen_pool_free(ethosudev->srampool,
+ (unsigned long)ethosudev->sram,
+ ethosudev->npu_info.sram_size);
+}
+
static int ethosu_init(struct ethosu_device *ethosudev)
{
int ret;
@@ -306,7 +314,7 @@ static int ethosu_init(struct ethosu_device *ethosudev)
pm_runtime_use_autosuspend(ethosudev->base.dev);
ret = devm_pm_runtime_set_active_enabled(ethosudev->base.dev);
if (ret)
- return ret;
+ goto err_suspend;
pm_runtime_get_noresume(ethosudev->base.dev);
ethosudev->npu_info.id = id = readl_relaxed(ethosudev->regs + NPU_REG_ID);
@@ -326,6 +334,11 @@ static int ethosu_init(struct ethosu_device *ethosudev)
ethosudev->npu_info.sram_size / 1024);
return 0;
+
+err_suspend:
+ pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
+ ethosu_device_suspend(ethosudev->base.dev);
+ return ret;
}
static int ethosu_probe(struct platform_device *pdev)
@@ -353,13 +366,21 @@ static int ethosu_probe(struct platform_device *pdev)
ret = ethosu_init(ethosudev);
if (ret)
- return ret;
+ goto err_job_fini;
ret = drm_dev_register(ðosudev->base, 0);
if (ret)
- pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
+ goto err_runtime_suspend;
pm_runtime_put_autosuspend(ethosudev->base.dev);
+ return 0;
+
+err_runtime_suspend:
+ pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
+ pm_runtime_put_sync_suspend(ethosudev->base.dev);
+ ethosu_sram_fini(ethosudev);
+err_job_fini:
+ ethosu_job_fini(ethosudev);
return ret;
}
@@ -369,9 +390,7 @@ static void ethosu_remove(struct platform_device *pdev)
drm_dev_unregister(ðosudev->base);
ethosu_job_fini(ethosudev);
- if (ethosudev->sram)
- gen_pool_free(ethosudev->srampool, (unsigned long)ethosudev->sram,
- ethosudev->npu_info.sram_size);
+ ethosu_sram_fini(ethosudev);
}
static const struct of_device_id dt_match[] = {
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/3] accel/ethosu: propagate SRAM initialization errors
2026-07-16 8:25 [PATCH 0/3] accel/ethosu: fix probe and job cleanup errors zhaoguohan
2026-07-16 8:25 ` [PATCH 1/3] accel/ethosu: clean up resources on probe failure zhaoguohan
@ 2026-07-16 8:25 ` zhaoguohan
2026-07-16 8:25 ` [PATCH 3/3] accel/ethosu: fix job completion fence cleanup zhaoguohan
2 siblings, 0 replies; 4+ messages in thread
From: zhaoguohan @ 2026-07-16 8:25 UTC (permalink / raw)
To: Rob Herring (Arm), Tomeu Vizoso, dri-devel
Cc: Oded Gabbay, Frank Li, Thomas Zimmermann, linux-kernel, stable
From: GuoHan Zhao <zhaoguohan@kylinos.cn>
ethosu_sram_init() returns -ENOMEM if allocation from a declared SRAM
pool fails. ethosu_init() ignores the error after recording the pool size.
Probe then registers a device with a nonzero SRAM size and a zero sramphys
address, allowing jobs to program address zero for SRAM.
Propagate the error and synchronously suspend the device before the probe
path releases the scheduler.
Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260716065219.931088-1-zhaoguohan@kylinos.cn?part=1
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
Sashiko report:
> This is a pre-existing issue, but was not introduced by this patch.
>
> In ethosu_sram_init(), if gen_pool_dma_alloc() fails, it returns an
> error. However, ethosu_init() ignores this return value.
>
> Because npu_info.sram_size was already set to gen_pool_size(), it remains
> non-zero while sramphys remains 0.
>
> When a user submits a job requesting sram, the size check passes, and
> ethosu_job_hw_submit() configures the hardware to use physical sram
> address 0x0.
>
> Could this result in a memory corruption regression or an IOMMU fault?
drivers/accel/ethosu/ethosu_drv.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index d1153b15ca3e..479156f79bc1 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -320,7 +320,9 @@ static int ethosu_init(struct ethosu_device *ethosudev)
ethosudev->npu_info.id = id = readl_relaxed(ethosudev->regs + NPU_REG_ID);
ethosudev->npu_info.config = config = readl_relaxed(ethosudev->regs + NPU_REG_CONFIG);
- ethosu_sram_init(ethosudev);
+ ret = ethosu_sram_init(ethosudev);
+ if (ret)
+ goto err_runtime_suspend;
dev_info(ethosudev->base.dev,
"Ethos-U NPU, arch v%ld.%ld.%ld, rev r%ldp%ld, cmd stream ver%ld, %d MACs, %dKB SRAM\n",
@@ -335,6 +337,10 @@ static int ethosu_init(struct ethosu_device *ethosudev)
return 0;
+err_runtime_suspend:
+ pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
+ pm_runtime_put_sync_suspend(ethosudev->base.dev);
+ return ret;
err_suspend:
pm_runtime_dont_use_autosuspend(ethosudev->base.dev);
ethosu_device_suspend(ethosudev->base.dev);
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 3/3] accel/ethosu: fix job completion fence cleanup
2026-07-16 8:25 [PATCH 0/3] accel/ethosu: fix probe and job cleanup errors zhaoguohan
2026-07-16 8:25 ` [PATCH 1/3] accel/ethosu: clean up resources on probe failure zhaoguohan
2026-07-16 8:25 ` [PATCH 2/3] accel/ethosu: propagate SRAM initialization errors zhaoguohan
@ 2026-07-16 8:25 ` zhaoguohan
2 siblings, 0 replies; 4+ messages in thread
From: zhaoguohan @ 2026-07-16 8:25 UTC (permalink / raw)
To: Rob Herring (Arm), Tomeu Vizoso, dri-devel
Cc: Oded Gabbay, Frank Li, Thomas Zimmermann, linux-kernel, stable
From: GuoHan Zhao <zhaoguohan@kylinos.cn>
ethosu_ioctl_submit_job() allocates done_fence before validating buffer
handles. Errors after allocation call ethosu_job_err_cleanup(), which frees
the job but leaks the uninitialized fence.
A scheduler dependency error also lets ethosu_job_run() return before
dma_fence_init(). Normal cleanup then passes a zeroed refcount to
dma_fence_put().
Release done_fence in the common cleanup path and use
dma_fence_was_initialized() to distinguish initialized fences from raw
allocations.
Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260716065219.931088-1-zhaoguohan@kylinos.cn?part=1
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
Sashiko reports:
> drivers/accel/ethosu/ethosu_job.c:ethosu_job_err_cleanup() {
> unsigned int i;
>
> 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);
> }
>
> This is a pre-existing issue, but was not introduced by this patch.
>
> ethosu_ioctl_submit_job() allocates done_fence. If a subsequent
> validation fails, the error path calls ethosu_job_err_cleanup().
>
> This function frees the job structure but misses freeing job->done_fence.
>
> Can an unprivileged user repeatedly call the submit ioctl with invalid
> handles to trigger a done_fence leak?
> drivers/accel/ethosu/ethosu_job.c:ethosu_job_run() {
> ...
> if (unlikely(job->base.s_fence->finished.error))
> return NULL;
>
> dma_fence_init(fence, ðosu_fence_ops, &dev->fence_lock,
> dev->fence_context, ++dev->emit_seqno);
> ...
> }
>
> This is a pre-existing issue, but was not introduced by this patch.
>
> If job->base.s_fence->finished.error is true, the function returns early
> without calling dma_fence_init() on job->done_fence.
>
> When the job is later destroyed, ethosu_job_cleanup() unconditionally
> calls dma_fence_put() on the fence:
>
> drivers/accel/ethosu/ethosu_job.c:ethosu_job_cleanup() {
> ...
> dma_fence_put(job->done_fence);
> ...
> }
>
> Since the fence is zero-initialized, will calling kref_put() on it
> violate the API and cause a refcount underflow?
drivers/accel/ethosu/ethosu_job.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index b76924645aaa..be745e36f1fa 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -152,6 +152,13 @@ static void ethosu_job_err_cleanup(struct ethosu_job *job)
drm_gem_object_put(job->cmd_bo);
+ if (job->done_fence) {
+ if (dma_fence_was_initialized(job->done_fence))
+ dma_fence_put(job->done_fence);
+ else
+ dma_fence_free(job->done_fence);
+ }
+
kfree(job);
}
@@ -162,7 +169,6 @@ static void ethosu_job_cleanup(struct kref *ref)
pm_runtime_put_autosuspend(job->dev->base.dev);
- dma_fence_put(job->done_fence);
dma_fence_put(job->inference_done_fence);
ethosu_job_err_cleanup(job);
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-16 8:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-16 8:25 [PATCH 0/3] accel/ethosu: fix probe and job cleanup errors zhaoguohan
2026-07-16 8:25 ` [PATCH 1/3] accel/ethosu: clean up resources on probe failure zhaoguohan
2026-07-16 8:25 ` [PATCH 2/3] accel/ethosu: propagate SRAM initialization errors zhaoguohan
2026-07-16 8:25 ` [PATCH 3/3] accel/ethosu: fix job completion fence cleanup zhaoguohan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox