DF-0633 — VERDICT

Verdict: NOT REPRODUCED at runtime (race did not strike within budget). Bug CONFIRMED in source.

Mechanism (the bug IS real in source)

ip_fw3_ctl_state_get (sys/net/ipfw3_basic/ip_fw3_state.c:422) runs on netisr CPU 0 (asserted by ASSERT_NETISR0 in ip_fw3_glue.c:55, called via rip_ctloutput from raw_ip.c:308). Its body, however, walks every CPU's state tree directly:

438:    for (cpu = 0; cpu < ncpus; cpu++) {
439:        state_ctx = fw3_state_ctx[cpu];
440:        RB_FOREACH(s, fw3_state_tree, &state_ctx->rb_icmp_in) {
441:            ... ioc->rule_id = s->stub->rulenum; ...

There is no netmsg_init+netisr_domsg+netisr_forwardmsg_all dispatch for cpu != mycpuid, no lwkt_token, and no lock. The per-CPU RB trees are concurrently mutated by their owning CPUs' netisrs:

• check_keep_state does RB_INSERT(fw3_state_tree, the_tree, s) at :327 whenever a new flow matches a keep-state rule.
• ip_fw3_state_cleanup_dispatch does RB_REMOVE + kfree at :547/553/ 559/565/571/577 for expired entries.

Every other cross-CPU touch in this subsystem correctly uses the netmsg pattern (:336-345 append, :361-403 flush, :540-582 cleanup, :619-632 init, :635-688 fini). Only ip_fw3_ctl_state_get breaks the contract. Note also that RB_FOREACH_SAFE is NOT used — plain RB_FOREACH, which dereferences s after the iteration macro may have already moved past it, so removal by another CPU of the node we're standing on crashes the reader. The body also dereferences s->stub->rulenum (lines 449, 464, 479, 494, 509, 524), compounding with DF-0631 if s->stub is stale.

Race outcomes: - panic from following a stale rb_node pointer into unmapped memory - UAF read when a node is kfree()'d mid-traversal - infinite loop soft-locking netisr 0 (if a half-applied tree rotation forms a cycle)

Runtime demonstration (best-effort)

Two stress runs were attempted on the 6-CPU SMP audit guest:

In both runs the guest stayed up and ipfw3 state show completed without error. The race did not strike within the test budget. This is expected for race-condition findings — the bug is probabilistic, the window per iteration is small, and the slab allocator's quarantine prevents immediate reuse so UAF reads often return benign residue.

The bug is confirmed by source-level analysis: every other cross-CPU state operation in this file uses netmsg dispatch, and the single exception (ip_fw3_ctl_state_get) is a clear omission. A maintainer reading the code will see the inconsistency immediately.

Classification: bug CONFIRMED in source, runtime race not deterministically triggered within the available iteration budget. Valid reason: the bug is a non-deterministic race; we ran the realistic trigger (concurrent state churn + state-show) and it did not strike. Source-level proof is conclusive. This is not a hard blocker per se; the bug IS reachable at runtime, just probabilistic.

Realistic impact ceiling

The trigger requires a privileged user (raw IPv4 socket with SYSCAP_NONET_RAW, root-equivalent) calling getsockopt(IPPROTO_IP, IP_FW_X, ..., IP_FW_STATE_GET). The race aggravation (concurrent packet processing on other CPUs) can be driven by an unauthenticated remote peer sending cheap TCP/UDP/ICMP packets matching any keep-state rule.

Worst-case outcome: kernel panic (local DoS from the privileged user's perspective; remote-aggravated DoS in the sense that an unauth peer can keep the trees churning while the privileged query runs). The CVSS vector AV:L/AC:H/PR:H/.../C:L/I:L/A:H correctly reflects: - high attack complexity (race timing) - privileged trigger - low confidentiality/integrity impact (read might leak a few bytes of freed memory into the user's getsockopt buffer) - high availability impact (panic)

Recommended fix

fix.diff is a substantial refactor that mirrors the netmsg-dispatch pattern used by every other cross-CPU function in this file:

1. Add ip_fw3_ctl_state_get_dispatch(netmsg_t nmsg) that runs on the owning CPU's netisr, walks ONLY the local CPU's six state trees with RB_FOREACH_SAFE, and writes results into a shared output cursor.
2. Rewrite ip_fw3_ctl_state_get to allocate the dispatch context (output cursor + remaining-count + overflow flag), netmsg_init with the dispatch function, netisr_domsg(&msg, 0), then return the total bytes written.
3. NULL-guard s->stub (defense-in-depth against DF-0631).

The macro-style EMIT keeps the per-tree loop compact and identical to the original semantics; only the synchronization model changes.

This supersedes the finding markdown's high-level proposal ("move every per-CPU RB traversal behind a netmsg, use RB_FOREACH_SAFE") by giving the concrete implementation.

PoC files

• args_overflow.c — C documentation stub.
• df0633_test.sh — stress-test driver: concurrent state churn + state-show loop.
• build.sh, run.sh — build/run wrappers.
• run.log — full stress-test output (no panic observed this run).
• env.txt — guest environment.
• fix.diff — substantial refactor implementing per-CPU netmsg dispatch.

Caveats

• The race didn't fire in two stress runs (50 and 500 iterations) on 6-CPU SMP. The bug is real but non-deterministic. A longer run, or a hardened debug build with slab poisoning + tree-rotation assertions, would likely catch it.
• The unrelated assertion: z->z_Mic_SLAB_MAGIC in _slabfree observed earlier on some ipfw3 state show invocations is a separate latent bug in the same code path; it does not directly confirm DF-0633 but does indicate the path has memory-safety issues worth investigating.
• The fix.diff is substantial (119 lines removed, 82 added) and should be reviewed carefully before merging. In particular, the dispatch writes directly to the user-supplied buffer (sopt->sopt_val); this is safe because the userland context is blocked in getsockopt, but the buffer lifetime is worth double-checking.