Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Possible Use-After-Free of firmware structures when group_process_tiler_oom() runs concurrently with or after panthor_fw_unplug(). This problem wasn't introduced by this patch, but remains unresolved as replacing fw_events_work with direct threaded IRQ processing still leaves tiler_oom_work queued to an unflushed workqueue. -- --- Patch [8]: [PATCH v7 8/9] drm/panthor: Don't defer FW event processing --- commit 99ae40464e2d4753b77cca3059716afd0d1bd396 Author: Boris Brezillon <[email protected]> drm/panthor: Don't defer FW event processing This commit avoids a workqueue roundtrip by processing firmware events immediately from panthor_sched_report_fw_events() rather than deferring them to the fw_events_work workqueue. > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c > b/drivers/gpu/drm/panthor/panthor_sched.c > index 64b5fc1a0984f..7f99745ff4041 100644 > --- a/drivers/gpu/drm/panthor/panthor_sched.c > +++ b/drivers/gpu/drm/panthor/panthor_sched.c [ ... ] > @@ -4083,7 +4055,6 @@ void panthor_sched_unplug(struct panthor_device *ptdev) > struct panthor_scheduler *sched = ptdev->scheduler; > > disable_delayed_work_sync(&sched->tick_work); > - disable_work_sync(&sched->fw_events_work); [Severity: High] This is a pre-existing issue, but since we are modifying the workqueue teardown here in panthor_sched_unplug(), is it possible for a tiler_oom_work to execute after panthor_fw_unplug() has already run? In panthor_device_unplug(), the sequence calls panthor_sched_unplug() which stops the scheduler tick and disables some works, but it doesn't appear to flush sched->heap_alloc_wq where tiler_oom_work is queued. If panthor_fw_unplug() subsequently destroys the firmware mappings, and a pending tiler_oom_work executes group_process_tiler_oom(): drivers/gpu/drm/panthor/panthor_sched.c:group_process_tiler_oom() cs_iface = panthor_fw_get_cs_iface(ptdev, csg_id, cs_id); heaps = panthor_vm_get_heap_pool(group->vm, false); heap_address = cs_iface->output->heap_address; Could this dereference the unmapped cs_iface->output memory and cause a use-after-free, since the async worker doesn't seem to check drm_dev_enter()? > disable_work_sync(&sched->sync_upd_work); > > mutex_lock(&sched->lock); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
