DF-4407 — /dev/tpm open/read/write are not serialized across CPUs (TOCTOU on TPM_OPEN + interleaved TIS transactions)

• File: sys/dev/crypto/tpm/tpm.c (pass 2, GLM 5.3)
• Class: CWE-362 (race condition) / CWE-667 (improper locking)
• Severity: Low — the device node is created 0600 root:wheel (tpm.c:253-254), so both racers must already be privileged (tcsd runs as root). Impact is TPM state-machine corruption / self-inflicted DoS, not memory corruption.

Build / run

Not runnable on the audit guest: no TPM TIS device is exposed by QEMU here, the driver is optional tpm (not in X86_64_GENERIC) and no tpm module is loaded — see env.txt. On real hardware with a TIS 1.2 TPM:

# as root, on an SMP machine, two processes:
A: while :; do dd if=/dev/tpm of=/dev/null bs=64 count=1 &  done
B: while :; do dd if=/dev/zero of=/dev/tpm bs=64 count=1 &   done

Success criterion: with the bug, tpmopen's single-open check is bypassed (two simultaneous opens succeed) and the TIS locality/STS state machine receives interleaved register writes; expect TPM_E_IO / device wedges (tcsd hangs) and tpm_request_locality EBUSY storms that require a power cycle. Expected after fix: second open returns EBUSY; transactions serialize.

Why (source)

• tpmopen (tpm.c:1146-1161) does a plain check-then-set of sc->sc_flags |= TPM_OPEN with no lock — two CPUs can both observe the flag clear and both succeed (classic TOCTOU), defeating the exclusive-open invariant.
• tpmread/tpmwrite (tpm.c:1181-1292) use crit_enter()/crit_exit() as their only "serialization". In DragonFly a critical section blocks interrupts and preemption on the current CPU only; it provides no inter-CPU mutual exclusion. Two processes that won the open race (or a process racing the interrupt handler / suspend path) can run tpm_tis12_write concurrently on different CPUs, interleaving TPM_DATA FIFO writes, TPM_ACCESS locality requests (tpm.c:412-413) and shared sc->sc_stat updates (tpm.c:736, 832, 851, 878, 895).
• This driver was imported from FreeBSD r365144 (see $FreeBSD: tag at tpm.c:18), which is precisely the commit that added sx device locking to FreeBSD's tpm(4); the DragonFly port kept the tag but not the lock.
• tpmclose (tpm.c:1163-1178) has the same unlocked flag clear.

Recommended fix

Add a lock to struct tpm_softc and hold it across open/close/read/write bodies (see fix.diff). This mirrors upstream FreeBSD r365144.

DF-4407 VERDICT — untested (source-verified; hardware- and privilege-gated)

Reproduced?

No. Status untested: the audit guest exposes no TPM (no /dev/tpm, no tpm module, QEMU image has no TPM TIS device — env.txt), and even on hardware both racing processes must be root (make_dev(..., UID_ROOT, GID_WHEEL, 0600) at tpm.c:253-254). No unprivileged trigger exists, so this does not meet the PoC bar of the audit (not Critical/High, no unprivileged local program).

Defect (certain from source)

1. TOCTOU on the exclusive-open flag. tpmopen (tpm.c:1146-1161):

c if (sc->sc_flags & TPM_OPEN) /* check */ return EBUSY; sc->sc_flags |= TPM_OPEN; /* set — not atomic w.r.t. the check */

No lock, no atomic op. On SMP, N concurrent open("/dev/tpm") calls can all observe the flag clear. tpmclose (tpm.c:1172-1175) clears it the same racy way.

2. No inter-CPU serialization of transactions. tpmread (tpm.c:1193-1252) and tpmwrite (tpm.c:1267-1292) bracket the whole command with crit_enter()/crit_exit(). A DragonFly critical section is per-CPU (interrupt/preemption shielding only — see sys/kern/kern_synch.c which freely combines crit sections with sleeping, e.g. tstop()); it does not exclude another CPU from executing the same driver entry simultaneously. Consequences of two concurrent transactions: - interleaved TPM_DATA FIFO writes from tpm_tis12_write (tpm.c:821-843) — the TPM receives two merged commands; the response stream is then parsed by tpmread (tpm.c:1208) with a bogus length, typically wedging the chip until TPM_STS_CMD_READY abort (tpm.c:750) or power cycle; - interleaved locality handoff in tpm_request_locality (tpm.c:412-427) — one thread releases locality (tpm_tis12_end, tpm.c:891-892) while the other still assumes it; - data races on shared sc->sc_stat (tpm.c:736, 832, 851, 878, 895) and on the tsleep/wakeup channels (function pointers used as idents, tpm.c:420, 463, 548, 604 vs tpm_intr tpm.c:932-942).

3. Provenance: the file header carries $FreeBSD: head/sys/dev/tpm/tpm.c 365144 2020-09-01 mjg $ — FreeBSD r365144 is the commit that added struct sx dev_lock serialization to tpm(4). The DragonFly import kept the revision tag but the lock (and the softc member) is absent here; the crit_enter() calls are the OpenBSD spl() heritage, which never provided SMP exclusion.

Impact ceiling

Privileged-only DoS of the TPM subsystem (chip wedge / tcsd hang / forced reboot to recover). No kernel memory corruption: all copies stay within buf[TPM_BUFSIZ] regardless of interleaving, because every length used against the buffer is MIN(sizeof(buf), …) (tpm.c:1273) or bounded by the response length check (tpm.c:1212, len > uio->uio_resid with uio_resid being size_t, which also rejects negative lengths — verified separately as a killed candidate).

Fix

fix.diff — add struct lock sc_lock to struct tpm_softc, init in tpm_attach, uninit at the end of tpm_detach, and take it exclusively in tpmopen/tpmclose/tpmread/tpmwrite. Mirrors FreeBSD r365144. Unvalidated on hardware (fix_status: not_testable) for the same gating reasons as above.

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.