Re: [PATCH v3 05/22] accel: ethosu: Quiesce jobs before scheduler teardown

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but prematurely tearing down the DRM scheduler during device removal causes a Use-After-Free and unkillable process hang if userspace keeps the DRM file open.
- [Critical] This problem wasn't introduced by this patch, but missing disable_irq() during driver teardown allows the IRQ handler to re-arm the scheduler timeout work after or during scheduler teardown, leading to a Use-After-Free.
--

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

This commit attempts to safely tear down the scheduler during driver removal
by stopping the submission and timeout workqueues, resetting the NPU device,
and synchronizing the IRQ before freeing the scheduler.

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> index ec65305e0cd7d..a1443cd0c5b05 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
[ ... ]
> @@ -363,6 +375,15 @@ int ethosu_job_init(struct ethosu_device *edev)
>  
>  void ethosu_job_fini(struct ethosu_device *dev)
>  {
> +	drm_sched_wqueue_stop(&dev->sched);
> +	cancel_delayed_work_sync(&dev->sched.work_tdr);
> +
> +	if (READ_ONCE(dev->in_flight_job)) {
> +		WRITE_ONCE(dev->in_flight_job, NULL);
> +		ethosu_device_reset(dev);
> +	}
> +
> +	synchronize_irq(dev->irq);

[Severity: Critical]
This isn't a bug introduced by this patch, but does the missing disable_irq()
during driver teardown leave a window for the IRQ handler to re-arm the
scheduler timeout work?

While synchronize_irq() waits for currently running IRQ handlers to finish, it
does not disable the IRQ line itself. Since devm_request_threaded_irq() keeps
the IRQ alive until the devres release phase (which happens after the driver
remove function returns), a late interrupt could still fire.

If an interrupt fires after synchronize_irq() returns, ethosu_job_handle_irq()
could execute and call drm_sched_fault(), which re-arms the delayed timeout
work (work_tdr) on a destroyed scheduler. Could this lead to a
Use-After-Free?

>  	drm_sched_fini(&dev->sched);

[Severity: Critical]
This is a pre-existing issue, but does prematurely tearing down the DRM
scheduler during device removal cause a Use-After-Free and an unkillable
process hang if userspace keeps the DRM file open?

When the device is unbound (e.g., via sysfs or hotplug), ethosu_job_fini()
halts the workqueues and destroys the scheduler. If a userspace process keeps
the DRM file descriptor open and closes it later, ethosu_postclose() will
initiate the teardown of the entity via ethosu_job_close().

Because the scheduler has already been freed, the entity teardown will access
freed memory and hang forever waiting for jobs to execute on the stopped
scheduler. Can we ensure the scheduler remains alive until all entities
are destroyed?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-ethosu-fixes-v3-0-490fe215286f@kernel.org?part=5