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

[Gerry Liu <gerry@xxxxxxxxxxxxxxxxx>]

2025年1月5日 13:22，Shuo Liu <shuox.liu@xxxxxxxxxxxxxxxxx> 写道：

Hi Gerry,

On Sun  5.Jan'25 at 10:45:29 +0800, Jiang Liu wrote:

Fix possible resource leakage on error recovery path in function
kgd2kfd_device_init().

Signed-off-by: Jiang Liu <gerry@xxxxxxxxxxxxxxxxx>
---
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++;

The err cleanup path can release node itself. So the following
kfd_cleanup_nodes() may do a double free? 

Hi Shuo,

I missed the `kfree()` in function kfd_init_node(), so this patch is wrong and should be dropped.

Thanks,

Gerry

node_alloc_error:
kfd_cleanup_nodes(kfd, i);
kfd_doorbell_fini(kfd);

shuo