Re: [PATCH] drm/amd/pm: Remove vbios bounds check from pp_entries_max()

[John Olender <john.olender@gmail.com>]

On 7/9/26 5:57 AM, Kamal, Asad wrote:
> AMD General
> 
> Hi @John Olender,
> 
> Good catch. However,  as Alex points out, removing the check entirely drops protection for VBIOS-sourced tables.
> 
> Instead of removing the check, could you condition it on hardcode_pp_table? When it is NULL we are on the VBIOS path and the bios_end check should stay. When it is non-NULL a custom table was uploaded via sysfs and the check can be skipped — soft_pp_table_size is already set to the kernel-supplied upload size.
> 
> if (!hwmgr->hardcode_pp_table) {
>         const char *bios_end = (const char *)adev->bios + adev->bios_size;
>         if (pp_end > bios_end)
>                 return 0;
> }
> 
> This fixes your regression while keeping Alex's concern addressed. Please let me know if it ok with you.
> 
> Thanks & Regards
> Asad

Hmm.

Checking for vbios overflow when a table header is parsed should allow
pp_entries_max() to implicitly detect vbios overflow for the
vbios-provided pp_table.

Is there a reason all atom tables are not sanity checked in this way?
It seems odd that the pp_table header isn't trusted while the rest of
the table headers are.

Thanks,
John

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
> Sent: Wednesday, July 8, 2026 10:08 PM
> To: John Olender <john.olender@gmail.com>
> Cc: amd-gfx@lists.freedesktop.org; Feng, Kenneth <Kenneth.Feng@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>; Wang, Yang(Kevin) <KevinYang.Wang@amd.com>
> Subject: Re: [PATCH] drm/amd/pm: Remove vbios bounds check from pp_entries_max()
> 
> On Sat, Jul 4, 2026 at 6:19 AM John Olender <john.olender@gmail.com> wrote:
>>
>> The soft_pp_table is not contained within the vbios when using a
>> pp_table override since commit 3cfe433630508 ("drm/amd/pm: Use
>> uploaded size for legacy custom PPTable").
>>
>> Fixes: e30b3e3ab51ad ("drm/amdgpu/pm: add pp_entries_max() helper")
>> Signed-off-by: John Olender <john.olender@gmail.com>
> 
> Removing this will effectively remove the bounds check from tables read from the bios.  I don't think we want to do that.
> 
> Alex
> 
>> ---
>>  drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
>> b/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
>> index 7ebc1344023ff..19023a850e882 100644
>> --- a/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
>> +++ b/drivers/gpu/drm/amd/pm/powerplay/inc/hwmgr.h
>> @@ -833,14 +833,10 @@ 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 *entries  = (const char *)sub_table + hdr_size;
>>
>> -       if (pp_end > bios_end)
>> -               return 0;
>>         if (!rec_size || entries >= pp_end)
>>                 return 0;
>>         return (uint32_t)((pp_end - entries) / rec_size);
>>
>> base-commit: e3cc8c108f9a91728f9ff6e24cbf3b41c58b8f8b
>> --
>> 2.47.3
>>