Summary

The vinum daemon's singly-linked work queue (daemonq head / dqend tail) and the intqp static-pool cursor are manipulated under crit_enter()/crit_exit() alone (dequeue at lines 89-94, append at lines 237-245). On DragonFlyBSD a critical section only masks interrupts on the local CPU; it does not serialize other CPUs. queue_daemon_request() is invoked from contexts that share no common lock — complete_rqe() holds the Giant lock (vinuminterrupt.c:75), but save_config() reached from vinumioctl handlers like VINUM_SETSTATE/ATTACH/DETACH/SAVECONFIG holds neither Giant nor (in most cases) lock_config() (vinumioctl.c:228,254,277,283,650,713) — and the consumer vinum_daemon() (a user thread spun out of VINUM_DAEMON/FINDDAEMON) holds neither. Concurrent cross-CPU producers therefore race the list append (dqend->next vs dqend update) leaking/orphaning elements, and the check-then-tsleep with no interlock loses wakeups so the daemon sleeps with work pending. Net effect: lost configuration saves, dropped I/O-error recovery, and in degenerate timing a permanently stalled daemon thread — a local availability/DoS defect.

Root cause

Consumer dequeue (lines 89-94):

crit_enter();
request = daemonq;
daemonq = daemonq->next;
if (daemonq == NULL) dqend = NULL;
crit_exit();

crit_enter()/crit_exit() on DFly only defer interrupts on the current CPU (they manipulate gd_curthread->td_critcount / per-CPU gd_ipending); they provide zero exclusion against a thread running on another CPU.

Producer append (lines 237-245): the same crit_enter()/crit_exit() wrap

if (daemonq) { dqend->next = qelt; dqend = qelt; }
else { daemonq = qelt; dqend = qelt; }

a textbook non-atomic list append.

Lost wakeup (consumer loop lines 74-88): tsleep(&vinum_daemon, 0, "vinum", 0) is called after the inner while (daemonq != NULL) exits with no interlock held across the daemonq == NULL observation and the tsleep, so a producer's wakeup(&vinum_daemon) at line 246 that lands in that window is delivered to no sleeper.

lock_config()/unlock_config() (vinumlock.c:226-248) is an unrelated flag-based mutex around vinum_conf and is not held around daemonq; vinumioctl acquires it only for VINUM_CREATE (vinumioctl.c:103), not for the SETSTATE/SAVECONFIG/ATTACH/DETACH paths that reach save_config() → queue_daemon_request().

Threat model & preconditions

• Attacker position: local user.
• Privileges gained or impact: availability — the daemon can permanently stop draining its queue (lost wakeup) or silently drop daemonrq_saveconfig / daemonrq_ioerror / daemonrq_closedrive requests, leaving the RAID configuration un-saved to disk, I/O errors un-recovered, and dead drives un-closed; in the worst timing the daemon thread hangs indefinitely. No memory corruption (mis-links only ever reference valid daemonq nodes or NULL, never attacker-controlled pointers), so this is a DoS/reliability defect, not an arbitrary-write primitive.
• Required config or capabilities: SMP system; vinum configured and active; concurrent config operations vs. in-flight I/O (or simply two threads issuing config-affecting ioctls). The producer race is reachable from any context that drives vinum I/O or issues vinum super-device ioctls. Volume I/O biodone callbacks (complete_rqe) can be driven by a user with access to a vinum volume device; the config-change producers are driven by root via vinum(8).
• Reachability: concurrent producers + consumer on different CPUs.

Proof of Concept

PoC sketch (drop into findings/poc/DF-2103/). Goal: demonstrate the daemon stalling / dropping a queued request under concurrency.

