Re: [PATCH v14 13/15] drm/panfrost: Avoid cache flush after perfcnt sample in fully coherent systems

[sashiko-bot@kernel.org]

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 <adrian.larumbe@collabora.com>

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/20261009-claude-fixes-v14-0-cb743b3b973b@collabora.com?part=13