[PATCH] drm/amdgpu: fix memory leak in amdgpu_fru_get_product_info()

[Luben Tuikov]

On 2023-09-22 01:27, Yang Wang wrote:
> fix a memory leak that occurs when csum is 0,
> the origin function will return directly and forgets to free 'pia' resource.
> 
> Fixes: 0dbf2c562625 ("drm/amdgpu: Interpret IPMI data for product information (v2)")
> 
> CC: Luben Tuikov <luben.tuikov at amd.com>
> Signed-off-by: Yang Wang <kevinyang.wang at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index 401651f28ba2..50b6eb447726 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -111,7 +111,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  {
>  	unsigned char buf[8], *pia;
>  	u32 addr, fru_addr;
> -	int size, len;
> +	int size, len, ret = 0;
>  	u8 csum;
>  
>  	if (!is_fru_eeprom_supported(adev, &fru_addr))
> @@ -171,16 +171,17 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  	/* Read the whole PIA. */
>  	len = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addr, pia, size);
>  	if (len != size) {
> -		kfree(pia);
>  		DRM_ERROR("Couldn't read the Product Info Area: %d", len);
> -		return len < 0 ? len : -EIO;
> +		ret = len < 0 ? len : -EIO;
> +		goto Out;
>  	}
>  
>  	for (csum = 0; size > 0; size--)
>  		csum += pia[size - 1];
>  	if (csum) {
>  		DRM_ERROR("Bad Product Info Area checksum: 0x%02x", csum);
> -		return -EIO;
> +		ret = -EIO;

Actually the memory leak is ONLY here.

I wonder if you could've simply added a

		kfree(pia);

> +		goto Out;
>  	}

before the goto Out; which would result in a one-line change.

Yeah, please do that instead.

So, don't add "ret" and what not. Just add the one liner "kfree(pia);" before
the "goto Out;" and make it a SINGLE-LINE change to fix this memory leak.

You don't need so many (formulaic) changes of adding "ret" and what not.
Just a one-liner change, please.

>  
>  	/* Now extract useful information from the PIA.
> @@ -220,7 +221,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  	adev->serial[sizeof(adev->serial) - 1] = '\0';
>  Out:
>  	kfree(pia);
> -	return 0;
> +	return ret;
>  }
>  
>  /**

-- 
Regards,
Luben