Re: [PATCH v2 5/5] accel/ethosu: fix job completion fence cleanup

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Calling drm_sched_job_cleanup() on an uninitialized struct drm_sched_job causes a NULL pointer dereference in the ethosu_ioctl_submit_job() error path.
- [High] The shared IRQ handler accesses hardware registers without ensuring the device is active via Runtime PM, and the suspend callback fails to synchronize IRQs, risking fatal bus faults.
--

--- Patch [5]: [PATCH v2 5/5] accel/ethosu: fix job completion fence cleanup ---
commit b390c1806154d7460d3ff212d56c250283785f11
Author: GuoHan Zhao <zhaoguohan@kylinos.cn>
Subject: accel/ethosu: fix job completion fence cleanup

This commit addresses memory leaks and refcount underflows in the Ethos-U NPU
driver's job cleanup path. It ensures that the done_fence is properly released
during error cleanup and uses dma_fence_was_initialized() to handle raw
allocations gracefully.

Link: https://sashiko.dev/#/patchset/20260716065219.931088-1-zhaoguohan@kylinos.cn?part=1

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index 5847433e87e7e..b68a905d22f83 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);
>  }
>  

[Severity: High]
This is a pre-existing issue, but was not introduced by this patch.

When the done_fence allocation fails in ethosu_ioctl_submit_job(), it jumps
to out_cleanup_job:

drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job() {
    ...
	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 not called yet, ejob->base is completely
zero-initialized, meaning ejob->base.s_fence is NULL.

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)) {
    ...
}

Will this result in a NULL pointer dereference because the function assumes
job->s_fence is a valid pointer?


[Severity: High]
This isn't a bug introduced by this patch, but there appears to be an issue
with how shared interrupts interact with runtime PM.

The NPU interrupt is registered with the IRQF_SHARED flag:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_init() {
    ...
	ret = devm_request_threaded_irq(dev, edev->irq,
				ethosu_job_irq_handler,
				ethosu_job_irq_handler_thread,
				IRQF_SHARED, KBUILD_MODNAME,
				edev);
    ...
}

If another device sharing the IRQ line triggers an interrupt while the NPU is
runtime-suspended, the handler executes and reads MMIO registers:

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

	if (!(status & STATUS_IRQ_RAISED))
		return IRQ_NONE;
    ...
}

Could this unconditionally read registers without verifying the device's PM
state via pm_runtime_get_if_active()?

Furthermore, the runtime suspend callback disables clocks:

drivers/accel/ethosu/ethosu_drv.c:ethosu_device_suspend() {
    ...
	clk_bulk_disable_unprepare(ethosudev->num_clks, ethosudev->clks);
	return 0;
}

Since this doesn't call synchronize_irq() to wait for executing handlers,
could concurrent IRQ execution cause invalid hardware writes or a bus fault
when accessing powered-off MMIO registers?

> @@ -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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717061145.1478139-1-zhaoguohan@kylinos.cn?part=5