[PATCH] drm/amd/pm: bound SCLK FCW range entries to the table size

[PATCH] drm/amd/pm: bound SCLK FCW range entries to the table size

[Hari Mishal]

atomctrl_get_smc_sclk_range_table() copies ucSclkEntryNum entries from
the VBIOS SMU_Info table into pp_atom_ctrl_sclk_range_table. Both the
source (asSclkFcwRangeEntry[]) and destination (table->entry[]) arrays
are fixed at MAX_SCLK_RANGE (8) entries, but ucSclkEntryNum is an
unchecked u8 read straight from the VBIOS, so it can be up to 255. The
destination is stack-allocated in both callers
(polaris10_get_sclk_range_table(), vegam_get_sclk_range_table()), so a
count above 8 overflows a stack buffer, not just a heap read.

Reject counts above MAX_SCLK_RANGE rather than clamping to it. Only
one version of this VBIOS struct (ATOM_SMU_INFO_V2_1) has ever
existed, so a count above 8 corresponds to no legitimate table format
and means either a corrupted or a tampered VBIOS.

Fixes: a23eefa2f461 ("drm/amd/powerplay: enable dpm for baffin.")
Assisted-by: gkh_clanker_t1000
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
index 4b796d60b03d..3217ad3c1d17 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
@@ -1025,6 +1025,9 @@ int atomctrl_get_smc_sclk_range_table(struct pp_hwmgr *hwmgr, struct pp_atom_ctr
 	if (!psmu_info)
 		return -EINVAL;
 
+	if (psmu_info->ucSclkEntryNum > MAX_SCLK_RANGE)
+		return -EINVAL;
+
 	for (i = 0; i < psmu_info->ucSclkEntryNum; i++) {
 		table->entry[i].ucVco_setting = psmu_info->asSclkFcwRangeEntry[i].ucVco_setting;
 		table->entry[i].ucPostdiv = psmu_info->asSclkFcwRangeEntry[i].ucPostdiv;
-- 
2.43.0

Re: [PATCH] drm/amd/pm: bound SCLK FCW range entries to the table size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Unvalidated VBIOS array sizes and offsets cause out-of-bounds memory reads in atomctrl_get_leakage_vddc_base_on_leakage().
- [High] Missing VBIOS table size check in `atomctrl_get_avfs_information` allows out-of-bounds memory reads.
- [Medium] Fallback path uses uninitialized `smu_data->range_table` if the VBIOS table was successfully loaded.
- [Medium] The function `get_edc_leakage_table` correctly calculates the offset but incorrectly returns a pointer to the base table.
--

commit 79900a4165725430ba1e1ef41665d1982912cab7
Author: Hari Mishal <harimishal1@gmail.com>

drm/amd/pm: bound SCLK FCW range entries to the table size

This patch limits the number of entries read from the VBIOS SMU_Info table
into pp_atom_ctrl_sclk_range_table to MAX_SCLK_RANGE. It prevents a stack
buffer overflow caused by an unchecked ucSclkEntryNum count from the VBIOS.

> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
> index 4b796d60b03d4..3217ad3c1d17e 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
> @@ -1025,6 +1025,9 @@ int atomctrl_get_smc_sclk_range_table(struct pp_hwmgr *hwmgr, struct pp_atom_ctr
>  	if (!psmu_info)
>  		return -EINVAL;
>  
> +	if (psmu_info->ucSclkEntryNum > MAX_SCLK_RANGE)
> +		return -EINVAL;
> +
>  	for (i = 0; i < psmu_info->ucSclkEntryNum; i++) {
>  		table->entry[i].ucVco_setting = psmu_info->asSclkFcwRangeEntry[i].ucVco_setting;
>  		table->entry[i].ucPostdiv = psmu_info->asSclkFcwRangeEntry[i].ucPostdiv;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing VBIOS bounds
checking in ppatomctrl.c, I noticed another potential out-of-bounds read in
atomctrl_get_avfs_information():

drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c:atomctrl_get_avfs_information() {
	profile = (ATOM_ASIC_PROFILING_INFO_V3_6 *)
			smu_atom_get_data_table(hwmgr->adev,
					GetIndexIntoMasterTable(DATA, ASIC_ProfilingInfo),
					NULL, NULL, NULL);
	if (!profile)
		return -1;

	param->ulAVFS_meanNsigma_Acontant0 = le32_to_cpu(profile->ulAVFS_meanNsigma_Acontant0);
	...
}

Does this code fail to validate the size of the ASIC_ProfilingInfo table?

If a corrupted or older VBIOS provides a smaller table, could this lead to
reading past the end of the table mapping since the size parameter passed
to smu_atom_get_data_table() is NULL?

[Severity: High]
This is a pre-existing issue, but I found another unvalidated VBIOS array size
used in atomctrl_get_leakage_vddc_base_on_leakage():

drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c:atomctrl_get_leakage_vddc_base_on_leakage() {
	...
			for (i = 0; i < profile->ucElbVDDC_Num; i++) {
				if (vddc_id_buf[i] == virtual_voltage_id) {
	...
							*vddc = vddc_buf[j * profile->ucElbVDDC_Num + i];
	...
}

Can this loop iterate out-of-bounds if a malicious or corrupted VBIOS provides
an excessively large ucElbVDDC_Num?

The count and unvalidated offsets might lead to out-of-bounds memory reads.

[Severity: Medium]
This isn't a bug introduced by this patch, but get_edc_leakage_table() seems
to return the wrong pointer:

drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c:get_edc_leakage_table() {
	...
	temp = (char *)table_address;
	table_address += offset;

	return (AtomCtrl_EDCLeakgeTable *)temp;
}

Is this returning a pointer to the base table instead of the calculated offset?

Since table_address is incremented but temp is returned, might this cause
callers to read incorrect data from the beginning of the GFX_Info table rather
than the intended offset?

[Severity: Medium]
This is a pre-existing issue, but there seems to be an uninitialized memory
issue in the fallback path after loading the range table:

drivers/gpu/drm/amd/pm/powerplay/smumgr/polaris10_smumgr.c:polaris10_get_sclk_range_table() {
	...
		}
		return;
	}
	...
}

drivers/gpu/drm/amd/pm/powerplay/smumgr/polaris10_smumgr.c:polaris10_calculate_sclk_params() {
	...
		if (clock > smu_data->range_table[i].trans_lower_frequency
		&& clock <= smu_data->range_table[i].trans_upper_frequency) {
	...
}

If atomctrl_get_smc_sclk_range_table() succeeds,
polaris10_get_sclk_range_table() returns without initializing
smu_data->range_table.

If a subsequent divider calculation fails, could
polaris10_calculate_sclk_params() use this uninitialized range_table in its
fallback loop?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914132920.10127-1-harimishal1@gmail.com?part=1

Re: [PATCH] drm/amd/pm: bound SCLK FCW range entries to the table size

[Alex Deucher]

Applied.  Thanks!

On Mon, Sep 14, 2026 at 10:36 AM Hari Mishal <harimishal1@gmail.com> wrote:
>
> atomctrl_get_smc_sclk_range_table() copies ucSclkEntryNum entries from
> the VBIOS SMU_Info table into pp_atom_ctrl_sclk_range_table. Both the
> source (asSclkFcwRangeEntry[]) and destination (table->entry[]) arrays
> are fixed at MAX_SCLK_RANGE (8) entries, but ucSclkEntryNum is an
> unchecked u8 read straight from the VBIOS, so it can be up to 255. The
> destination is stack-allocated in both callers
> (polaris10_get_sclk_range_table(), vegam_get_sclk_range_table()), so a
> count above 8 overflows a stack buffer, not just a heap read.
>
> Reject counts above MAX_SCLK_RANGE rather than clamping to it. Only
> one version of this VBIOS struct (ATOM_SMU_INFO_V2_1) has ever
> existed, so a count above 8 corresponds to no legitimate table format
> and means either a corrupted or a tampered VBIOS.
>
> Fixes: a23eefa2f461 ("drm/amd/powerplay: enable dpm for baffin.")
> Assisted-by: gkh_clanker_t1000
> Signed-off-by: Hari Mishal <harimishal1@gmail.com>
> ---
>  drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
> index 4b796d60b03d..3217ad3c1d17 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppatomctrl.c
> @@ -1025,6 +1025,9 @@ int atomctrl_get_smc_sclk_range_table(struct pp_hwmgr *hwmgr, struct pp_atom_ctr
>         if (!psmu_info)
>                 return -EINVAL;
>
> +       if (psmu_info->ucSclkEntryNum > MAX_SCLK_RANGE)
> +               return -EINVAL;
> +
>         for (i = 0; i < psmu_info->ucSclkEntryNum; i++) {
>                 table->entry[i].ucVco_setting = psmu_info->asSclkFcwRangeEntry[i].ucVco_setting;
>                 table->entry[i].ucPostdiv = psmu_info->asSclkFcwRangeEntry[i].ucPostdiv;
> --
> 2.43.0
>