On Wed, Jun 10, 2020 at 6:57 AM Quan, Evan <evan.q...@amd.com> wrote:
>
> [AMD Official Use Only - Internal Distribution Only]
>
> Reviewed-by: Evan Quan <evan.q...@amd.com>
Applied. Thanks!
Alex
>
> -----Original Message-----
> From: Dan Carpenter <dan.carpen...@oracle.com>
> Sent: Wednesday, June 10, 2020 4:57 PM
> To: Deucher, Alexander <alexander.deuc...@amd.com>
> Cc: Koenig, Christian <christian.koe...@amd.com>; David Airlie
> <airl...@linux.ie>; Daniel Vetter <dan...@ffwll.ch>; Quan, Evan
> <evan.q...@amd.com>; Zhang, Hawking <hawking.zh...@amd.com>; Kuehling, Felix
> <felix.kuehl...@amd.com>; Zhou1, Tao <tao.zh...@amd.com>; Ma, Le
> <le...@amd.com>; Xiao, Jack <jack.x...@amd.com>; Kim, Jonathan
> <jonathan....@amd.com>; Greathouse, Joseph <joseph.greatho...@amd.com>;
> amd-gfx@lists.freedesktop.org; kernel-janit...@vger.kernel.org
> Subject: [PATCH v2] drm/amdgpu: Fix a buffer overflow handling the serial
> number
>
> The comments say that the serial number is a 16-digit HEX string so the
> buffer needs to be at least 17 characters to hold the NUL terminator.
>
> The other issue is that "size" returned from sprintf() is the number of
> characters before the NUL terminator so the memcpy() wasn't copying the
> terminator. The serial number needs to be NUL terminated so that it
> doesn't lead to a read overflow in amdgpu_device_get_serial_number().
> Also it's just cleaner and faster to sprintf() directly to adev->serial[]
> instead of using a temporary buffer.
>
> Fixes: 81a16241114b ("drm/amdgpu: Add unique_id and serial_number for
> Arcturus v3")
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> ---
> v2: Change adev->serial. The original patch would have just moved the
> overflow until amdgpu_device_get_serial_number() is called.
>
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 6 ++----
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 135530286f34f..905cf0bac100c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -986,7 +986,7 @@ struct amdgpu_device {
> /* Chip product information */
> charproduct_number[16];
> charproduct_name[32];
> -charserial[16];
> +charserial[20];
>
> struct amdgpu_autodumpautodump;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index df7b408319f76..d58146a5fa21d 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -2265,8 +2265,7 @@ static void arcturus_i2c_eeprom_control_fini(struct
> i2c_adapter *control)
> static void arcturus_get_unique_id(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> -uint32_t top32, bottom32, smu_version, size;
> -char sn[16];
> +uint32_t top32, bottom32, smu_version;
> uint64_t id;
>
> if (smu_get_smc_version(smu, NULL, &smu_version)) {
> @@ -2289,8 +2288,7 @@ static void arcturus_get_unique_id(struct smu_context
> *smu)
> /* For Arcturus-and-later, unique_id == serial_number, so convert it to a
> * 16-digit HEX string for convenience and backwards-compatibility
> */
> -size = sprintf(sn, "%llx", id);
> -memcpy(adev->serial, &sn, size);
> +sprintf(adev->serial, "%llx", id);
> }
>
> static bool arcturus_is_baco_supported(struct smu_context *smu)
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
