Re: [PATCH v4 13/13] drm/panfrost: Bump driver minor to reflect new DUMP IOCTL req field

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The error path in `panfrost_perfcnt_enable_locked` incorrectly sets `perfcnt->user = user` instead of `NULL`, leading to a corrupt state and Use-After-Free.
- [Medium] Hardware reset notification is permanently lost if the ioctl wait is interrupted by a signal.
--

--- Patch [13]: [PATCH v4 13/13] drm/panfrost: Bump driver minor to reflect new DUMP IOCTL req field ---
commit 33e476c0f2af99299bbeb687048c33539616ce05
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Fix races between perfcnt and reset sequence

This commit fixes races between the reset sequence and perfcnt session
teardown. It uses a read/write semaphore to serialize access to the
hardware during resets and updates the DRM uAPI to notify userspace of
resets during a perfcnt DUMP operation.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index ad1156678e91c..b92ad90618931 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
[ ... ]
> @@ -55,38 +59,97 @@ void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
>  	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
>  }
>  
> -static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)
> +static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev,
> +					u64 *reset_happened)
>  {
> -	u64 gpuva;
> +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> +	u64 gpuva = perfcnt->mapping->mmnode.start << PAGE_SHIFT;
> +	u64 retries = PERFCNT_DUMP_MAX_RETRIES;
>  	int ret;
>  
> -	reinit_completion(&pfdev->perfcnt->dump_comp);
> -	gpuva = pfdev->perfcnt->mapping->mmnode.start << PAGE_SHIFT;
> -	gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
> -	gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
> -	gpu_write(pfdev, GPU_INT_CLEAR,
> -		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> -		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> -	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
> +dump_retry:
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		*reset_happened = perfcnt->reset_happened;
> +		perfcnt->reset_happened = false;

[Severity: Medium]
Does this code permanently lose the hardware reset notification if the ioctl
wait is interrupted by a signal?

Since the internal state is cleared prior to the interruptible wait and not
restored on the -ERESTARTSYS error path, could a restarted syscall overwrite
req->hw_reset with the cleared value, causing userspace to accumulate
counters that were actually reset?

> +		reinit_completion(&pfdev->perfcnt->dump_comp);
> +		gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
> +		gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
> +		gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_CLEAN_CACHES_COMPLETED |
> +						GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> +		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
> +	}
> +
>  	ret = wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp,
>  							msecs_to_jiffies(1000));
[ ... ]
> @@ -127,60 +192,46 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
[ ... ]
>  	/* The BO ref is retained by the mapping. */
>  	drm_gem_object_put(&bo->base);
>  
> -	perfcnt->user = user;
> -
>  	return 0;
>  
>  err_vunmap:
> +	scoped_guard(rwsem_read, &pfdev->reset.lock)
> +		perfcnt->user = user;

[Severity: Critical]
Is it possible this error path incorrectly reassigns perfcnt->user to user
instead of setting it to NULL?

If panfrost_perfcnt_hw_enable() fails (e.g. when panfrost_mmu_as_get fails
because address spaces are exhausted), could this leave perfcnt->user set
while the backing resources are freed, possibly leading to a use-after-free
on subsequent ioctls?

>  	drm_gem_vunmap(&bo->base, &map);
>  err_put_mapping:
>  	panfrost_gem_mapping_put(perfcnt->mapping);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-claude-fixes-v4-0-01968f2ec77a@collabora.com?part=13