DF-5068 — psm_init_power_state_table() ignores get_pp_table_entry() failures: partially-initialized / all-zero power states enter the state table and can be selected and programmed to the SMC

Build

Not buildable/runnable on the audit guest: requires an AMD GPU (amdgpu attach) whose VBIOS PowerPlay table drives psm_init_power_state_table(). The guest has only QEMU std VGA (chip=0x11111234); /dev/dri does not exist (see env.txt).

Run

N/A — see env.txt.

Expected behavior if triggered on hardware

1. Attach an AMD GPU whose PowerPlay table contains an entry that makes the per-entry parser fail midway (trivial with the pass-1 unvalidated-offset family: DF-1468/DF-1469/DF-1470 — any USHORT offset/UCHAR count pointing outside soft_pp_table_size).
2. get_pp_table_entry() (pp_psm.c:76) returns nonzero for that entry. The result is never checked.
3. The (partially written or all-zero) entry stays in the table: - if classification.flags was written before the parser failed and has the Boot bit, the garbage state is memcpy'd into current_ps/request_ps (pp_psm.c:78-82) and becomes boot_ps; - entries with matching ui_label bits are selectable via psm_get_ui_state() (pp_psm.c:122-128) → psm_set_states() memcpy (pp_psm.c:163-169) → power_state_management() programs the garbage hardware struct to the SMC (pp_psm.c:248-255). - if every entry fails, boot_ps stays NULL and current/request stay zeroed while init reports success (return 0) — downstream adjust runs on an all-zero pp_power_state. 3'. Fix behavior: init fails with the parser's error code and the allocations are unwound (fix.diff).

Fix

Apply fix.diff: fail psm_init_power_state_table() on any per-entry parser error, freeing the trio and returning the error (hwmgr_hw_init err2 path then unwinds the backend as it already does for other init failures).

VERDICT — DF-5068 (pass 2, df-bsd-reviewer)

Status: untested (hardware-gated; not runnable on the audit guest — requires an AMD GPU with a crafted VBIOS PowerPlay table). Defect existence: certain (the ignored error return is objective). Downstream impact: garbage/zeroed power states can be selected and programmed to the SMC; realistic ceiling is wrong-clock programming / hardware misbehavior and downstream panics in consumers that assume valid states — not a controllable memory-corruption primitive by itself.

Where the defect is (path:line)

sys/dev/drm/amd/powerplay/hwmgr/pp_psm.c:75-89:

75:     for (i = 0; i < table_entries; i++) {
76:         result = hwmgr->hwmgr_func->get_pp_table_entry(hwmgr, i, state);
77:
78:         if (state->classification.flags & PP_StateClassificationFlag_Boot) {
79:             hwmgr->boot_ps = state;
80:             memcpy(hwmgr->current_ps, state, size);
81:             memcpy(hwmgr->request_ps, state, size);
82:         }
...
86:         if (state->classification.flags & PP_StateClassificationFlag_Uvd)
87:             hwmgr->uvd_ps = state;
88:         state = (struct pp_power_state *)((unsigned long)state + size);
89:     }
91:     return 0;

result is assigned at :76 and never read — not checked, not returned. Whatever get_pp_table_entry() (the per-ASIC PowerPlay-entry parser, e.g. tonga_get_pp_table_entry / vega10_get_powerplay_table_entry / smu8 reset power state builders) wrote into state before failing is kept, and every subsequent stage treats the table as fully valid because init returns 0.

Consequence chain (all path:line)

• A partially-written entry: the per-ASIC parsers write state->classification early and the hardware union piecemeal from VBIOS records; any mid-entry failure (the DF-1468/1469/1470 unvalidated-offset family makes offsets/counts pointing outside soft_pp_table_size trivial to craft) leaves classification flags live over a truncated hardware struct.
• pp_psm.c:78-82 — a Boot-flagged half state becomes boot_ps and is copied into current_ps and request_ps, which every later psm_adjust_power_state_dynamic() diff/program cycle consumes (pp_psm.c:244-255).
• pp_psm.c:122-128 / :163-169 — psm_get_ui_state() matches ui_label bits and psm_set_states() memcpy's the half state into request_ps.
• pp_psm.c:248-255 — phm_apply_state_adjust_rules() + phm_set_power_state() program the garbage hardware (engine clocks, voltages, PCIE lanes) to the SMC.
• If ALL entries fail: boot_ps/uvd_ps stay NULL, current/request stay zeroed (kzalloc), yet init returns 0 and hwmgr->ps != NULL — e.g. smu8 consumers of the related boot-state assumption already panic (see DF-4169/DF-4143 family, where a missing Boot-classified state crashes cast consumers). psm itself keeps operating on an all-zero pp_power_state (zero clocks/voltages handed to adjust rules and the SMC).

Why untested on the guest

Requires amdgpu attach + the crafted VBIOS. Guest has no AMD display device (pciconf: vgapci0 chip=0x11111234 = QEMU std VGA; /dev/dri absent; no amdgpu/radeon/inteldrm in dmesg — see env.txt). No runtime trigger possible.

Fix (fix.diff)

    for (i = 0; i < table_entries; i++) {
        result = hwmgr->hwmgr_func->get_pp_table_entry(hwmgr, i, state);
+       if (result)
+           goto err_out;

plus an err_out label that unwinds the three allocations, NULLs them, resets num_ps/ps_size, and returns result. hwmgr_hw_init() already routes a nonzero psm_init return to its err2 unwind (hwmgr.c:238-240,257-264).

Threat model honesty: exploiting this needs control of the VBIOS PowerPlay table (crafted GPU / re-flashed VBIOS / malicious device), the same precondition the pass-1 VBIOS-parsing family (DF-1166 … DF-4703) was filed under. Severity Low: no direct memory-safety violation in this file; the damage is state-table corruption consumed by later stages.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.