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) <[email protected]> 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/[email protected]?part=5
