DF-3281 — unlocked ifg_refcnt++ + duplicate-capable group creation in if_addgroup()

File: sys/net/if.c (pass 2) Severity: Low · CWE: CWE-362 / CWE-105 · Bucket: memcorrupt (latent UAF/double-free)

Bug

if_addgroup() (sys/net/if.c:1149-1193):

1. Membership/group scans run under LK_SHARED (if.c:1160-1174), then the lock is released (if.c:1174).
2. ifg->ifg_refcnt++ (if.c:1183) executes outside any lock — a non-atomic read-modify-write on a refcount that if_delgroup_locked() decrements under LK_EXCLUSIVE (if.c:1231) — asymmetric protection, classic lost-update.
3. The find-or-create decision (group absent at if.c:1169-1172 → call if_creategroup) is also made outside exclusive locking, so two concurrent creators can both see "absent" and create duplicate same-named groups; if_creategroup() inserts without a duplicate check (if.c:1116-1118).

Lost update ⇒ refcount below real membership ⇒ if_delgroup_locked() drops it to 0 while members remain ⇒ if_destroygroup() kfrees a group with live ifgl/ifgm references ⇒ stale walks / list corruption / double-free on subsequent deletes.

Why it could NOT be reproduced from userland (honest negative)

ifioctl() holds the global ifnet_mtx across the entire ioctl body (if.c:2029 … if.c:2450), so any two SIOCAIFGROUP/SIOCDIFGROUP calls are fully serialized and can never overlap inside if_addgroup(). The unlocked increment can only race:

• if_attach()'s if_addgroup(ifp, IFG_ALL) (if.c:558) — runs without ifnet_mtx (taken later at if.c:689); concurrent driver attaches on different CPUs can overlap, and
• if_detach()'s group teardown, which runs after if_detach releasesifnet_mtx(if.c:991) underifgroup_lockonly (if.c:993-996`) — i.e. interface destroy/hot-unplug racing a concurrent SIOCAIFGROUP.

Both partners are privileged/kernel-side. PoC addgrp_race.c ran two per-interface racers (lo0, vtnet0) for 45s — ~1.9M add/del cycles each, 0 anomalous ioctl returns (run.log), guest healthy. (A per-ifp racer design is mandatory: any benign EEXIST/ENOENT error return leaks ifnet_mtx via the known DF-0272 bug and wedges the guest.)

Fix

fix.diff — if_creategroup_locked() helper + if_addgroup() restructured to perform membership scan, group lookup, creation, ifg_refcnt++, and both list insertions atomically under one LK_EXCLUSIVE section (allocations hoisted before the lock; events invoked after release; EEXIST path frees its allocations). The fix kernel (built together with DF-3280's fix, kernel #1 Sep 10 03:02:42) compiled clean and churned 3M+ group add/del cycles with zero errors (../DF-3280/churnFIX.log) — behavioral baseline for the race itself does not exist, so fix_status is not_testable.

DF-3281 VERDICT — NOT REPRODUCED from userland (ifnet_mtx serialization); static race confirmed, Low

Bottom line

The code defect is real: ifg->ifg_refcnt++ at sys/net/if.c:1183 runs outside ifgroup_lock while every other modification of ifg_refcnt is lock-protected, and the find-or-create decision for new groups is also made without exclusive locking (duplicate-group creation possible). However, a userland-only race cannot reach it: ifioctl() holds the global ifnet_mtx for the entire dispatch (if.c:2029→if.c:2450), so all SIOCAIFGROUP/SIOCDIFGROUP calls are serialized end-to-end. The remaining race partners are kernel-side:

• if_attach() → if_addgroup(ifp, IFG_ALL) at if.c:558 — runs before if_attach takes ifnet_mtx (if.c:689); concurrent device attaches (different CPUs) can overlap with each other and with an SIOCAIFGROUP.
• if_detach() group teardown at if.c:993-996 — runs after if_detach released ifnet_mtx (if.c:991), i.e. an SIOCAIFGROUP on interface X can overlap X's detach-driven group destruction. This is the realistic scenario: ifconfig <if> group <g> racing ifconfig <if> destroy (or a hot-unplug). Both sides privileged.

Attempt made

addgrp_race.c v2: two forked racers, one per interface (lo0 / vtnet0), churning SIOCAIFGROUP+SIOCDIFGROUP on the same group rgrpB at ~42k cycles/s each for 45s (~1.9M cycles per racer, ~3.8M total). Any ioctl error would have been printed as a MARKER (per-ifp churn makes benign EEXIST/ENOENT impossible — in a healthy serialized system every add succeeds and every del of a just-added member succeeds). Result (run.log): 0 markers, 0 anomalies, guest healthy. This is the expected outcome given the serialization: userland simply cannot produce two concurrent if_addgroup() executions.

An earlier v1 design (both racers churning the same ifp) is unusable on this tree: the first benign EEXIST return leaks ifnet_mtx via known DF-0272 and permanently wedges all further interface ioctls (observed live: processes stuck in D-state, wchan ifnet).

Conclusion & classification

• status not_reproduced, impact none (from userland), confidence likely for the kernel-side race (static analysis certain; a concrete hot-plug overlap was not exercised).
• Fix provided (fix.diff: if_creategroup_locked() + fully exclusive if_addgroup(); allocations hoisted out of the lock, error path frees them). Built into the same fix kernel as DF-3280 (kernel #1, Sep 10 03:02:42) — compiles clean, and 3M+ real SIOCAIFGROUP/SIOCDIFGROUP cycles ran error-free on the patched kernel (../DF-3280/churnFIX.log), so the API regression surface is exercised; but with no userland baseline, 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.