On 7/13/26 12:29 AM, Asad Kamal wrote:
> 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.
>
When I hit this issue, the pp_end > bios_end check didn't always return
true. The order of pp_table overrides mattered for triggering the
failure on a system with multiple cards.
I also wouldn't call this failure path silent. Callers of
pp_entries_max() print appropriate warnings, painting a nice picture of
what's happening during the resulting cascade:
$ sudo cp custom_pp_table.bin /sys/class/drm/cardN/device/pp_table
Killed
[ 71.320109] kernel: amdgpu: amdgpu: MM dependency table: clamping
ucNumEntries 8 -> 0
[ 71.320137] kernel: amdgpu: amdgpu: Polaris SCLK dependency table: clamping
ucNumEntries 8 -> 0
[ 71.320149] kernel: amdgpu: amdgpu: MCLK dependency table: clamping
ucNumEntries 4 -> 0
[ 71.320166] kernel: amdgpu: Number of Pcie Entries exceed the number of SCLK
Dpm Levels! Disregarding the excess entries...
[ 71.320179] kernel: amdgpu: SCLK DPM index for VRHot cannot exceed the total
sclk level count!
[ 71.320190] kernel: amdgpu: amdgpu: VddcLookup table: clamping ucNumEntries
15 -> 0
[ 71.320200] kernel: amdgpu: amdgpu: VddcLookup table: clamping ucNumEntries
8 -> 0
[ 71.320211] kernel: amdgpu: [powerplay] Lookup table is empty
[ 71.320222] kernel: amdgpu: [powerplay] Lookup table is empty
...
[ 71.678679] kernel: note: cp[2243] exited with irqs disabled
> 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]>
Tested-by: John Olender <[email protected]>
Thanks,
John
> ---
> 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;
> +
> + if (pp_end > bios_end)
> + return 0;
> + }
> if (!rec_size || entries >= pp_end)
> return 0;
> return (uint32_t)((pp_end - entries) / rec_size);
