⬢ DragonFlyBSD Kernel Audit
← triage · dashboard
DF-4761

32-bit arithmetic overflow in fan-RPM math: 60 * crystal_clock_freq * 10000 wraps for any realistic xclk, skewing reported fan speed and the programmed TACH target

Summary

Both RPM directions compute 60*crystal_clock_freq*10000 in u32 - wraps before the division for 25MHz-class xclk (1.5e10), so get_fan_speed_rpm reports wrong RPM and set_fan_speed_rpm programs a TACH target from the wrapped constant (same expression as upstream Linux). Telemetry/hw-reliability only; Vega10 hw-gated. u64-cast fix.

Discussion (0)

No comments yet.

PoC verification

Evidence pack

findings/poc/DF-4761 · 2 files
FileTypeDescriptionSize
README.md file 2.3 KB ↓ raw
verdict.json file 2.1 KB view raw
README.md file
↓ download raw

DF-4761 — vega10 powerplay fan-RPM arithmetic overflows 32 bits (60 * xclk * 10000), producing wrong fan telemetry and a wrong TACH target; div-by-zero in the set path requires crafted VBIOS + root

  • File: sys/dev/drm/amd/powerplay/hwmgr/vega10_thermal.c:115 and :325
  • Severity: Info (correctness/hardening; no memory-safety impact; reachability requires an AMD Vega10-class GPU and, for the interesting variants, root via hwmon)
  • Status: untested (the audit guest has no AMD GPU — QEMU/virtio hardware only; the arithmetic is provable from source)

What

*speed = 60 * crystal_clock_freq * 10000 / tach_period;   /* :115, u32 math */
...
tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed); /* :325 */

amdgpu_asic_get_xclk() returns the crystal clock in kHz (25 MHz-class, e.g. 25000 for Vega10). 60 * 25000 * 10000 = 1.5e10 exceeds UINT32_MAX (4.29e9); the u32 numerator wraps to 2,115,098,112 before the division, so:

  • the reported fan RPM (vega10_fan_ctrl_get_fan_speed_rpm, the non-SMC path) is systematically wrong (~12% class error at typical RPM) — telemetry only;
  • the TACH target programmed on the set path (set_fan_speed_rpm) is derived from the same wrapped constant — fan regulation reference is skewed (hardware correctness/reliability, no kernel memory impact);
  • the set path divides by 8 * speed: 8 * speed == 0 (mod 2^32) requires speed == 2^29 exactly, which the ulMinRPM/ulMaxRPM clamp (lines 314-318, values from the card's VBIOS ATOM fan table) makes unreachable on real firmware — a div-by-zero needs a crafted VBIOS and a root hwmon write. The get path's tach_period == 0 is guarded (line 110-111) and usMaxFanRPM != 0 guards the percent path (80-84).

All writes in the file are masked register fields (REG_SET_FIELD) or named PPTable members — no attacker-driven indexing, no copyin, no allocations, no work items (the thermal IRQ registration the task mentions lives outside this file). The same expression exists verbatim in upstream Linux (long-standing port), where it is also unguarded.

Compute in 64 bits: *speed = 60 * (uint64_t)crystal_clock_freq * 10000 / tach_period; (and likewise for tach_period on the set path), or precompute 60 * xclk / 8 once as u64.

Fix verification

not_testable
baseline no→ patch + rebuild →patched clean

not tested (no AMD GPU on the audit guest)

per-fix-DF-4761

Confirmed kernel references

Detail

Evidence (decisive lines)

findings/poc/DF-4761/README.md (arithmetic walkthrough; no guest artifacts)

PoC changes

none (no hardware on the audit guest)

Verified recommended fix

Compute the RPM/TACH numerators in 64 bits (60ULL * crystal_clock_freq * 10000) before dividing

Verdict

Analysis-only. 60 * crystal_clock_freq * 10000 (u32) wraps for any realistic xclk (25 MHz-class -> 1.5e10 > UINT32_MAX) in both the fan-RPM read (vega10_thermal.c:115) and the TACH-target set (:325) paths, producing systematically wrong fan telemetry and a skewed fan-regulation reference. The div-by-zero variant needs 8*speed==0 (speed==2^29) which the firmware-provided ulMinRPM/ulMaxRPM clamp makes unreachable on real VBIOS (crafted VBIOS + root hwmon write required). No memory-safety impact: all writes are masked register fields or named PPTable members; div-by-zero guards exist on the get paths. Guest has no AMD GPU, so not testable here; the arithmetic is source-provable (same expression as long-standing upstream Linux code).