DF-5181 — smu9 SMC-message failures are swallowed (return 0), vacating caller error guards and feeding stale smu9_get_argument() data into Vega10/12 powerplay

File: sys/dev/drm/amd/powerplay/smumgr/smu9_smumgr.c (155 LOC) Class: CWE-252 (Unchecked Return Code) / CWE-754 (Improper Check for Exceptional Conditions) Severity: Low · Confidence: certain · Bucket: base:drm (vendored-parity with upstream Linux)

What the code does

smu9_send_msg_to_smc() (smu9_smumgr.c:101-117) and smu9_send_msg_to_smc_with_parameter() (smu9_smumgr.c:128-147) both detect SMC failure — ret = smu9_wait_for_response(hwmgr); if (ret != 1) — but only pr_err() it and then unconditionally return 0 (:116, :146).

The wait itself is bounded (phm_wait_for_register_unequal, smu_helper.c:151-173, hwmgr->usec_timeout = AMD_MAX_USEC_TIMEOUT = 200 ms, hwmgr.c:85) so there is no hang; each wedged message burns two full timeouts (pre-send drain at :106/:134 + real wait at :112/:142).

Consequence (why the return value matters)

Callers were written expecting a propagated error:

• vega10_smumgr.c:147-150: PP_ASSERT_WITH_CODE(!smu9_send_msg_to_smc(hwmgr, PPSMC_MSG_GetDriverIfVersion), "Attempt to get SMC IF Version Number Failed!", return -EINVAL) — with the always-0 return this guard can NEVER fire; :151 then reads smc_driver_if_version = smu9_get_argument(hwmgr) from C2PMSG_82, which was not overwritten by the failed message (only the response reg C2PMSG_90 is zeroed at :108/:136) → stale value flows into the version check at :160.
• vega12_smumgr.c:52-57 writes the condition explicitly as smu9_send_msg_to_smc_with_parameter(...) == 0 with the message "Attempt to Set Dram Addr High Failed!" — dead guard today.
• vega10_smumgr.c:47-58 (vega10_copy_table_from_smc): three unchecked sends (SetDriverDramAddrHigh/Low + TransferTableSmu2Dram) followed by memcpy(table, priv->smu_tables.entry[table_id].table, ...size) — on a failed transfer the driver memcpy's the previous contents of the driver-side buffer and the powerplay state machine consumes stale table data.
• vega10_smumgr.c:107-108: stale features_enabled bitmap drives vega10_is_dpm_running() (:113-123) decisions.

No memory-unsafe sink was found: all table-copy sizes are driver-fixed at allocation (vega10_smumgr.c:45-46, :57-58), never derived from smu9_get_argument(). Impact ceiling is wrong power/clock/DPM decisions from stale SMU state.

Reproduce

Requires Vega10/Vega12 silicon with a hung/mismatched SMC firmware — not testable on the audit QEMU guest (virt hardware, no AMD dGPU). The defect is a source-semantics property fully demonstrated by citation above; no runtime verification was attempted (status untested). Fix validation likewise not_testable — see VERDICT.md.

Fix

fix.diff in this directory: propagate return (ret == 1) ? 0 : -EIO; in both send functions. Caller-compatible (vega10/vega12 guards already treat 0 as success). Matches the direction upstream Linux eventually needs; this file is a verbatim port of upstream smu9_smumgr.c, so the fix owner is upstream drm-amd.

VERDICT — DF-5181

• Finding: smu9 SMC-message failures swallowed (return 0), vacuous caller guards, stale smu9_get_argument() consumption.
• File: sys/dev/drm/amd/powerplay/smumgr/smu9_smumgr.c
• PoC status: untested (Phase V skipped per reviewer contract: Low severity, hardware-fault-gated). reproduced = false, impact = none.
• Confidence: certain — the defect is unconditional in the source and the vacuous-guard consequence is directly readable in the callers.

Why skipped (honest classification)

The QEMU/KVM audit guest has no AMD Vega10/Vega12 dGPU; the amdgpu powerplay path for smu9 (hwmgr->smumgr_funcs wired by vega10_smumgr.c:353-358 / vega12_smumgr.c:372-377) cannot be instantiated, so neither the baseline nor a patched kernel can be exercised on it. The trigger additionally requires the SMC firmware to fail/time out — a hardware/firmware fault state, not an attacker-injectable input. There is no memory-corruption primitive to develop: every table-transfer memcpy uses driver-fixed sizes allocated at init (vega10_smumgr.c:45-46 → :57-58); smu9_get_argument() output only reaches version/feature logic (vega10_smumgr.c:151,160; :107-108 → :113-123).

Root cause (path:line)

• smu9_smumgr.c:112-116 and :142-146: failure detected (ret != 1), logged, then return 0 unconditionally.
• smu9_wait_for_response (:61-76): bounded wait — no hang possible (smu_helper.c:161-172, usec_timeout = 200 ms, hwmgr.c:85, amd_shared.h:29). Two full timeouts burned per failed message (pre-send drain :106/:134 + real wait :112/:142).
• Argument staleness: :108/:136 zero only the response register (C2PMSG_90); C2PMSG_82 (read raw by smu9_get_argument, :150-155) retains the previous message's value.

Caller-side proof the guards are vacuous

• pp_debug.h:37-43: PP_ASSERT_WITH_CODE(cond, ...) fires iff !(cond).
• vega10_smumgr.c:147-150: cond = !smu9_send_msg_to_smc(...) = !0 = always true → the "Attempt to get SMC IF Version Number Failed!" branch is unreachable; :151 consumes stale C2PMSG_82.
• vega12_smumgr.c:52-57: cond written as smu9_send_msg_to_smc_with_parameter(...) == 0 with failure messages — always true today, i.e. dead error handling (proving callers expect a propagated error).
• vega10_smumgr.c:47-58: zero checks at all before the table memcpy.

Impact ceiling

Robustness/logic only: on an SMC that is wedged or firmware-mismatched, the Vega10/12 driver proceeds as if messages succeeded — stale driver-if version, stale enabled-feature bitmaps (vega10_is_dpm_running), stale SMU table contents consumed by hwmgr. Wrong clocks/power gating; no kernel memory safety violation, no unprivileged-local trigger (message paths run at init/suspend/power-state transitions; no user data reaches C2PMSG_66/82 here).

Upstream parity: file is a verbatim port of Linux drivers/gpu/drm/amd/powerplay/smumgr/smu9_smumgr.c — upstream carries the identical defect; fix owner upstream.

Fix validation

fix_status = not_testable (no Vega silicon on the audit guest; fix is a two-line return-value change whose caller compatibility is verifiable by inspection — every in-tree caller treats 0 as success: vega10_smumgr.c:147, vega12_smumgr.c:52-57,63-68 and the function-pointer table at vega10_smumgr.c:353-354 / vega12_smumgr.c:372-373).

fix.diff authored against the read-only sys/ tree; never applied there.

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.