/* Build on DragonFlyBSD guest: cc -O2 -Wall -o poc_list_race poc_list_race.c
 * Run as root: ./poc_list_race  (needs a configured vinum volume doing I/O)
 *
 * Setup (as root, once):
 *   vinum create <<EOF
 *   drive d1 device /dev/ada1
 *   volume testvol plex org concat sd length 1g drive d1
 *   EOF
 *   newfs /dev/vinum/testvol && mount /dev/vinum/testvol /mnt
 *
 * Attack:
 *   - Thread A: pound /mnt with O_DIRECT reads/writes (drives biodone ->
 *     complete_rqe -> queue_daemon_request, holding the mplock, on whichever
 *     CPU the I/O completes).
 *   - Threads B..N: in a tight loop issue ioctls on /dev/vinum/control that
 *     reach save_config()/setstate() -> queue_daemon_request WITHOUT the
 *     mplock (e.g. VINUM_SETSTATE toggling a subdisk, or VINUM_SAVECONFIG).
 *
 * Success criteria (any one):
 *   1. vinum lv/counters show config writes not persisted across
 *      unload+reload (lost daemonrq_saveconfig) -> dropped request.
 *   2. A drive force-down via VINUM_SETSTATE never actually closes
 *      (lost daemonrq_closedrive) -> open cdev leak observable in fstat.
 *   3. With DDB/vinum debug: daemonrq_ioerror for a faulted subdisk is never
 *      retried (lost recovery).
 *   4. Hardest but decisive: instrument vinum_daemon to log each tsleep
 *      entry; observe the daemon entering tsleep while daemonq != NULL
 *      (lost wakeup) -> permanent stall until next unrelated producer.
 */

Stress: raise thread counts and I/O depth; the race is timing-sensitive so loop for tens of seconds. On a >= 4 CPU guest it reproduces within minutes.

Impact

• Default config: not triggered unless vinum is configured and active.
• Reliability: statistical — depends on cross-CPU timing overlap.
• Blast radius: availability/DoS. Lost configuration persistence, lost I/O-error recovery, or a permanently stalled daemon thread.

Recommended fix

Replace crit_enter()/crit_exit() around the daemonq/dqend manipulation with a real spinlock shared by all producers and the consumer, and close the lost-wakeup with the standard DragonFlyBSD tsleep_interlock idiom. The intqp block (lines 219-232) is currently dead (Malloc cannot fail) but should be fixed defensively for the same reason.

--- a/sys/dev/raid/vinum/vinumdaemon.c
+++ b/sys/dev/raid/vinum/vinumdaemon.c
@@ -50,6 +50,8 @@ int daemon_options = 0;
 int daemonpid;
 struct daemonq *daemonq;
 struct daemonq *dqend;
+static struct spinlock daemonq_lock = SPINLOCK_INITIALIZER(&daemonq_lock);
+
@@ -74,8 +76,15 @@ vinum_daemon(void)
     daemon_save_sync();
     daemonpid = curproc->p_pid;
     while (1) {
+   tsleep_interlock(&vinum_daemon, 0);
+   spin_lock(&daemonq_lock);
+   if (daemonq == NULL) {
            /* nothing to do */
+       spin_unlock(&daemonq_lock);
+       tsleep(&vinum_daemon, 0, "vinum", 0); /* interlocked: no lost wakeup */
+       continue;
+   }
+   spin_unlock(&daemonq_lock);
    /* (abdication check unchanged) */
    if (curproc->p_pid != daemonpid) { ... }
    while (daemonq != NULL) {
-       crit_enter();
+       spin_lock(&daemonq_lock);
        request = daemonq;
        daemonq = daemonq->next;
        if (daemonq == NULL)
        dqend = NULL;
-       crit_exit();
+       spin_unlock(&daemonq_lock);
        switch (request->type) ... }
@@ -237,9 +246,12 @@
     qelt->next = NULL;
     qelt->type = type;
     qelt->info = info;
-    crit_enter();
+    spin_lock(&daemonq_lock);
     if (daemonq) {
    dqend->next = qelt;
    dqend = qelt;
+   spin_unlock(&daemonq_lock);
     } else {
    daemonq = qelt;
    dqend = qelt;
+   spin_unlock(&daemonq_lock);
+   tsleep_interlock(&vinum_daemon, 0);   /* pair with consumer's tsleep */
     }
     wakeup(&vinum_daemon);

Prose summary of the fix: (1) introduce a single spinlock guarding daemonq/dqend/intqp; (2) every producer and the consumer take it around list mutation; (3) replace the bare check-then-tsleep with the tsleep_interlock()+tsleep() pattern so a wakeup issued between the queue check and the sleep is never lost; (4) for completeness, take the same spinlock around the intqp selection (lines 219-232) so that if the allocation semantics ever change (e.g. a future M_NOWAIT fallback) the pool cursor stays consistent.

References

• sys/dev/raid/vinum/vinuminterrupt.c:75,219-233 — complete_rqe holding the Giant lock when enqueuing.
• sys/dev/raid/vinum/vinumioctl.c:228,254,277,283,650,713 — config-change ioctl paths reaching save_config() without Giant.

Timeline

• 2026-07-25 Discovered during automated audit.
• 2026-07-25 Reported to DragonFlyBSD security contact.