Re: [PATCH] drm/amdgpu: Fix error handling in amdgpu_xcp_cfg_sysfs_init()

[Lazar, Lijo]

On 28-Apr-26 5:15 PM, Guangshuo Li wrote:
> [You don't often get email from lgs201920130244@xxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Once kobject_init_and_add() fails for an XCP resource kobject, we
> should call kobject_put() to decrement the reference count for cleanup.
> Otherwise, it could cause a memory leak.
>
> The error handling loop also uses xcp_res[i] instead of xcp_res[j],
> so it fails to put the previously added resource kobjects and may put
> the failed kobject more than once.
>
> Fix this by putting the failed resource kobject before jumping to the
> error path, and by using the correct loop index when putting the
> previously added resource kobjects.
>
> Found by code review.
>
> Fixes: 4ae86dc87850 ("drm/amdgpu: Add sysfs nodes to get xcp details")
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
> index cc5f4e01e38f..315e33a9d7c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
> @@ -948,15 +948,17 @@ static void amdgpu_xcp_cfg_sysfs_init(struct amdgpu_device *adev)
>                                           &xcp_cfg_res_sysfs_ktype,
>                                           &xcp_cfg->kobj, "%s",
>                                           xcp_res_names[rid]);
> -               if (r)
> +               if (r) {
> +                       kobject_put(&xcp_res->kobj);
>                          goto err;
> +               }
>          }
>
>          adev->xcp_mgr->xcp_cfg = xcp_cfg;
>          return;
>   err:
>          for (j = 0; j < i; j++) {
> -               xcp_res = &xcp_cfg->xcp_res[i];
> +               xcp_res = &xcp_cfg->xcp_res[j];

Good catch. What about just keeping it j <= i? Seeing only one path to 
get to this error handling.

Thanks,
Lijo

>                  kobject_put(&xcp_res->kobj);
>          }
>
> --
> 2.43.0