Fix DF-3264 (pf_state use-after-free: pf_send_tcp drops pf_token mid-state-operation) Mechanism: the packet path processes states holding pf_token *shared* (pf_check_in/out). pf_send_tcp() releases pf_token around ip_output() (pf.c:2254-2256, 2274-2276, 2289-2291). Callers on the state path keep using the struct pf_state after that call returns — pf_test_state_tcp's synproxy branches (pf.c:4918-4924, 4948-4954, 4963-4986), the bad-state RST path (pf.c:4738-4746), the create-state synproxy send (pf.c:4294-4298 -> pf_test tail pf.c:6712-6812), and pf_unlink_state (pf.c:1489-1497). During the token-drop window the pf_purge_thread can acquire the token exclusively, decide the just-expired state is garbage, and unlink+free it. When ip_output returns, the netisr thread continues on freed memory (lockmgr on freed lk, seq bookkeeping writes, pfsync_update_state, counter updates) — a remote-triggerable UAF. Design of the fix: every state-path caller of pf_send_tcp() that touches the state afterwards already holds (or now holds) the per-state lock s->lk exclusively. pf_free_state() (purge thread only) try-locks s->lk before freeing; if the lock is unavailable the state is left on the state list in the PFTM_UNLINKED two-phase state and is retried on the next sweep (the same deferral pattern already used for pfsync bulk references at pf.c:1516-1519). Three hunks: 1. pf_free_state: try-lock & defer. 2. pf_create_state: hold s->lk across the synproxy pf_send_tcp. 3. pf_test: hold s->lk across the SYNPROXY_DROP tail processing. --- a/sys/net/pf/pf.c +++ b/sys/net/pf/pf.c @@ -1514,6 +1514,22 @@ pf_free_state(struct pf_state *cur) if (pfsyncif != NULL && (pfsyncif->sc_bulk_send_next == cur || pfsyncif->sc_bulk_terminator == cur)) return; KKASSERT(cur->timeout == PFTM_UNLINKED); + + /* + * A packet thread may still be operating on this state: + * pf_send_tcp() temporarily releases pf_token around ip_output(), + * and its callers keep using the state afterwards while holding + * cur->lk exclusively. If we cannot get the lock without + * blocking, the state is in flight — leave it on the list + * (already unlinked, PFTM_UNLINKED) and retry next sweep. + */ + if (lockmgr(&cur->lk, LK_EXCLUSIVE | LK_NOWAIT) != 0) { + DPFPRINTF(PF_DEBUG_MISC, + ("pf_free_state: state in flight, deferring free\n")); + return; + } + /* * decrements in rule should be ok, token is * held exclusively in this code path. @@ -4283,10 +4299,12 @@ pf_create_state(...) s->src.seqhi = htonl(karc4random()); /* Find mss option */ mss = pf_get_mss(m, off, th->th_off, pd->af); mss = pf_calc_mss(pd->src, pd->af, mss); mss = pf_calc_mss(pd->dst, pd->af, mss); s->src.mss = mss; s->state_flags &= ~PFSTATE_CREATEINPROG; + lockmgr(&s->lk, LK_EXCLUSIVE); pf_send_tcp(r, pd->af, pd->dst, pd->src, th->th_dport, th->th_sport, s->src.seqhi, ntohl(th->th_seq) + 1, TH_SYN|TH_ACK, 0, s->src.mss, 0, 1, 0, NULL, NULL); + lockmgr(&s->lk, LK_RELEASE); REASON_SET(&reason, PFRES_SYNPROXY); return (PF_SYNPROXY_DROP); } --- a/sys/net/pf/pf.c +++ b/sys/net/pf/pf.c @@ -6710,6 +6710,14 @@ pf_test(...) done: + /* + * SYNPROXY_DROP: pf_create_state()/pf_test_state_tcp() sent a + * generated packet with pf_send_tcp(), which releases pf_token + * around ip_output(); the state could have been purged in that + * window. Pin it with its lock while we finish the tail + * processing below. + */ + int locked = 0; + if (action == PF_SYNPROXY_DROP && s != NULL) { + lockmgr(&s->lk, LK_EXCLUSIVE); + locked = 1; + } if (action == PF_PASS && h->ip_hl > 5 && !((s && s->state_flags & PFSTATE_ALLOWOPTS) || r->allow_opts)) { @@ -6833,6 +6841,9 @@ if (action == PF_SYNPROXY_DROP) { + if (locked) + lockmgr(&s->lk, LK_RELEASE); m_freem(*m0); *m0 = NULL; action = PF_PASS; } else if (r->rt) { /* pf_route can free the mbuf causing *m0 to become NULL */ pf_route(m0, r, dir, kif->pfik_ifp, s, &pd); + if (locked) + lockmgr(&s->lk, LK_RELEASE); } Note: hunk 3 keeps pf_free_state's deferral meaningful for the create-state path; hunk 2/3 must both be applied. A stricter long-term remedy is to stop dropping pf_token inside pf_send_tcp() (queue the generated packet instead), which would make every caller safe without per-state locking; that is the recommended upstream direction.