Summary

pfi_update_status (DIOCGETSTATUS/DIOCCLRSTATUS) bcopy's p->pfik_group->ifg_members into a stack TAILQ head WITHOUT the ifgroup lock and only then takes LK_SHARED to walk it. Group membership mutations run under LK_EXCLUSIVE and kfree the removed ifg_member; an entry that is head-of-list at bcopy time can be freed before the walk takes the lock. NOT reproduced in >5.3M adversarial status calls (bcopy->lock window a few instructions; most hits read freed-but-intact memory silently) - honest not_reproduced; code-certain lifetime defect, root-vs-root. Fix: take the ifgroup lock before the snapshot and hold it through the walk.

PoC verification

Evidence pack

findings/poc/DF-3420 · 4 files

File	Type	Size
README.md	—	2.7 KB	↓ raw
setstatif.c	—	923 B	view raw
statloop.c	—	642 B	view raw
verdict.json	—	3.1 KB	view raw

README.md

↓ download raw

DF-3420 — pfi_update_status() copies the group member-list head outside the ifgroup lock (pf_if.c:722-724)

pfi_update_status() (pf_if.c:706-761, reached from DIOCGETSTATUS / DIOCCLRSTATUS, pf_ioctl.c:1792/1821) snapshots a group kif's member list head without holding the ifgroup lock:

722 if (p->pfik_group != NULL) {
723     bcopy(&p->pfik_group->ifg_members, &ifg_members,
724         sizeof(ifg_members));          /* <-- NO LOCK */
725 } else { ...stack list... }
736 ifgroup_lockmgr(LK_SHARED);
737 TAILQ_FOREACH(ifgm, &ifg_members, ifgm_next) { ... }

Group membership mutations (if_addgroup/if_delgroup, if.c:1186/1224) run under ifgroup_lockmgr(LK_EXCLUSIVE) and kfree the removed ifg_member. An entry that is head-of-list at bcopy time can be removed and freed before the walker takes the shared lock; the walk then dereferences freed memory (ifgm->ifgm_ifp at pf_if.c:738).

Verification attempts on the 6-cpu INVARIANTS guest (all as root, which is also the only reachable trigger since /dev/pf is 0600 and group changes are admin operations):

round	DIOCGETSTATUS iterations	membership churn	result
1	30,000 (pfctl -si)	3,000 tap create/destroy	up, no panic
2	2,000,000 (tight ioctl loop)	8,000 tap create/destroy	up, no panic
3	3,000,000 (tight ioctl loop, head-of-list pattern: churn the oldest member tap0)	6,000 tap0 create/destroy	up, no panic

5M+ iterations across three churn patterns never hit the (few-instruction) bcopy->lock window; additionally, most hits would read a freed-but-intact chunk and be silent. not_reproduced — the lifetime defect is code-certain (unlocked snapshot of a mutable list head that is walked for pointers), but no observable trigger was demonstrated in ~8 minutes of adversarial stress. Root-vs-root only.

Fix

Take the lock before the snapshot (and hold it across the walk):

--- a/sys/net/pf/pf_if.c
+++ b/sys/net/pf/pf_if.c
@@ -719,12 +719,13 @@ pfi_update_status(const char *name, struct pf_status *pfs)
    if (p == NULL) {
        crit_exit();
        return;
    }
+   ifgroup_lockmgr(LK_SHARED);
    if (p->pfik_group != NULL) {
        bcopy(&p->pfik_group->ifg_members, &ifg_members,
            sizeof(ifg_members));
    } else {
        /* build a temporary list for p only */
        bzero(&p_member, sizeof(p_member));
        p_member.ifgm_ifp = p->pfik_ifp;
@@ -733,8 +734,6 @@ pfi_update_status(const char *name, struct pf_status *pfs)
    if (pfs) {
        bzero(pfs->pcounters, sizeof(pfs->pcounters));
        bzero(pfs->bcounters, sizeof(pfs->bcounters));
    }
-   ifgroup_lockmgr(LK_SHARED);
    TAILQ_FOREACH(ifgm, &ifg_members, ifgm_next) {

(with the matching LK_RELEASE kept at the existing exit point).

Fix verification

not_testable

baseline no→ patch + rebuild →patched clean

Baseline never reproduced, so fix validation is not meaningful; the lock-scope fix in README.md removes the unlocked snapshot by construction.

per-fix-DF-3420

Detail

Evidence (decisive lines)

['findings/poc/DF-3420/README.md — defect description, stress matrix (3 rounds, >5.3M iterations), proposed lock-scope fix', 'findings/poc/DF-3420/setstatif.c — sets pf status-if to the group name so DIOCGETSTATUS exercises the group branch', 'findings/poc/DF-3420/statloop.c — tight DIOCGETSTATUS loop used for rounds 2-3', 'sys/net/pf/pf_if.c:722-724 — unlocked bcopy of ifg_members head', 'sys/net/pf/pf_if.c:736-738 — walk of the snapshot under LK_SHARED', 'sys/net/if.c:1186,1224 — membership insert/remove under LK_EXCLUSIVE, kfree of ifg_member']

PoC changes

Authored from scratch: setstatif.c (DIOCSETSTATUSIF to a group name), statloop.c (tight ioctl loop; first version used a guessed struct size and failed with 'Operation not supported' - fixed by compiling against the guest's /usr/src pfvar.h), churn scripts cycling tap create/destroy including the oldest-member/head-of-list pattern.

Verified recommended fix

Acquire ifgroup_lockmgr(LK_SHARED) before the bcopy of ifg_members and hold it through the walk; see fix sketch in README.md

Verdict

Code-certain lifetime defect: pfi_update_status() bcopy's the pf group ifg_members TAILQ head without the ifgroup lock (pf_if.c:722-724) and then walks that snapshot under LK_SHARED (pf_if.c:736-758); a concurrently removed+kfree'd ifg_member (if.c:1224, under LK_EXCLUSIVE) that was head-of-list at snapshot time is dereferenced after free. Three adversarial stress rounds on the 6-cpu INVARIANTS guest (30k pfctl -si + 3k tap churn; 2M tight DIOCGETSTATUS + 8k churn; 3M tight DIOCGETSTATUS + 6k oldest-member churn targeting the head-entry pattern) totalled >5.3M status calls and never hit the few-instruction window; no panic, no corruption observed. Root-vs-root only (/dev/pf 0600, group ops are admin). Filed Low: hardening-scope lifetime bug, realistic impact negligible.

pfi_update_status() snapshots the pf-group member-list head outside the ifgroup lock, then walks the snapshot — use-after-free of ifg_member under concurrent group churn