On surprise removal the PCI core runs amdgpu_device_fini_hw(), which
calls amdgpu_amdkfd_device_fini_sw() as soon as pci_dev_is_disconnected()
is set. That frees kfd.dev - including the kfd_processes_count it
embeds - before amdgpu_amdkfd_teardown_processes() in
amdgpu_device_ip_fini_early() ever runs. The unconditional
adev->kfd.dev->kfd_processes_count dereference in the wait loop then
crashes the pciehp removal thread (observed as a +0x1b4 NULL deref on
MI50 Thunderbolt unplug).
Move the process teardown ahead of the early fini_sw() on the
disconnected path so that terminate events are sent and the drain wait
runs while kfd.dev is still valid. Keep a NULL guard in
kgd2kfd_teardown_processes() for the second invocation from
ip_fini_early, where kfd.dev is already NULL.
The wait itself stays unbounded: returning while processes still
reference kfd.dev would just trade the NULL deref for a deferred
use-after-free in kfd_process_remove_pdd() when it decrements
kfd_processes_count on freed memory.
Fixes: 28695ca09d32 ("drm/amd: Clean up kfd node on surprise disconnect")
Fixes: d12d05c4bc4c ("drm/amdkfd: Check if there are kfd porcesses using adev
by kfd_processes_count")
Cc: Xiaogang Chen <[email protected]>
Cc: <[email protected]>
Signed-off-by: Olivier Kaloudoff <[email protected]>
---
Changes in v3:
- drop the 30s bound; the drain wait is unbounded again (CK)
- run teardown before the early fini_sw() on the disconnected path so
the wait actually observes kfd_processes_count while kfd.dev is
still valid; NULL guard kept for the second invocation from
ip_fini_early
- v1 took a bounded-wait approach; v2 of this patch was never posted
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 ++++-
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 9 ++++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9269e78..bd9d73a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4315,8 +4315,11 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
* before ip_fini_early to prevent kfd locking refcount issues by
calling
* amdgpu_amdkfd_suspend()
*/
- if (pci_dev_is_disconnected(adev->pdev))
+ if (pci_dev_is_disconnected(adev->pdev)) {
+ /* Drain KFD processes while kfd.dev is still valid */
+ amdgpu_amdkfd_teardown_processes(adev);
amdgpu_amdkfd_device_fini_sw(adev);
+ }
amdgpu_coredump_fini(adev);
amdgpu_device_ip_fini_early(adev);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 586e640..43029b2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -1801,7 +1801,14 @@ void kgd2kfd_teardown_processes(struct amdgpu_device
*adev)
mutex_unlock(&kfd_processes_mutex);
- /* wait all kfd processes use adev terminate */
+ /* kfd.dev may already be freed on surprise removal */
+ if (!adev->kfd.dev)
+ return;
+
+ /*
+ * Wait for all KFD processes using adev to terminate. Must be
+ * unbounded: device state is freed once they are gone.
+ */
while (!!atomic_read(&adev->kfd.dev->kfd_processes_count))
cond_resched();
}
--
2.47.3
