DragonFlyBSD Kernel Audit
DF-2721 / fix.diff
← back to finding ↓ download raw
--- a/sys/kern/kern_slaballoc.c
+++ b/sys/kern/kern_slaballoc.c
@@ -1397,7 +1397,6 @@
     int *kup;
     unsigned long size;
     SLChunk *bchunk;
-    int rsignal;
 
     logmemory_quick(free_beg);
     gd = mycpu;
@@ -1506,12 +1505,12 @@
 	 *	    cpu can rip z's storage out from under us.
 	 *
 	 *	    Bumping RCount prevents z's storage from getting
-	 *	    ripped out.
+	 *	    ripped out.  Hold the reference across the entire
+	 *	    push so that z can be safely re-read afterwards
+	 *	    (z_RSignal, z_CpuGd) - the owner can only destroy
+	 *	    the zone while z_RCount == 0.
 	 */
-	rsignal = z->z_RSignal;
-	cpu_lfence();
-	if (rsignal)
-		atomic_add_int(&z->z_RCount, 1);
+	atomic_add_int(&z->z_RCount, 1);
 
 	chunk = ptr;
 	for (;;) {
@@ -1530,18 +1529,25 @@
 	 * move the zone back on.
 	 *
 	 * We only need to deal with NULL->non-NULL RChunk transitions
-	 * and only if z_RSignal is set.  We interlock by reading rsignal
-	 * before adding our chunk to RChunks.  This should result in
-	 * virtually no IPI traffic.
+	 * and only if z_RSignal is set.
 	 *
-	 * We can use a passive IPI to reduce overhead even further.
+	 * DF-2721 fix: the old pre-read interlock (sampling rsignal
+	 * before pushing the chunk) is not airtight.  If the owner
+	 * sets z_RSignal and removes the zone from the ZoneAry in
+	 * between our sample and our push, no IPI is ever sent and
+	 * the zone becomes permanently stranded (leaked) with its
+	 * remotely freed chunks in z_RChunks, unreachable by any
+	 * recovery path.  Make the decision *after* the push by
+	 * re-reading z_RSignal, which is safe because we hold
+	 * z_RCount.
 	 */
-	if (bchunk == NULL && rsignal) {
+	cpu_lfence();
+	if (bchunk == NULL && z->z_RSignal) {
 	    logmemory(free_request, ptr, type,
 		      (unsigned long)z->z_ChunkSize, 0);
 	    lwkt_send_ipiq_passive(z->z_CpuGd, kfree_remote, z);
 	    /* z can get ripped out from under us from this point on */
-	} else if (rsignal) {
+	} else {
 	    atomic_subtract_int(&z->z_RCount, 1);
 	    /* z can get ripped out from under us from this point on */
 	}