DF-5031 — it.c: missing signed conversion of IT87xx temperature registers

File: sys/dev/powermng/it/it.c Severity: Info (sensor-data correctness; no memory-safety impact) Confidence: certain (code inspection; chip semantics from IT87xx datasheet family)

What the code does

void
it_generic_stemp(struct it_softc *sc, struct ksensor *sensors)
{
    int i, sdata;

    for (i = 0; i < 3; i++) {
        sdata = it_readreg(sc, ITD_SENSORTEMPBASE + i);   /* u_int8_t */
        /* Convert temperature to Fahrenheit degres */
        sensors[i].value = sdata * 1000000 + 273150000;   /* it.c:261 */
    }
}

it_readreg() returns u_int8_t (it.c:192-197), so sdata is 0..255. IT87xx temperature data registers (0x29-0x2B here) are signed two's complement degrees Celsius. The formula degC * 1e6 + 273150000 is the correct µK conversion used across the DragonFly hw.sensors framework, but without an (int8_t) cast every sub-zero reading (0x80..0xFF = -128..-1 °C) is reported as +128 .. +255 °C (up to ~528 K) under hw.sensors.it0.tempN. The stale comment also says "Fahrenheit" — the framework unit is micro-Kelvin.

Impact

Wrong monitoring data only. An overheating-vs-frozen misread is an operational hazard (a chilled sensor reads as scorching), but there is no memory-safety, privilege, or DoS consequence: sensors[i].value is int64_t (sys/sys/sensors.h:146) and the arithmetic stays far below overflow even with the sign bug (255e6 + 273.15e6 ≈ 5.3e8 < 2^63).

Why it is only Info

The driver requires two admin gates before it exists at runtime: sys/conf/files:2601 gates it behind optional it isa (not in X86_64_GENERIC — guest-verified 0 matches; no it.ko shipped or loaded), and the ISA device needs an explicit port hint before probe/attach run. Once attached, the parsed data comes from the monitored chip's own registers, not from any unprivileged user input; unprivileged users can only read the resulting sysctl values.

Reproduce (reachability, read-only)

reachability.sh / reachability.log — on the QEMU guest: device it absent from the config, no it.ko, not loaded. No runtime PoC possible (no IT87xx hardware behind QEMU).

Recommended fix

--- a/sys/dev/powermng/it/it.c
+++ b/sys/dev/powermng/it/it.c
@@ -258,7 +258,8 @@ it_generic_stemp(struct it_softc *sc, struct ksensor *sensors)
    int i, sdata;

    for (i = 0; i < 3; i++) {
-       sdata = it_readreg(sc, ITD_SENSORTEMPBASE + i);
-       /* Convert temperature to Fahrenheit degres */
+       /* temp registers are signed two's complement, degC */
+       sdata = (int8_t)it_readreg(sc, ITD_SENSORTEMPBASE + i);
+       /* Convert temperature to micro-Kelvin */
        sensors[i].value = sdata * 1000000 + 273150000;
    }
 }

(Full context in fix.diff; never applied to sys/.)

DF-5031 VERDICT — untested (Phase V skipped: Info-severity; no IT87xx hardware on guest)

Bottom line

it_generic_stemp() (sys/dev/powermng/it/it.c:253-263) reads the IT87xx temperature registers through it_readreg() which returns u_int8_t (it.c:192-197) and never applies the signed conversion the registers require. Sub-zero temperatures (0x80..0xFF, i.e. -128..-1 °C) are published to the hw.sensors framework as +128..+255 °C. Pure sensor-data correctness defect inherited from the 2007 OpenBSD port ($OpenBSD: it.c,v 1.22): the framework unit is µK (degC*1e6 + 273150000), the in-tree comment mislabels it "Fahrenheit". No memory-safety impact: the destination is int64_t and the math cannot overflow.

Why no runtime PoC (status untested)

The it driver is not present on the audit guest at all (guest-verified: no device it in X86_64_GENERIC, no it.ko in /boot/kernel, nothing loaded — reachability.log), and even with the module hand-built it needs an ISA port hint plus real IT8705-class Super-I/O silicon to produce a reading; QEMU exposes no such device. There is no unprivileged input path into this code in any case — the only user-facing surface is reading the (wrong) values back via sysctl.

Evidence

• reachability.log — guest output: device-it-in-config: 0, it.ko … NONE, it-loaded: 0; only coretemp from the powermng family is in the default config.
• Source: it.c:259-261 (the unsigned read + µK conversion), it.c:192-197 (it_readreg returns u_int8_t), itvar.h:62 (temp base 0x29), sys/sys/sensors.h:146 (value int64_t), sys/conf/files:2601 (optional it isa).

Other classes checked and killed in this file (pass 2)

• Fan divisor div-by-zero (it.c:288-313): impossible — sdata == 0 and 0xff are handled before 1350000 / (sdata << (divisor & 7)) at it.c:308; the divisor argument is ≥ 1 for every surviving path.
• Volt scaling overflow (it.c:266-285): max intermediate 4080 × 51500 = 2.10e8 < INT32_MAX, stored in int64_t; (sdata<<4)-4096 can be negative by design (negative rails) — values stay sane.
• Sensor array bounds (it.c:154-156, 323-325 vs itvar.h:32,86): fan 0-2, volt 3-11, temp 12-14 in sensors[15]; refresh passes matching bases.
• Chip-ID bypass (it.c:125-130): exact equality on 0x90; no mask, no bypass; single-register probe is the standard IT87xx EC ID check.
• Detach UAF vs sensor task (it.c:175-190): killed by kern_sensors.c — st->func(st->arg) executes under thr->lock (kern_sensors.c:334) and sensor_task_unregister takes the same lock before clearing running (kern_sensors.c:243-248); the task entry is framework-owned and freed by the sensors thread (kern_sensors.c:326-331) without touching arg.
• Uninitialized sensor values via sysctl: newbus softc is allocated with M_INTWAIT | M_ZERO (kern/subr_bus.c:1952-1953), and sysctl_handle_sensor copies explicit fields only (kern_sensors.c:484-493).
• Port-range OOB: only offsets 5/6 (ITC_ADDR/ITC_DATA, itvar.h:39-40) of the 8 allocated ports are ever touched (it.c:115-116, 143-144, 195-203); register indices like ITD_CHIPID 0x58 are address-port values, not port offsets.
• Probe/attach interleaving: sensor_task_register at it.c:158 may fire before monitoring enable at 161-163, but sc_iot/sc_ioh are set at 149-150 before registration; a garbage first sample is harmless.
• Probe returns 1 instead of ENXIO (it.c:119,130) — style only; any non-zero probe return means no match in newbus.

Fix validation

Not performed — no runtime baseline exists on the guest (driver absent, hardware absent). fix.diff is the reviewer-authored one-line sign fix, untested at runtime by construction.

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.