[PATCH] drm/amdkfd: Confirm list is non-empty before utilizing list_first_entry in kfd_topology.c

[Lazar, Lijo]

On 12/22/2023 9:44 AM, Srinivasan Shanmugam wrote:
> Before using list_first_entry, make sure to check that list is not
> empty.
> 
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1347 kfd_create_indirect_link_prop() warn: can 'gpu_link' even be NULL?
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1428 kfd_add_peer_prop() warn: can 'iolink1' even be NULL?
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1433 kfd_add_peer_prop() warn: can 'iolink2' even be NULL?
> 
> 'Fixes: 0f28cca87e9a ("drm/amdkfd: Extend KFD device topology to surface
> peer-to-peer links")'
> Suggested-by: Felix Kuehling <Felix.Kuehling at amd.com>
> Cc: Felix Kuehling <Felix.Kuehling at amd.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index dc7c8312e8c7..a4cc88f08df2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1342,10 +1342,9 @@ static int kfd_create_indirect_link_prop(struct kfd_topology_device *kdev, int g
>   		num_cpu++;
>   	}
>   
> -	gpu_link = list_first_entry(&kdev->io_link_props,
> -					struct kfd_iolink_properties, list);
> -	if (!gpu_link)
> -		return -ENOMEM;
> +	if (!list_empty(&kdev->io_link_props))

To match the original code logic, I think it needs to be something like

	if (list_empty(&kdev->io_link_props))
		return -ENOMEM;
(though the choice of this error code is a matter of another patch).

Otherwise, this will end up in accessing uninitialized gpu_link 
variable.  Same is the case for other modifications also.

Thanks,
Lijo

> +		gpu_link = list_first_entry(&kdev->io_link_props,
> +					    struct kfd_iolink_properties, list);
>   
>   	for (i = 0; i < num_cpu; i++) {
>   		/* CPU <--> GPU */
> @@ -1423,15 +1422,13 @@ static int kfd_add_peer_prop(struct kfd_topology_device *kdev,
>   				peer->gpu->adev))
>   		return ret;
>   
> -	iolink1 = list_first_entry(&kdev->io_link_props,
> -							struct kfd_iolink_properties, list);
> -	if (!iolink1)
> -		return -ENOMEM;
> +	if (!list_empty(&kdev->io_link_props))
> +		iolink1 = list_first_entry(&kdev->io_link_props,
> +					   struct kfd_iolink_properties, list);
>   
> -	iolink2 = list_first_entry(&peer->io_link_props,
> -							struct kfd_iolink_properties, list);
> -	if (!iolink2)
> -		return -ENOMEM;
> +	if (!list_empty(&peer->io_link_props))
> +		iolink2 = list_first_entry(&peer->io_link_props,
> +					   struct kfd_iolink_properties, list);
>   
>   	props = kfd_alloc_struct(props);
>   	if (!props)