[PATCH v2 1/6] amdgpu: fix possible resource leakage in kfd_cleanup_nodes()

[Chen, Xiaogang]

On 1/4/2025 8:45 PM, Jiang Liu wrote:
> Fix possible resource leakage on error recovery path in function
> kgd2kfd_device_init().
>
> Signed-off-by: Jiang Liu<gerry at linux.alibaba.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index a29374c86405..fa5054940486 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -898,15 +898,15 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>   		if (kfd->adev->xcp_mgr)
>   			kfd_setup_interrupt_bitmap(node, i);
>   
> +		spin_lock_init(&node->watch_points_lock);
> +
> +		kfd->nodes[i] = node;
> +
>   		/* Initialize the KFD node */
>   		if (kfd_init_node(node)) {
>   			dev_err(kfd_device, "Error initializing KFD node\n");
>   			goto node_init_error;
>   		}
> -
> -		spin_lock_init(&node->watch_points_lock);
> -
> -		kfd->nodes[i] = node;
>   	}
>   
>   	svm_range_set_max_pages(kfd->adev);
> @@ -921,6 +921,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>   	goto out;
>   
>   node_init_error:
> +	i++;
>   node_alloc_error:
>   	kfd_cleanup_nodes(kfd, i);
>   	kfd_doorbell_fini(kfd);

I think this change is not right: if kfd_init_node fail it does clean up 
for the kfd_node that it is initializing internally. kfd_cleanup_nodes 
does not need to clean up the kfd node i which failed to init, just 
clean up the kfd_nodes that were initialized previously.

Regard

Xiaogang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20250107/d0054341/attachment.htm>