AMD General

> -----Original Message-----
> From: Kamal, Asad <[email protected]>
> Sent: Monday, July 13, 2026 12:29 PM
> To: [email protected]
> Cc: Lazar, Lijo <[email protected]>; Zhang, Hawking
> <[email protected]>; Ma, Le <[email protected]>; Zhang, Morris
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Wang, Yang(Kevin)
> <[email protected]>; Kamal, Asad <[email protected]>; John
> Olender <[email protected]>
> Subject: [PATCH] drm/amd/pm: Fix pp_entries_max() bios check
>
> Commit 3cfe43363050 ("drm/amd/pm: Use uploaded size for legacy custom
> PPTable") changed pp_dpm_set_pp_table() to kmemdup the uploaded buffer
> directly and set soft_pp_table_size to the uploaded size.  As a result
> soft_pp_table now points to an allocation completely outside adev->bios,
> making the unconditional pp_end > bios_end check in pp_entries_max()
> always true for custom PP tables — silently returning 0 and breaking PP table
> overrides via sysfs.
>
> Fix this by conditioning the BIOS containment check on hardcode_pp_table
> being NULL.  hardcode_pp_table is zero-initialised
> (kzalloc) and only set when a custom table is uploaded via sysfs, so:
>
>   - hardcode_pp_table == NULL: VBIOS path — enforce pp_end <= bios_end
>     to reject a malicious VBIOS inflating usStructureSize past the BIOS
>     image.
>
>   - hardcode_pp_table != NULL: custom upload path — skip the bios_end
>     check, soft_pp_table_size is the kernel-supplied upload size and is
>     already trusted.
>
> Fixes: e30b3e3ab51a ("drm/amdgpu/pm: add pp_entries_max() helper")
> Reported-by: John Olender <[email protected]>
> Signed-off-by: Asad Kamal <[email protected]>
> ---
>  drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
> b/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
> index 7ebc1344023f..a23e01921842 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
> @@ -833,14 +833,17 @@ static inline uint32_t pp_entries_max(const struct
> pp_hwmgr *hwmgr,
>                                     const void *sub_table,
>                                     size_t hdr_size, size_t rec_size)  {
> -     struct amdgpu_device *adev = (struct amdgpu_device *)hwmgr-
> >adev;
> -     const char *bios_end = (const char *)adev->bios + adev->bios_size;
> -     const char *pp_end   = (const char *)hwmgr->soft_pp_table
> -                            + hwmgr->soft_pp_table_size;
> +     const char *pp_start = (const char *)hwmgr->soft_pp_table;
> +     const char *pp_end   = pp_start + hwmgr->soft_pp_table_size;
>       const char *entries  = (const char *)sub_table + hdr_size;
>
> -     if (pp_end > bios_end)
> -             return 0;
> +     if (!hwmgr->hardcode_pp_table) {
> +             struct amdgpu_device *adev = (struct amdgpu_device
> *)hwmgr->adev;
> +             const char *bios_end = (const char *)adev->bios + adev-
> >bios_size;

The two explicit type casts are unnecessary as I know; please double-check this 
part.

Apart is LGTM , the patch is
Reviewed-by: Yang Wang <[email protected]>

Best Regards,
Kevin
> +
> +             if (pp_end > bios_end)
> +                     return 0;
> +     }
>       if (!rec_size || entries >= pp_end)
>               return 0;
>       return (uint32_t)((pp_end - entries) / rec_size);
> --
> 2.46.0

Reply via email to