• File: sys/dev/drm/amd/powerplay/amd_powerplay.c
• Lines: 619, 640
• Severity: Medium
• CVSS: CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U:C:L/I:L/A:H
• CWE: CWE-787 Out-of-bounds Write
• Confidence: likely

Summary

pp_dpm_get_pp_num_states() loops i from 0 to hwmgr->num_ps-1 writing enum constants into data->states[i], where data is a struct pp_states_info whose states[] array is hard-coded to 16 entries (kgd_pp_interface.h:153-156).

hwmgr->num_ps is set from the VBIOS-derived ucNumEntries byte (uint8_t, max 255) by psm_init_power_state_table() in pp_psm.c:43 via pp_tables_get_num_of_entries() at processpptables.c:878.

When num_ps > 16 the loop writes up to (num_ps-16) uint32_t slots past the end of data->states[] into the caller's kernel stack frame.

The written values are bounded POWER_STATE_TYPE_* enum ints (0..7), so the impact is primarily denial-of-service (kernel stack corruption / panic), with a narrow info-leak read in the caller's subsequent loop if data.nums ends up larger than 16.

Root cause

The bug is in sys/dev/drm/amd/powerplay/amd_powerplay.c:619-640:

619:    data->nums = hwmgr->num_ps;
620:
621:    for (i = 0; i < hwmgr->num_ps; i++) {
622:        struct pp_power_state *state = (struct pp_power_state *)
623:            ((unsigned long)hwmgr->ps + i * hwmgr->ps_size);
624:        switch (state->classification.ui_label) {
625:        case PP_StateUILabel_Battery:
626:            data->states[i] = POWER_STATE_TYPE_BATTERY;
...
639:        }
640:    }

struct pp_states_info is fixed at { uint32_t nums; uint32_t states[16]; } (sys/dev/drm/amd/include/kgd_pp_interface.h:153-156, ARRAY_SIZE(states) == 16).

hwmgr->num_ps is taken verbatim from the parsed PowerPlay table's ucNumEntries byte (sys/dev/drm/amd/powerplay/hwmgr/processpptables.c:874-880 reads either pstate_arrays->ucNumEntries or powerplay_table->ucNumStates, both UCHAR i.e. uint8_t, range 0..255) via pp_tables_get_num_of_entries() and assigned in sys/dev/drm/amd/powerplay/hwmgr/pp_psm.c:43.

There is no clamp.

The hwmgr->ps allocation itself is correct (kcalloc(num_ps, ps_size) at pp_psm.c:53, so the source read in the loop is in-bounds), but the destination data->states[i] is fixed-size 16.

When num_ps is, say, 255, the loop writes data->states[16..254] (i.e. up to 956 bytes) past &data->states[15] into the caller amdgpu_get_pp_num_states()'s kernel stack frame (sys/dev/drm/amd/amdgpu/amdgpu_pm.c:332 declares struct pp_states_info data; on the stack and passes &data in at line 336).

The same overflow is also reachable via amdgpu_get_pp_cur_state() at amdgpu_pm.c:362.

Threat

Two realistic trigger models.

1. Hardware supply-chain / malicious peripheral: an attacker attaches an AMD GPU whose VBIOS PowerPlayInfo state-array header advertises ucNumEntries > 16 (a single malformed byte in the option ROM). On host boot, the driver parses it, sets num_ps to the claimed value, and any subsequent read of /sys/class/drm/cardN/device/pp_num_states (mode 0444, world-readable — amdgpu_pm.c:1002) or pp_cur_state (also S_IRUGO) by an unprivileged user corrupts the kernel stack and typically panics the box.

2. Root-plant-then-trigger: a privileged user (or one that has just exploited DF-1560 to gain root) calls pp_dpm_set_pp_table() with a crafted ATOM_PPLIB table whose ucNumEntries is > 16; amd_powerplay_reset() re-parses it into hwmgr->num_ps; any subsequent unprivileged read of pp_num_states triggers the overflow.

Net impact on a default config: local unprivileged user can panic the kernel (DoS) by reading a world-readable sysfs file, given a malformed PP table (whether from hardware or pre-planted).

No elevated privilege required to read the sysfs node.

Exploit / PoC

Two-stage reproduction.

STAGE A (root, one-shot, plant the trap): overwrite the pp_table sysfs file with a crafted ATOM PowerPlayInfo table whose StateArray header at offset usStateArrayOffset advertises ucNumEntries=255.

# plant_table.py — as root on the amdgpu box
import struct
path='/sys/class/drm/card0/device/pp_table'
t=bytearray(open(path,'rb').read())
sa_off=struct.unpack_from('<H', t, 12)[0]
print('StateArray offset', sa_off, 'old ucNumEntries', t[sa_off])
t[sa_off]=255                       # claim 255 states
open(path,'wb').write(bytes(t))     # triggers amd_powerplay_reset -> num_ps=255

STAGE B (unprivileged, trigger the overflow): as ANY local user, simply read the world-readable attribute:

cat /sys/class/drm/card0/device/pp_num_states    # <-- kernel stack OOB write fires here

Expected result on a non-hardened kernel: immediate kernel panic from corrupted stack frame. On KASAN: BUG: stack-out-of-bounds in pp_dpm_get_pp_num_states with write of size 4 at data->states[16..254].

Recommended fix

Bound the loop to the destination array. Reject any table whose claimed state count exceeds what struct pp_states_info can describe:

--- a/sys/dev/drm/amd/powerplay/amd_powerplay.c
+++ b/sys/dev/drm/amd/powerplay/amd_powerplay.c
@@ -612,6 +612,10 @@ static int pp_dpm_get_pp_num_states(void *handle,
    if (!hwmgr || !hwmgr->pm_en ||!hwmgr->ps)
        return -EINVAL;

+   if (hwmgr->num_ps > ARRAY_SIZE(data->states))
+       return -EINVAL;
+
    mutex_lock(&hwmgr->smu_lock);

    data->nums = hwmgr->num_ps;

ARRAY_SIZE(data->states) is 16 (kgd_pp_interface.h:155).

The same bound should also be applied in the callers amdgpu_get_pp_num_states() and amdgpu_get_pp_cur_state() in amdgpu_pm.c, but the fix at the source (this function) is sufficient because both callers pass &data through amdgpu_dpm_get_pp_num_states() and rely on its return value being 0 before using data.nums.

An alternative defense-in-depth is to also cap data->nums = min(hwmgr->num_ps, 16) after the bound check so that the caller's for-loop also cannot exceed the array.

Related findings

• DF-1560 (sibling): pp_dpm_set_pp_table heap overflow — can be used to plant the malformed table that triggers this finding.