⬢ DragonFlyBSD Kernel Audit
DF-2755 / 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
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
--- a/sys/kern/vfs_vnops.c
+++ b/sys/kern/vfs_vnops.c
@@ -1045,12 +1045,25 @@
 				break;
 			}
 
-			/* Get rid of reference to old control tty */
+			/*
+			 * Atomically take ownership of the s_ttyvp slot
+			 * transition (DF-2755).  The cmpset can succeed for
+			 * exactly one racing thread per old value, keeping
+			 * the vref/vrele pairing exact against concurrent
+			 * twins, the devfs half-close, ttyclosesession()
+			 * and the revoke scan.
+			 */
 			ovp = sess->s_ttyvp;
 			vref(vp);
-			sess->s_ttyvp = vp;
-			if (ovp)
-				vrele(ovp);
+			if (atomic_cmpset_ptr(&sess->s_ttyvp,
+					      (uintptr_t)ovp,
+					      (uintptr_t)vp)) {
+				/* release slot ref on old control tty */
+				if (ovp)
+					vrele(ovp);
+			} else {
+				vrele(vp);	/* lost race, drop spec. ref */
+			}
 			rel_mplock();
 		}
 		break;
--- a/sys/vfs/devfs/devfs_vnops.c
+++ b/sys/vfs/devfs/devfs_vnops.c
@@ -1150,8 +1150,14 @@
 		opencount = 0;
 	}
 
-	if (p && vp->v_opencount <= 1 && vp == p->p_session->s_ttyvp) {
-		p->p_session->s_ttyvp = NULL;
+	/*
+	 * Detect the last close on a controlling terminal and clear the
+	 * session (half-close).  The cmpset makes the clear atomic with
+	 * the check so exactly one racer releases the slot reference
+	 * (DF-2755).
+	 */
+	if (p && vp->v_opencount <= 1 &&
+	    atomic_cmpset_ptr(&p->p_session->s_ttyvp, (uintptr_t)vp, 0)) {
 		vrele(vp);
 	}
 
@@ -1601,13 +1607,23 @@
 		}
 
 		/*
-		 * Get rid of reference to old control tty
+		 * Get rid of reference to old control tty.
+		 *
+		 * Atomically take ownership of the s_ttyvp slot transition
+		 * (DF-2755): the cmpset succeeds for exactly one racing
+		 * thread per old value, keeping the vref/vrele pairing
+		 * exact against concurrent twins, the devfs half-close,
+		 * ttyclosesession() and the revoke scan.
 		 */
 		ovp = sess->s_ttyvp;
 		vref(vp);
-		sess->s_ttyvp = vp;
-		if (ovp)
-			vrele(ovp);
+		if (atomic_cmpset_ptr(&sess->s_ttyvp,
+				      (uintptr_t)ovp, (uintptr_t)vp)) {
+			if (ovp)
+				vrele(ovp);
+		} else {
+			vrele(vp);	/* lost race, drop speculative ref */
+		}
 	}
 
 out:
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -2029,8 +2029,8 @@
 	 * uses curproc instead of p.
 	 */
 	if (p->p_session && info->type == DTYPE_VNODE &&
-	    info->data == p->p_session->s_ttyvp) {
-		p->p_session->s_ttyvp = NULL;
+	    atomic_cmpset_ptr(&p->p_session->s_ttyvp,
+			      (uintptr_t)info->data, 0)) {
 		vrele(info->data);
 	}
 
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -362,9 +362,15 @@
 		}
 
 		/*
-		 * Close and revoke as needed
+		 * Close and revoke as needed.  cmpset keeps the clear
+		 * atomic with the re-check so the slot reference is
+		 * released exactly once (DF-2755).
 		 */
-		sp->s_ttyvp = NULL;
+		if (atomic_cmpset_ptr(&sp->s_ttyvp, (uintptr_t)vp, 0) == 0) {
+			vn_unlock(vp);
+			vdrop(vp);
+			goto retry;
+		}
 		if (vp->v_flag & VCTTYISOPEN) {
 			vclrflags(vp, VCTTYISOPEN);
 			VOP_CLOSE(vp, FREAD|FWRITE, NULL);
@@ -374,7 +380,8 @@
 			vrevoke(vp, proc0.p_ucred);
 		vdrop(vp);
 	} else {
-		sp->s_ttyvp = NULL;
+		if (atomic_cmpset_ptr(&sp->s_ttyvp, (uintptr_t)vp, 0) == 0)
+			goto retry;
 	}
 	vrele(vp);
 	lwkt_reltoken(&prg->proc_token);