[PATCH v2] drm/amdkfd: Dereference null return value

[Russell, Kent]

[Public]



________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Andrew Martin <Andrew.Martin at amd.com>
Sent: Monday, December 2, 2024 7:45:55 a.m.
To: amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Kuehling, Felix <Felix.Kuehling at amd.com>; Tudor, Alexandru <Alexandru.Tudor at amd.com>; Martin, Andrew <Andrew.Martin at amd.com>; Martin, Andrew <Andrew.Martin at amd.com>
Subject: [PATCH v2] drm/amdkfd: Dereference null return value

In the function pqm_uninit there is a call-assignment of "pdd =
kfd_get_process_device_data" which could be null, and this value was
later dereferenced without checking.

Fixes: fb91065851cd ("drm/amdkfd: Refactor queue wptr_bo GART mapping")
Signed-off-by: Andrew Martin <Andrew.Martin at amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index c76db22a1000..89aa578f6c68 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -212,11 +212,13 @@ static void pqm_clean_queue_resource(struct process_queue_manager *pqm,
 void pqm_uninit(struct process_queue_manager *pqm)
 {
         struct process_queue_node *pqn, *next;
-       struct kfd_process_device *pdd;

         list_for_each_entry_safe(pqn, next, &pqm->queues, process_queue_list) {
                 if (pqn->q) {
-                       pdd = kfd_get_process_device_data(pqn->q->device, pqm->process);
+                       struct kfd_process_device *pdd = kfd_get_process_device_data(pqn->q->device,
+                                                                                    pqm->process);
+                       if (WARN_ON(!pdd))

Would we want a "continue" instead of "break" if the pdd is NULL? Just in case other ones in the list are still valid? Or is one NULL enough to just WARN and abort?

 Kent

+                               return;
                         kfd_queue_unref_bo_vas(pdd, &pqn->q->properties);
                         kfd_queue_release_buffers(pdd, &pqn->q->properties);
                         pqm_clean_queue_resource(pqm, pqn);
--
2.43.0


-------------- next part --------------
A non-text attachment was scrubbed...
Name: winmail.dat
Type: application/ms-tnef
Size: 21353 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20241203/f0dbf853/attachment-0001.bin>