⬢ DragonFlyBSD Kernel Audit
DF-4482 / fix.diff
← back to finding ↓ download raw 
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
/* ==============================================================
 * DF-4482 fix.diff — BEST ATTEMPT, DEMONSTRABLY INSUFFICIENT.
 * Fixes the two provable UAF arms in the waitlist machinery:
 *   (1) drain walks/relaunches nodes freed by inline completion
 *   (2) append walks the same freed chain
 * but the panic ALSO persists with this applied (run.4.log) — at
 * least one more lifetime bug remains in the revive-conflict

 * relaunch path (see VERDICT.md, arm analysis).  The real fix is a

 * redesign: drained requests must not be launched while the walker

 * still references them, and freerq() must be waitlist-aware (or


 * completions must be deferred).  Kept for the record.
 * ============================================================== */
diff -urN a/sys/dev/raid/vinum/vinumrequest.c b/sys/dev/raid/vinum/vinumrequest.c
--- a/sys/dev/raid/vinum/vinumrequest.c	2026-09-20 18:17:04.340371043 +0000
+++ b/sys/dev/raid/vinum/vinumrequest.c	2026-09-20 18:17:10.540292314 +0000
@@ -327,16 +327,18 @@
     if ((rq->flags & XFR_REVIVECONFLICT)		    /* possible revive conflict */
     &&(!reviveok)) {					    /* and we don't want to do it now, */
 	struct sd *sd;
-	struct request *waitlist;			    /* point to the waitlist */
 
 	sd = &SD[rq->sdno];
-	if (sd->waitlist != NULL) {			    /* something there already, */
-	    waitlist = sd->waitlist;
-	    while (waitlist->next != NULL)		    /* find the end */
-		waitlist = waitlist->next;
-	    waitlist->next = rq;			    /* hook our request there */
-	} else
-	    sd->waitlist = rq;				    /* hook our request at the front */
+	/*
+	 * Push at the head with a cmpset loop.  NEVER walk the chain:
+	 * revive_block() steals and frees these requests while draining,
+	 * so chain nodes can vanish at any time.
+	 */
+	do {
+	    rq->next = sd->waitlist;
+	} while (atomic_cmpset_long(
+	    (volatile unsigned long *)&sd->waitlist,
+	    (unsigned long)rq->next, (unsigned long)rq) == 0);
 
 #ifdef VINUMDEBUG
 	if (debug & DEBUG_REVIVECONFLICT) {
diff -urN a/sys/dev/raid/vinum/vinumrevive.c b/sys/dev/raid/vinum/vinumrevive.c
--- a/sys/dev/raid/vinum/vinumrevive.c	2026-09-20 18:17:04.300371551 +0000
+++ b/sys/dev/raid/vinum/vinumrevive.c	2026-09-20 18:17:04.328371196 +0000
@@ -69,6 +69,7 @@
     int paritysd = 0;					    /* set if this is the parity stripe */
     struct rangelock *lock;				    /* for locking */
     vinum_off_t stripeoffset;				    /* offset in stripe */
+    struct request *rqwait;				    /* drained waitlist */
 
     plexblkno = 0;					    /* to keep the compiler happy */
     sd = &SD[sdno];
@@ -195,9 +196,22 @@
 	}
 	if (lock)					    /* we took a lock, */
 	    unlockrange(sd->plexno, lock);		    /* give it back */
-	while (sd->waitlist) {				    /* we have waiting requests */
+	/*
+	 * Atomically steal the entire waitlist.  Concurrent
+	 * launch_requests() (XFR_REVIVECONFLICT) appends requests with a
+	 * cmpset loop on the list head and never walks the chain, so the
+	 * stolen chain is private to us: nothing else can reference these
+	 * requests while we launch (and synchronously complete and free)
+	 * them.
+	 */
+	rqwait = (struct request *)
+	    atomic_swap_long((volatile unsigned long *)&sd->waitlist, 0);
+	while (rqwait) {				    /* we have waiting requests */
+	    struct request *rq = rqwait;		    /* dequeue head first */
+
+	    rqwait = rq->next;				    /* unlink before launch */
+	    rq->next = NULL;
 #ifdef VINUMDEBUG
-	    struct request *rq = sd->waitlist;
 	    cdev_t dev;
 
 	    if (debug & DEBUG_REVIVECONFLICT) {
@@ -213,8 +227,7 @@
 		    rq->bio->bio_buf->b_bcount);
 	    }
 #endif
-	    launch_requests(sd->waitlist, 1);		    /* do them now */
-	    sd->waitlist = sd->waitlist->next;		    /* and move on to the next */
+	    launch_requests(rq, 1);			    /* do it now */
 	}
     }
     Free(bp->b_data);