DF-4336 — twa: tw_cl_reset_ctlr entry is unsynchronized — watchdog callout and timeout threads can run the reset state machine concurrently

File: sys/dev/raid/twa/tw_cl_io.c (tw_cl_reset_ctlr, lines 1049-1130) Class: race / state-machine corruption (DoS-grade; corruption-grade interleavings not excluded) Severity: Low — hardware-gated (3ware 9000-series), operator- or fault-triggered, realistic manifestation is controller/driver wedge. Status: untested (kernel path requires twa hardware; the audit guest has none). Pure code-trace verdict.

Callers (both unserialized)

1. twa_watchdog callout — tw_osl_freebsd.c:431-478: scans OSL request deadlines, tw_cl_set_reset_needed(), then if (i_need_a_reset) { callout_reset(70*hz); tw_cl_reset_ctlr(ctlr_handle); } — runs on an MPSAFE callout thread (callout_init_mp, line 422).
2. tw_osli_fw_passthru timeout path — tw_osl_freebsd.c:1005-1011: on 60 s lksleep EWOULDBLOCK, if (!tw_cl_is_reset_needed(...)) tw_cl_reset_ctlr(...). Any number of ioctl threads can be here simultaneously.

Neither caller takes a lock, and tw_cl_reset_ctlr itself never checks ctlr->reset_in_progress as an entry guard — it only sets it (tw_cl_io.c:1060). reset_needed is cleared only at the very end (tw_cl_io.c:1124), so once the watchdog has flagged it, both the watchdog thread and (for ~60 s windows) ioctl threads can be inside tw_cl_reset_ctlr at the same time.

Interleavings

• Two concurrent tw_cli_soft_reset executions (tw_cl_io.c:1171-1276): both issue TWA_CONTROL_ISSUE_SOFT_RESET, both poll the same status register, and both race the G133 phase-1 handshake (tw_cli_find_response(..., TWA_RESET_PHASE1_NOTIFICATION_RESPONSE), tw_cl_io.c:1208-1213) — the single firmware notification is consumed by one thread; the other misses it and burns its full 60 s loop, then reports "Missed firmware handshake after soft-reset" and retries — with the second reset clobbering the first's tw_cli_init_connection (message-credit renegotiation) mid-flight.
• Interleaved queue drains (tw_cl_io.c:1071-1073) vs the RESET_Q→PENDING requeue (tw_cl_io.c:1113-1116): each drain consumes items atomically (per-op gen_lock), so no single request is drained twice, but requests requeued by reset #1 land in PENDING while reset #2 is already inside its soft-reset — tw_cli_submit_pending_queue then submits commands to a controller being reset (admitted mid-submission window, tw_cl_io.c:1064-1069).
• Premature flag clearing: reset #1 finishing sets reset_in_progress = FALSE (tw_cl_io.c:1123) while reset #2 is still resetting — new I/O (tw_cl_start_io/tw_cl_fw_passthru gate on reset_in_progress, tw_cl_io.c:142-143, 338-339) starts flowing into reset #2's window → those commands are killed by #2's drains → their OSL owners time out → more resets: a self-sustaining reset storm (system-level DoS on all I/O through the controller).

Trigger

3ware controller + any condition that makes an OSL request miss its deadline (wedged firmware, heavy queue saturation). The watchdog scans every 5 s (tw_osl_freebsd.c:479); every hung passthru ioctl thread is an independent second entry. No unprivileged trigger (device 0600 root:operator; CAM I/O pressure also qualifies).

Fix sketch

Serialize tw_cl_reset_ctlr entry: make it a test-and-set on reset_in_progress (return EBUSY to the losing caller), or guard callers with the existing gen_lock/io_lock pair; clear reset_needed before starting (not after finishing) so a second watchdog tick during an in-progress reset re-arms instead of recursing.

DF-4336 — VERDICT

Status: untested — the defect lives in the twa(4) reset state machine; exercising it requires a real 3ware 9000-series controller whose firmware hangs or whose command queue saturates past request deadlines. The audit guest has no twa hardware (/dev/twa* absent, 0 PCI vendor-0x13c1 devices; see ../DF-4335/build.log). No software-only entry point exists.

Confidence: likely — the missing entry guard is certain from the code; the severity of the resulting interleavings on real hardware is an engineering judgment (wedge/reset-storm is the realistic ceiling; no deterministic kernel-memory corruption was derivable because every queue operation is individually gen_lock-protected, tw_cl.h:270-313).

Trace

1. tw_cl_reset_ctlr (tw_cl_io.c:1049-1130) begins ctlr->reset_in_progress = TW_CL_TRUE; at 1060 and clears it at 1123 — the flag is never checked at entry, by itself or by any caller.
2. Caller A — watchdog callout thread (tw_osl_freebsd.c:469-477): i_need_a_reset = tw_cl_is_reset_needed(...); if (i_need_a_reset) { ... tw_cl_reset_ctlr(ctlr_handle); }. The callout is MPSAFE (tw_osl_freebsd.c:422-424) and takes no lock.
3. Caller B — ioctl thread in tw_osli_fw_passthru (tw_osl_freebsd.c:1005-1011): after a 60 s passthru timeout, if (!tw_cl_is_reset_needed(...)) tw_cl_reset_ctlr(...). The reset-needed test and the call are not atomic w.r.t. the watchdog setting the flag; and once the flag is set the watchdog calls unconditionally, even while caller B is mid-reset (the flag is cleared only at the END of a reset, tw_cl_io.c:1124).
4. Concurrent bodies: two tw_cli_soft_reset runs race the single firmware phase-1 notification (tw_cl_io.c:1208-1213, up to 60 s each), interleave tw_cli_init_connection credit renegotiation (tw_cl_io.c:1090-1101), interleave RESET_Q→PENDING requeues (tw_cl_io.c:1113-1116) with the other reset's drains (tw_cl_io.c:1071-1073), and the first finisher clears reset_in_progress (1123) while the second is still resetting — re-opening the submission gate (tw_cl_io.c:142-143, 338-339) into an active reset.

Impact ceiling

Controller reset storm / permanent I/O wedge (all disks behind the controller stall; each passthru owner loops on 60 s timeouts). Kernel memory-safety effects were investigated and NOT established: individual queue ops are gen_lock-serialized, drains partition requests atomically, and the mid-submission window (tw_cl_io.c:1064-1069 comment) hands the request to a post-reset controller rather than to another owner. Filed as Low (specific-hardware DoS / state-machine race), not memcorrupt.

Fix

tw_cl_reset_ctlr should decline to run when a reset is already in flight (test-and-set reset_in_progress under gen_lock, return TW_OSL_EBUSY), and reset_needed should be latched at reset start so a concurrent watchdog tick re-arms the next reset instead of recursing into this one. No fix.diff build validation possible on this guest (hardware absent) — fix_status: not_testable.

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.

(No fix.diff for this item: documented recommendation only - see Fix above.)