AMD General

The series is: Reviewed-by: Tao Zhou <[email protected]>

> -----Original Message-----
> From: Yang, Stanley <[email protected]>
> Sent: Monday, May 18, 2026 5:40 PM
> To: [email protected]
> Cc: Zhang, Hawking <[email protected]>; Zhou1, Tao
> <[email protected]>; Chai, Thomas <[email protected]>; Li, Candice
> <[email protected]>; Yang, Stanley <[email protected]>
> Subject: [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded
> helpers
>
> Replace the open-coded TLV walk with fru_pia_advance() and
> fru_pia_copy_field() helpers that bound every read by the actual EEPROM
> data length, preventing out-of-bounds reads on truncated or malformed FRU
> data.
>
> Signed-off-by: Stanley.Yang <[email protected]>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c    | 95 ++++++++++++-------
>  1 file changed, 63 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index c5178e2b794d..86b2d5a79993 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -115,6 +115,43 @@ static bool is_fru_eeprom_supported(struct
> amdgpu_device *adev, u32 *fru_addr)
>       }
>  }
>
> +/*
> + * IPMI FRU Product Info Area fields are TLV: one type/length byte
> + * (low 6 bits = data length) followed by that many data bytes. These
> + * helpers walk the cursor and copy a single field while bounding all
> + * accesses to the actual buffer length read from the EEPROM.
> + */
> +#define FRU_FIELD_LEN(p, a)  ((p)[a] & 0x3F)
> +
> +/* Advance cursor past the current TLV. Returns false if no more data.
> +*/ static bool fru_pia_advance(u32 *addr, const unsigned char *pia, int
> +len) {
> +     if (*addr >= (u32)len)
> +             return false;
> +     *addr += 1 + FRU_FIELD_LEN(pia, *addr);
> +     return true;
> +}
> +
> +/*
> + * Copy the current TLV's data into dst (NUL-terminated). Returns false
> +if
> + * the TLV header or data would read past the end of pia.
> + */
> +static bool fru_pia_copy_field(char *dst, size_t dst_size,
> +                            const unsigned char *pia, u32 addr, int len) {
> +     size_t fl;
> +
> +     if (addr + 1 >= (u32)len)
> +             return false;
> +
> +     fl = min3((size_t)FRU_FIELD_LEN(pia, addr),
> +                       dst_size -1,
> +                       (size_t)(len - addr - 1));
> +     memcpy(dst, pia + addr + 1, fl);
> +     dst[fl] = '\0';
> +     return true;
> +}
> +
>  int amdgpu_fru_get_product_info(struct amdgpu_device *adev)  {
>       struct amdgpu_fru_info *fru_info;
> @@ -223,52 +260,46 @@ int amdgpu_fru_get_product_info(struct
> amdgpu_device *adev)
>        * Read Manufacturer Name field whose length is [3].
>        */
>       addr = 3;
> -     if (addr + 1 >= len)
> +     if (!fru_pia_copy_field(fru_info->manufacturer_name,
> +                             sizeof(fru_info->manufacturer_name),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->manufacturer_name, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->manufacturer_name),
> -                  pia[addr] & 0x3F));
> -     fru_info->manufacturer_name[sizeof(fru_info->manufacturer_name)
> - 1] =
> -             '\0';
>
>       /* Read Product Name field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if (addr + 1 >= len)
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->product_name,
> +                             sizeof(fru_info->product_name),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->product_name, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->product_name), pia[addr] & 0x3F));
> -     fru_info->product_name[sizeof(fru_info->product_name) - 1] = '\0';
>
>       /* Go to the Product Part/Model Number field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if (addr + 1 >= len)
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->product_number,
> +                             sizeof(fru_info->product_number),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->product_number, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->product_number),
> -                  pia[addr] & 0x3F));
> -     fru_info->product_number[sizeof(fru_info->product_number) - 1] =
> '\0';
>
> -     /* Go to the Product Version field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> +     /* Skip the Product Version field. */
> +     if (!fru_pia_advance(&addr, pia, len))
> +             goto Out;
>
> -     /* Go to the Product Serial Number field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if (addr + 1 >= len)
> +     /* Read the Product Serial Number field. */
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->serial,
> +                             sizeof(fru_info->serial),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->serial, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->serial), pia[addr] & 0x3F));
> -     fru_info->serial[sizeof(fru_info->serial) - 1] = '\0';
>
> -     /* Asset Tag field */
> -     addr += 1 + (pia[addr] & 0x3F);
> +     /* Skip the Asset Tag field. */
> +     if (!fru_pia_advance(&addr, pia, len))
> +             goto Out;
>
>       /* FRU File Id field. This could be 'null'. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if ((addr + 1 >= len) || !(pia[addr] & 0x3F))
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->fru_id,
> +                             sizeof(fru_info->fru_id),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->fru_id, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->fru_id), pia[addr] & 0x3F));
> -     fru_info->fru_id[sizeof(fru_info->fru_id) - 1] = '\0';
>
>  Out:
>       kfree(pia);
> --
> 2.43.0

Reply via email to