Re: [PATCH] drm/amd: fix potential memleak in err branch

[Felix Kuehling]

Hi Bernard,

I just applied a patch from another contributor for the kfd_topology
part of this. See
https://cgit.freedesktop.org/~agd5f/linux/commit/?h=amd-staging-drm-next&id=fc0fe8138309fee303bd12991f12b23f01bbf58c

Please rebase your patch on that. I believe that should only leave the
fixes in kfd_process.c.

Thanks,
 Felix

Am 2020-06-19 um 7:45 a.m. schrieb Bernard Zhao:
> The function kobject_init_and_add alloc memory like:
> kobject_init_and_add->kobject_add_varg->kobject_set_name_vargs
> ->kvasprintf_const->kstrdup_const->kstrdup->kmalloc_track_caller
> ->kmalloc_slab, in err branch this memory not free. If use
> kmemleak, this path maybe catched.
> These changes are to add kobject_put in kobject_init_and_add
> failed branch, fix potential memleak.
>
> Signed-off-by: Bernard Zhao <bernard@xxxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c  |  2 ++
>  drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 20 +++++++++++++++-----
>  2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index d27221ddcdeb..5ee4d6cfb16d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -124,6 +124,7 @@ void kfd_procfs_init(void)
>  	if (ret) {
>  		pr_warn("Could not create procfs proc folder");
>  		/* If we fail to create the procfs, clean up */
> +		kobject_put(procfs.kobj);
>  		kfd_procfs_shutdown();
>  	}
>  }
> @@ -428,6 +429,7 @@ struct kfd_process *kfd_create_process(struct file *filep)
>  					   (int)process->lead_thread->pid);
>  		if (ret) {
>  			pr_warn("Creating procfs pid directory failed");
> +			kobject_put(process->kobj);
>  			goto out;
>  		}
>  
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index bb77f7af2b6d..dc3c4149f860 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -632,8 +632,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev,
>  
>  	ret = kobject_init_and_add(dev->kobj_node, &node_type,
>  			sys_props.kobj_nodes, "%d", id);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		kobject_put(dev->kobj_node);
>  		return ret;
> +	}
>  
>  	dev->kobj_mem = kobject_create_and_add("mem_banks", dev->kobj_node);
>  	if (!dev->kobj_mem)
> @@ -680,8 +682,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev,
>  			return -ENOMEM;
>  		ret = kobject_init_and_add(mem->kobj, &mem_type,
>  				dev->kobj_mem, "%d", i);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			kobject_put(mem->kobj);
>  			return ret;
> +		}
>  
>  		mem->attr.name = "properties";
>  		mem->attr.mode = KFD_SYSFS_FILE_MODE;
> @@ -699,8 +703,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev,
>  			return -ENOMEM;
>  		ret = kobject_init_and_add(cache->kobj, &cache_type,
>  				dev->kobj_cache, "%d", i);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			kobject_put(cache->kobj);
>  			return ret;
> +		}
>  
>  		cache->attr.name = "properties";
>  		cache->attr.mode = KFD_SYSFS_FILE_MODE;
> @@ -718,8 +724,10 @@ static int kfd_build_sysfs_node_entry(struct kfd_topology_device *dev,
>  			return -ENOMEM;
>  		ret = kobject_init_and_add(iolink->kobj, &iolink_type,
>  				dev->kobj_iolink, "%d", i);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			kobject_put(iolink->kobj);
>  			return ret;
> +		}
>  
>  		iolink->attr.name = "properties";
>  		iolink->attr.mode = KFD_SYSFS_FILE_MODE;
> @@ -798,8 +806,10 @@ static int kfd_topology_update_sysfs(void)
>  		ret = kobject_init_and_add(sys_props.kobj_topology,
>  				&sysprops_type,  &kfd_device->kobj,
>  				"topology");
> -		if (ret < 0)
> +		if (ret < 0) {
> +			kobject_put(sys_props.kobj_topology);
>  			return ret;
> +		}
>  
>  		sys_props.kobj_nodes = kobject_create_and_add("nodes",
>  				sys_props.kobj_topology);