[PATCH v1 3/3] drm/amdgpu: Prevent random memory access in FRU code

[Russell, Kent]

One tiny thing to fix before merging

> -----Original Message-----
> From: Kasiviswanathan, Harish <Harish.Kasiviswanathan at amd.com>
> Sent: Friday, February 4, 2022 10:01 AM
> To: Tuikov, Luben <Luben.Tuikov at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Tuikov, Luben
> <Luben.Tuikov at amd.com>; Russell, Kent <Kent.Russell at amd.com>
> Subject: RE: [PATCH v1 3/3] drm/amdgpu: Prevent random memory access in FRU code
> 
> [AMD Official Use Only]
> 
> This series acked-by: Harish Kasiviswanathan <Harish.Kasiviswanathan at amd.com>
> 
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Luben Tuikov
> Sent: Friday, February 4, 2022 12:27 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Tuikov, Luben
> <Luben.Tuikov at amd.com>; Russell, Kent <Kent.Russell at amd.com>
> Subject: [PATCH v1 3/3] drm/amdgpu: Prevent random memory access in FRU code
> 
> Prevent random memory access in the FRU EEPROM code by passing the size of
> the destination buffer to the reading routine, and reading no more than the
> size of the buffer.
> 
> Cc: Kent Russell <kent.russell at amd.com>
> Cc: Alex Deucher <Alexander.Deucher at amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c    | 21 +++++++++++--------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index 61c4e71e399855..07e045fae83a9a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -77,9 +77,10 @@ static bool is_fru_eeprom_supported(struct amdgpu_device *adev)
>  }
> 
>  static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr,
> -				  unsigned char *buf)
> +				  unsigned char *buf, size_t buf_size)
>  {
> -	int ret, size;
> +	int ret;
> +	u8 size;
> 
>  	ret = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addrptr, buf, 1);
>  	if (ret < 1) {
> @@ -90,9 +91,11 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev,
> uint32_t addrptr,
>  	/* The size returned by the i2c requires subtraction of 0xC0 since the
>  	 * size apparently always reports as 0xC0+actual size.
>  	 */
> -	size = buf[0] - I2C_PRODUCT_INFO_OFFSET;


You can also remove this definition from the eeprom.h file, since it's no longer needed.

 Kent
> +	size = buf[0] & 0x3F;
> +	size = min_t(size_t, size, buf_size);
> 
> -	ret = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addrptr + 1, buf, size);
> +	ret = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addrptr + 1,
> +				 buf, size);
>  	if (ret < 1) {
>  		DRM_WARN("FRU: Failed to get data field");
>  		return ret;
> @@ -129,7 +132,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  	 * and the language field, so just start from 0xb, manufacturer size
>  	 */
>  	addrptr = FRU_EEPROM_MADDR + 0xb;
> -	size = amdgpu_fru_read_eeprom(adev, addrptr, buf);
> +	size = amdgpu_fru_read_eeprom(adev, addrptr, buf, sizeof(buf));
>  	if (size < 1) {
>  		DRM_ERROR("Failed to read FRU Manufacturer, ret:%d", size);
>  		return -EINVAL;
> @@ -139,7 +142,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  	 * size field being 1 byte. This pattern continues below.
>  	 */
>  	addrptr += size + 1;
> -	size = amdgpu_fru_read_eeprom(adev, addrptr, buf);
> +	size = amdgpu_fru_read_eeprom(adev, addrptr, buf, sizeof(buf));
>  	if (size < 1) {
>  		DRM_ERROR("Failed to read FRU product name, ret:%d", size);
>  		return -EINVAL;
> @@ -155,7 +158,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  	adev->product_name[len] = '\0';
> 
>  	addrptr += size + 1;
> -	size = amdgpu_fru_read_eeprom(adev, addrptr, buf);
> +	size = amdgpu_fru_read_eeprom(adev, addrptr, buf, sizeof(buf));
>  	if (size < 1) {
>  		DRM_ERROR("Failed to read FRU product number, ret:%d", size);
>  		return -EINVAL;
> @@ -173,7 +176,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  	adev->product_number[len] = '\0';
> 
>  	addrptr += size + 1;
> -	size = amdgpu_fru_read_eeprom(adev, addrptr, buf);
> +	size = amdgpu_fru_read_eeprom(adev, addrptr, buf, sizeof(buf));
> 
>  	if (size < 1) {
>  		DRM_ERROR("Failed to read FRU product version, ret:%d", size);
> @@ -181,7 +184,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
>  	}
> 
>  	addrptr += size + 1;
> -	size = amdgpu_fru_read_eeprom(adev, addrptr, buf);
> +	size = amdgpu_fru_read_eeprom(adev, addrptr, buf, sizeof(buf));
> 
>  	if (size < 1) {
>  		DRM_ERROR("Failed to read FRU serial number, ret:%d", size);
> --
> 2.35.0.3.gb23dac905b