[AMD Official Use Only] Please ignore the patch, will resend after removing multiple assignments. -----Original Message----- From: Jiansong Chen <Jiansong.Chen@xxxxxxx> Sent: Tuesday, May 25, 2021 2:17 PM To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Chen, Jiansong (Simon) <Jiansong.Chen@xxxxxxx> Subject: [PATCH] drm/amdgpu: refine amdgpu_fru_get_product_info 1. eliminate potential array index out of bounds. 2. return meaningful value for failure. Signed-off-by: Jiansong Chen <Jiansong.Chen@xxxxxxx> Change-Id: I9be36eb2e42ee46cd00464b0f2c35a4e4ea213e3 --- .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 69 ++++++++++--------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c index 8f4a8f8d8146..5c2b4403a5b6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c @@ -101,7 +101,8 @@ static int amdgpu_fru_read_eeprom(struct amdgpu_device *adev, uint32_t addrptr, int amdgpu_fru_get_product_info(struct amdgpu_device *adev) { unsigned char buff[34]; - int addrptr = 0, size = 0; + int addrptr, size; + int len; if (!is_fru_eeprom_supported(adev)) return 0; @@ -109,7 +110,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev) /* If algo exists, it means that the i2c_adapter's initialized */ if (!adev->pm.smu_i2c.algo) { DRM_WARN("Cannot access FRU, EEPROM accessor not initialized"); - return 0; + return -ENODEV; } /* There's a lot of repetition here. This is due to the FRU having @@ -125,75 +126,75 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev) * and the language field, so just start from 0xb, manufacturer size */ addrptr = 0xb; - size = amdgpu_fru_read_eeprom(adev, addrptr, buff); - if (size < 1) { - DRM_ERROR("Failed to read FRU Manufacturer, ret:%d", size); - return size; + len = size = amdgpu_fru_read_eeprom(adev, addrptr, buff); + if (len < 1) { + DRM_ERROR("Failed to read FRU Manufacturer, ret:%d", len); + return -EINVAL; } /* Increment the addrptr by the size of the field, and 1 due to the * size field being 1 byte. This pattern continues below. */ addrptr += size + 1; - size = amdgpu_fru_read_eeprom(adev, addrptr, buff); - if (size < 1) { - DRM_ERROR("Failed to read FRU product name, ret:%d", size); - return size; + len = size = amdgpu_fru_read_eeprom(adev, addrptr, buff); + if (len < 1) { + DRM_ERROR("Failed to read FRU product name, ret:%d", len); + return -EINVAL; } /* Product name should only be 32 characters. Any more, * and something could be wrong. Cap it at 32 to be safe */ - if (size > 32) { + if (len >= sizeof(adev->product_name)) { DRM_WARN("FRU Product Number is larger than 32 characters. This is likely a mistake"); - size = 32; + len = sizeof(adev->product_name) - 1; } /* Start at 2 due to buff using fields 0 and 1 for the address */ - memcpy(adev->product_name, &buff[2], size); - adev->product_name[size] = '\0'; + memcpy(adev->product_name, &buff[2], len); + adev->product_name[len] = '\0'; addrptr += size + 1; - size = amdgpu_fru_read_eeprom(adev, addrptr, buff); - if (size < 1) { - DRM_ERROR("Failed to read FRU product number, ret:%d", size); - return size; + len = size = amdgpu_fru_read_eeprom(adev, addrptr, buff); + if (len < 1) { + DRM_ERROR("Failed to read FRU product number, ret:%d", len); + return -EINVAL; } /* Product number should only be 16 characters. Any more, * and something could be wrong. Cap it at 16 to be safe */ - if (size > 16) { + if (len >= sizeof(adev->product_number)) { DRM_WARN("FRU Product Number is larger than 16 characters. This is likely a mistake"); - size = 16; + len = sizeof(adev->product_number) - 1; } - memcpy(adev->product_number, &buff[2], size); - adev->product_number[size] = '\0'; + memcpy(adev->product_number, &buff[2], len); + adev->product_number[len] = '\0'; addrptr += size + 1; - size = amdgpu_fru_read_eeprom(adev, addrptr, buff); + len = size = amdgpu_fru_read_eeprom(adev, addrptr, buff); - if (size < 1) { - DRM_ERROR("Failed to read FRU product version, ret:%d", size); - return size; + if (len < 1) { + DRM_ERROR("Failed to read FRU product version, ret:%d", len); + return -EINVAL; } addrptr += size + 1; - size = amdgpu_fru_read_eeprom(adev, addrptr, buff); + len = size = amdgpu_fru_read_eeprom(adev, addrptr, buff); - if (size < 1) { - DRM_ERROR("Failed to read FRU serial number, ret:%d", size); - return size; + if (len < 1) { + DRM_ERROR("Failed to read FRU serial number, ret:%d", len); + return -EINVAL; } /* Serial number should only be 16 characters. Any more, * and something could be wrong. Cap it at 16 to be safe */ - if (size > 16) { + if (len >= sizeof(adev->serial)) { DRM_WARN("FRU Serial Number is larger than 16 characters. This is likely a mistake"); - size = 16; + len = sizeof(adev->serial) - 1; } - memcpy(adev->serial, &buff[2], size); - adev->serial[size] = '\0'; + memcpy(adev->serial, &buff[2], len); + adev->serial[len] = '\0'; return 0; } -- 2.25.1 _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx