Re: [PATCH AUTOSEL 6.18-6.6] drm/amdgpu: Use system unbound workqueue for soft IH ring

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing work cancellation for soft IH ring (and ih1/ih2) causes use-after-free and NULL pointer dereference on device teardown.
--

commit b817b565b60c4bf0d6715436c240de40fe4ce255
Author: Timur Kristóf <timur.kristof@gmail.com>

drm/amdgpu: Use system unbound workqueue for soft IH ring

This commit allows the kernel to dispatch the soft IH work on other CPUs.
This prevents the soft IH ring from filling up before it starts processing
work items during retry page fault storms.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 8112ffc85995e..2522285e1ab23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -514,7 +514,7 @@ void amdgpu_irq_delegate(struct amdgpu_device *adev,
>  			 unsigned int num_dw)
>  {
>  	amdgpu_ih_ring_write(adev, &adev->irq.ih_soft, entry->iv_entry, num_dw);
> -	schedule_work(&adev->irq.ih_soft_work);
> +	queue_work(system_unbound_wq, &adev->irq.ih_soft_work);

[Severity: High]
This isn't a bug introduced by this patch, but changing the workqueue to
system_unbound_wq removes CPU-binding, which might increase the likelihood
of the work running concurrently with the device teardown thread on SMP
systems.

During teardown, amdgpu_irq_fini_hw() frees the ring buffer and shadow
pointers via amdgpu_ih_ring_fini() without flushing or cancelling the
pending ih_soft_work:

drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:amdgpu_irq_fini_hw() {
	...
	free_irq(adev->irq.irq, adev);
	...
	amdgpu_ih_ring_fini(adev, &adev->irq.ih_soft);
	...
}

If ih_soft_work (amdgpu_ih_process) was preempted right after evaluating
that the device is not yet shutting down, and then resumes after the
teardown path has freed the ring, could it dereference the freed
ih->wptr_cpu and ih->ring?

drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c:amdgpu_ih_process() {
	...
	if (!ih->enabled || adev->shutdown)
		return -ENOENT;
	/* Preempted here, teardown frees the ring */
	wptr = amdgpu_ih_get_wptr(adev, ih);
	...
}

Should there be a cancel_work_sync(&adev->irq.ih_soft_work) or similar
synchronization added during the teardown sequence before the ring
buffers are freed?

>  }
>  
>  /**

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-351-sashal@kernel.org?part=1