Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The IH work queues (ih1_work, ih2_work, ih_soft_work) are not cancelled before freeing the IH ring buffers during hardware teardown, leading to a potential use-after-free or NULL pointer dereference. -- commit d96f9c222586fdd4fb9ac2385c6edc335ba11a51 Author: Thadeu Lima de Souza Cascardo <[email protected]> drm/amdgpu: initialize irq.lock spinlock earlier If there is an early failure during amdgpu probe, it will end up calling amdgpu_irq_disable_all, which takes irq.lock spinlock without it being initialized. Initializing irq.lock earlier at amdgpu_device_init fixes the issue. > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > index 82bc6d657e5a0..836ba07713649 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > @@ -309,8 +309,6 @@ int amdgpu_irq_init(struct amdgpu_device *adev) > unsigned int irq, flags; > int r; > > - spin_lock_init(&adev->irq.lock); > - [Severity: High] This isn't a bug introduced by this patch, but looking at the driver teardown, are the IH work queues ever cancelled? In amdgpu_irq_init(), ih1_work, ih2_work, and ih_soft_work are initialized and can be scheduled by the interrupt handler. However, during teardown: drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:amdgpu_irq_fini_hw() { if (adev->irq.installed) { free_irq(adev->irq.irq, adev_to_drm(adev)); adev->irq.installed = false; ... amdgpu_ih_ring_fini(adev, &adev->irq.ih_soft); amdgpu_ih_ring_fini(adev, &adev->irq.ih); amdgpu_ih_ring_fini(adev, &adev->irq.ih1); amdgpu_ih_ring_fini(adev, &adev->irq.ih2); } The driver frees the IRQ and the IH ring buffers, but does not explicitly cancel the pending work items using cancel_work_sync(). If a scheduled work item executes concurrently with or after teardown, could it dereference the freed adev or ih->ring pointers, leading to a use-after-free or NULL pointer dereference? Should the work items be cancelled synchronously before the ring buffers are freed? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
