Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm: NULL pointer dereference in panfrost_perfcnt_sample_done() on spurious interrupt - [Medium] drm/panfrost: perfcnt dump race on timeout or signal --
commit ce948f180a0ee3902b304893abf73fc5060945c5 Author: Adrián Larumbe <[email protected]> drm/panfrost: Avoid cache flush after perfcnt sample in fully coherent systems The commit avoids flushing the GPU caches after a perfcnt sample is made available if the GPU and CPU are fully coherent. > diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > index ffc77121070e2..b3f71d7fd82a9 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > @@ -52,7 +52,10 @@ void panfrost_perfcnt_clean_cache_done(struct > panfrost_device *pfdev) > > void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev) > { > - gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES); > + if (pfdev->features.selected_coherency != COHERENCY_ACE) > + gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES); > + else > + complete(&pfdev->perfcnt->dump_comp); [Severity: High] Could this lead to a NULL pointer dereference if a spurious interrupt arrives early during device initialization? In the initialization sequence, the GPU IRQ handler is registered before the performance counter data structure is allocated: panfrost_device_init() panfrost_gpu_init() devm_request_irq(... panfrost_gpu_irq_handler ...); ... panfrost_perfcnt_init(pfdev); If a spurious interrupt or unhandled prior hardware state triggers the IRQ handler before panfrost_perfcnt_init() completes, the handler can call panfrost_perfcnt_sample_done(). On fully coherent systems, this would now dereference pfdev->perfcnt before it is allocated. [Severity: Medium] Does this completion handling introduce a race condition if the ioctl wait is interrupted? When userspace requests a performance counter dump, it eventually waits for the completion to be signaled: panfrost_perfcnt_dump_locked() ... reinit_completion(&pfdev->perfcnt->dump_comp); ... wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp, ...) If this wait is interrupted by a signal or times out, it leaves the GPU command in-flight. If userspace immediately issues a new dump request, reinit_completion() is called while the previous request is still pending. When the GPU completes the original request and asserts the interrupt, it will call complete() here, which immediately satisfies the new wait. Will this cause the ioctl to return a premature success to userspace while the GPU is still actively writing the new sample, leading to corrupted profiling statistics? > } > > static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev) -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
