This code has two bugs.  If kfd_topology_device_by_proximity_domain()
failed on the first iteration through the loop then "cpu_link" is
uninitialized and should not be dereferenced.

The second bug is that we cannot dereference a list iterator when it
points to the list head.  In other words, if we exit the
list_for_each_entry() loop exits without hitting a break then "cpu_link"
is not a valid pointer and should not be dereferenced.

Fix both of these problems by setting "cpu_link" to NULL when it is invalid
and non-NULL when it is valid.  That makes it easier to test for
valid vs invalid.

Fixes: 0f28cca87e9a ("drm/amdkfd: Extend KFD device topology to surface 
peer-to-peer links")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
I reported these in June but never heard back.

 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 25990bec600d..3f0a4a415907 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1392,8 +1392,8 @@ static int kfd_build_p2p_node_entry(struct 
kfd_topology_device *dev,
 
 static int kfd_create_indirect_link_prop(struct kfd_topology_device *kdev, int 
gpu_node)
 {
+       struct kfd_iolink_properties *gpu_link, *tmp_link, *cpu_link;
        struct kfd_iolink_properties *props = NULL, *props2 = NULL;
-       struct kfd_iolink_properties *gpu_link, *cpu_link;
        struct kfd_topology_device *cpu_dev;
        int ret = 0;
        int i, num_cpu;
@@ -1416,16 +1416,19 @@ static int kfd_create_indirect_link_prop(struct 
kfd_topology_device *kdev, int g
                        continue;
 
                /* find CPU <-->  CPU links */
+               cpu_link = NULL;
                cpu_dev = kfd_topology_device_by_proximity_domain(i);
                if (cpu_dev) {
-                       list_for_each_entry(cpu_link,
+                       list_for_each_entry(tmp_link,
                                        &cpu_dev->io_link_props, list) {
-                               if (cpu_link->node_to == gpu_link->node_to)
+                               if (tmp_link->node_to == gpu_link->node_to) {
+                                       cpu_link = tmp_link;
                                        break;
+                               }
                        }
                }
 
-               if (cpu_link->node_to != gpu_link->node_to)
+               if (!cpu_link)
                        return -ENOMEM;
 
                /* CPU <--> CPU <--> GPU, GPU node*/
-- 
2.35.1

Reply via email to