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
-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Alex Deucher
Sent: Wednesday, July 8, 2026 10:08 PM
To: John Olender <[email protected]>
Cc: [email protected]; Feng, Kenneth <[email protected]>;
Deucher, Alexander <[email protected]>; Kamal, Asad
<[email protected]>; Lazar, Lijo <[email protected]>; Zhang, Hawking
<[email protected]>; Wang, Yang(Kevin) <[email protected]>
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 <[email protected]> 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 <[email protected]>
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
>
