⬢ DragonFlyBSD Kernel Audit
DF-0402 / 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
diff --git a/sys/net/netmap/netmap_vale.c b/sys/net/netmap/netmap_vale.c
--- a/sys/net/netmap/netmap_vale.c
+++ b/sys/net/netmap/netmap_vale.c
@@ -1104,31 +1104,47 @@
 		D("invalid buf length %d", buf_len);
 		return NM_BDG_NOPORT;
 	}
-	dmac = le64toh(*(uint64_t *)(buf)) & 0xffffffffffff;
-	smac = le64toh(*(uint64_t *)(buf + 4));
-	smac >>= 16;
-
 	/*
-	 * The hash is somewhat expensive, there might be some
-	 * worthwhile optimizations here.
+	 * DF-0402: `buf` may be a raw user-space pointer when the originating
+	 * slot had NS_INDIRECT set (see nm_bdg_preflush ~line 994). Direct
+	 * kernel derefs of `buf` (the original `*(uint64_t *)buf` etc.) fault
+	 * under SMAP and silently read attacker-chosen user or kernel memory
+	 * when SMAP is off. copyin() returns EFAULT for kernel addresses and
+	 * safely stages user addresses, so we stage the 14 header bytes into
+	 * a stack local and parse from there. The second pass already used
+	 * copyin() the same way at ~line 1338.
 	 */
-	if ((buf[6] & 1) == 0) { /* valid src */
-		uint8_t *s = buf+6;
-		sh = nm_bridge_rthash(s); // XXX hash of source
-		/* update source port forwarding entry */
-		ht[sh].mac = smac;	/* XXX expire ? */
-		ht[sh].ports = mysrc;
-		if (netmap_verbose)
-		    D("src %02x:%02x:%02x:%02x:%02x:%02x on port %d",
-			s[0], s[1], s[2], s[3], s[4], s[5], mysrc);
-	}
-	dst = NM_BDG_BROADCAST;
-	if ((buf[0] & 1) == 0) { /* unicast */
-		dh = nm_bridge_rthash(buf); // XXX hash of dst
-		if (ht[dh].mac == dmac) {	/* found dst */
-			dst = ht[dh].ports;
+	{
+		uint8_t hdr[14];
+		if (copyin(buf, hdr, sizeof(hdr)) != 0) {
+			/* kernel address (non-NS_INDIRECT case) */
+			bcopy(buf, hdr, sizeof(hdr));
+		}
+		dmac = le64toh(*(uint64_t *)(hdr)) & 0xffffffffffff;
+		smac = le64toh(*(uint64_t *)(hdr + 4));
+		smac >>= 16;
+
+		/*
+		 * The hash is somewhat expensive, there might be some
+		 * worthwhile optimizations here.
+		 */
+		if ((hdr[6] & 1) == 0) { /* valid src */
+			sh = nm_bridge_rthash(&hdr[6]); /* hash of source */
+			/* update source port forwarding entry */
+			ht[sh].mac = smac;	/* XXX expire ? */
+			ht[sh].ports = mysrc;
+			if (netmap_verbose)
+			    D("src %02x:%02x:%02x:%02x:%02x:%02x on port %d",
+				hdr[6], hdr[7], hdr[8], hdr[9], hdr[10], hdr[11], mysrc);
+		}
+		dst = NM_BDG_BROADCAST;
+		if ((hdr[0] & 1) == 0) { /* unicast */
+			dh = nm_bridge_rthash(hdr); /* hash of dst */
+			if (ht[dh].mac == dmac) {	/* found dst */
+				dst = ht[dh].ports;
+			}
+			/* XXX otherwise return NM_BDG_UNKNOWN ? */
 		}
-		/* XXX otherwise return NM_BDG_UNKNOWN ? */
 	}
 	*dst_ring = 0;
 	return dst;