⬢ DragonFlyBSD Kernel Audit
DF-2942 / 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
# DF-2942 fix: distinguish initializing from terminating in the sysref
# negative refcount space.
#
# sysref_activate()'s KASSERT (kern_sysref.c:280) accepts
# count == -0x40000000, which is both the post-sysref_alloc initializing
# state AND the termination-in-progress state entered by _sysref_put's
# 1 -> -0x40000000 cmpset (:323). An activate landing in the termination
# window therefore resurrects a terminating object with the layer's
# blessing (demonstrated: findings/poc/DF-2942/run.log -- double
# termination, zero diagnostics; run.1.unbounded.log -- unbounded
# re-termination ending in a stack-exhaustion double fault).
#
# This diff adds SRF_TERMINATING, sets it when termination begins, clears
# it when (re)initializing, and extends sysref_activate's assert to
# reject terminating objects.
#
# Authored after verification; NOT applied to the read-only sys/ tree.

--- a/sys/sys/sysref.h
+++ b/sys/sys/sysref.h
@@ -113,5 +113,6 @@ struct sysref {
 #define SRF_SYSIDUSED	0x0001		/* sysid was used for access */
 #define SRF_ALLOCATED	0x0002		/* sysref_alloc used to allocate */
 #define SRF_PUTAWAY	0x0004		/* in objcache */
+#define SRF_TERMINATING 0x0008		/* termination in progress */

 #endif	/* _KERNEL || _KERNEL_STRUCTURES */
--- a/sys/kern/kern_sysref.c
+++ b/sys/kern/kern_sysref.c
@@ -161,6 +161,7 @@ sysref_alloc(struct sysref_class *srclass)
 	 * function has already allocated a sysid and emplaced the
 	 * structure in the RB tree.
 	 */
 	KKASSERT(sr->refcnt == 0);
 	sr->refcnt = -0x40000000;
+	sr->flags &= ~SRF_TERMINATING;		/* DF-2942 */
@@ -320,6 +321,7 @@ _sysref_put(struct sysref *sr)
 			data = (char *)sr - sr->srclass->offset;
 			sr->srclass->ops.lock(data);
 			if (atomic_cmpset_int(&sr->refcnt, count, -0x40000000)) {
+				sr->flags |= SRF_TERMINATING;	/* DF-2942 */
 				sr->srclass->ops.terminate(data);
 				/* callback unlocks */
 				break;
@@ -343,6 +345,7 @@ _sysref_put(struct sysref *sr)
 			KKASSERT(count == -0x40000000);
 			if (atomic_cmpset_int(&sr->refcnt, count, 0)) {
 				KKASSERT(sr->flags & SRF_ALLOCATED);
+				sr->flags &= ~(SRF_TERMINATING | SRF_SYSIDUSED);
 				sr->flags |= SRF_PUTAWAY;
--- a/sys/kern/kern_sysref.c
+++ b/sys/kern/kern_sysref.c
@@ -277,7 +277,12 @@ void
 sysref_activate(struct sysref *sr)
 {
 	int count;
 
 	for (;;) {
 		count = sr->refcnt;
-		KASSERT(count < 0 && count + 0x40000001 > 0,
+		/*
+		 * DF-2942: -0x40000000 must mean *initializing*, not
+		 * termination-in-progress.  Activating a terminating
+		 * object resurrects it and drives a double terminate.
+		 */
+		KASSERT(count < 0 && count + 0x40000001 > 0 &&
+			(sr->flags & SRF_TERMINATING) == 0,
 			("sysref_activate: bad count %08x", count));