⬢ DragonFlyBSD Kernel Audit
DF-5420 / fix.diff
← back to finding ↓ download raw
--- a/sys/platform/pc64/x86_64/pmap_inval.c
+++ b/sys/platform/pc64/x86_64/pmap_inval.c
@@ -277,16 +277,32 @@
 		pmap = kernel_pmap;
 
 	/*
+	 * DF-5420 (validated fix):
+	 *  (1) Acquire the interlock BEFORE sampling pm_active.  The old
+	 *      code sampled the mask first, so a cpu concurrently switching
+	 *      into the pmap could slip its CPULOCK_EXCL test in before we
+	 *      set it and never see the invalidation.
+	 *  (2) Never use the single-cpu shortcut for user pmaps when we
+	 *      are modifying a PTE: pm_active is not a sound indicator that
+	 *      other cpus have no live TLB entries for the pmap (a
+	 *      descheduled/preempted cpu retains its TLB; the protocol
+	 *      relies on its reactivation %cr3 reload to flush, which is
+	 *      not guaranteed under virtualization and is too fragile to
+	 *      hang PTE teardown correctness on).
+	 */
+	pmap_inval_init(pmap);
+
+	/*
 	 * Shortcut single-cpu case if possible.
 	 */
+	if (pmap != kernel_pmap && ptep != NULL)
+		goto noshortcut;
 	if (CPUMASK_CMPMASKEQ(pmap->pm_active, gd->gd_cpumask) &&
 	    pmap_inval_force_nonopt == 0) {
 		/*
 		 * Convert to invltlb if there are too many pages to
 		 * invlpg on.
 		 */
-		if (pmap->pm_flags & PMAP_MULTI)
-			pmap_inval_init(pmap);
 		if (npgs == 1) {
 			if (ptep)
 				opte = atomic_swap_long(ptep, npte);
@@ -314,11 +330,11 @@
 				--npgs;
 			}
 		}
-		if (pmap->pm_flags & PMAP_MULTI)
-			pmap_inval_done(pmap);
+		pmap_inval_done(pmap);
 
 		return opte;
 	}
+noshortcut:
 
 	/*
 	 * We need a critical section to prevent getting preempted while
@@ -327,8 +343,9 @@
 	 *
 	 * tsc_target is our watchdog timeout that will attempt to recover
 	 * from a lost IPI.  Set to 1/16 second for now.
+	 *
+	 * (DF-5420: interlock already acquired above.)
 	 */
-	pmap_inval_init(pmap);
 	info = &invinfo[cpu];
 
 	/*
@@ -372,8 +389,15 @@
 	info->mode = INVSTORE;
 
 	tmpmask = pmap->pm_active;	/* volatile (bits may be cleared) */
-	if (pmap_inval_force_allcpus)
+	if (pmap_inval_force_allcpus ||
+	    (pmap != kernel_pmap && ptep != NULL)) {
+		/*
+		 * DF-5420: PTE-modifying user-pmap operations must not
+		 * trust pm_active (descheduled cpus retain live TLB
+		 * entries); broadcast to all active cpus instead.
+		 */
 		tmpmask = smp_active_mask;
+	}
 	cpu_ccfence();
 	CPUMASK_ANDMASK(tmpmask, smp_active_mask);
 
@@ -426,6 +450,23 @@
 	 */
 	ATOMIC_CPUMASK_NANDBIT(smp_invmask, cpu);
 	write_rflags(rflags);
+
+	/*
+	 * DF-5420: wait for target cpus to finish their trailing
+	 * invalidations before returning.  The old code returned after
+	 * clearing only our own done bit, leaving a window where the PTE
+	 * was already torn down but target TLBs still held the old
+	 * translation.
+	 */
+	{
+		int spins = 0;
+
+		while (CPUMASK_TESTNZERO(info->done)) {
+			if (++spins > 100000000)
+				break;		/* defensive */
+			cpu_pause();
+		}
+	}
 	pmap_inval_done(pmap);
 
 	return opte;
@@ -455,31 +496,9 @@
 		pmap = kernel_pmap;
 
 	/*
-	 * Shortcut single-cpu case if possible.
-	 */
-	if (CPUMASK_CMPMASKEQ(pmap->pm_active, gd->gd_cpumask) &&
-	    pmap_inval_force_nonopt == 0) {
-		if (pmap->pm_flags & PMAP_MULTI)
-			pmap_inval_init(pmap);
-		if (atomic_cmpset_long(ptep, opte, npte)) {
-			if (va == (vm_offset_t)-1)
-				cpu_invltlb();
-			else
-				cpu_invlpg((void *)va);
-			if (pmap->pm_flags & PMAP_MULTI)
-				pmap_inval_done(pmap);
-			return 1;
-		} else {
-			if (pmap->pm_flags & PMAP_MULTI)
-				pmap_inval_done(pmap);
-			return 0;
-		}
-	}
-
-	/*
-	 * We need a critical section to prevent getting preempted while
-	 * we setup our command.  A preemption might execute its own
-	 * pmap_inval*() command and create confusion below.
+	 * DF-5420 (validated fix): acquire the interlock BEFORE sampling
+	 * pm_active, and do not use the single-cpu shortcut for user pmaps
+	 * (see pmap_inval_smp).
 	 */
 	pmap_inval_init(pmap);
 	info = &invinfo[cpu];
@@ -522,8 +541,15 @@
 	info->success = 0;
 
 	tmpmask = pmap->pm_active;	/* volatile */
-	if (pmap_inval_force_allcpus)
+	if (pmap_inval_force_allcpus ||
+	    (pmap != kernel_pmap && ptep != NULL)) {
+		/*
+		 * DF-5420: PTE-modifying user-pmap operations must not
+		 * trust pm_active (descheduled cpus retain live TLB
+		 * entries); broadcast to all active cpus instead.
+		 */
 		tmpmask = smp_active_mask;
+	}
 	cpu_ccfence();
 	CPUMASK_ANDMASK(tmpmask, smp_active_mask);
 	CPUMASK_ORBIT(tmpmask, cpu);
@@ -555,6 +581,20 @@
 
 	ATOMIC_CPUMASK_NANDBIT(smp_invmask, cpu);
 	write_rflags(rflags);
+
+	/*
+	 * DF-5420: wait for target cpus to finish their trailing
+	 * invalidations before returning (see pmap_inval_smp).
+	 */
+	{
+		int spins = 0;
+
+		while (CPUMASK_TESTNZERO(info->done)) {
+			if (++spins > 100000000)
+				break;		/* defensive */
+			cpu_pause();
+		}
+	}
 	pmap_inval_done(pmap);
 
 	return success;