⬢ DragonFlyBSD Kernel Audit
DF-5521 / 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
--- a/sys/bus/iicbus/iiconf.c
+++ b/sys/bus/iicbus/iiconf.c
@@ -148,6 +148,22 @@
 }
 
 /*
+ * iicbus_bus_owned()
+ *
+ * Non-zero when 'dev' currently owns 'bus', i.e. an iicbus transaction
+ * routed through 'dev' is in flight.  Bus drivers layered on top of
+ * iicbus (e.g. the iicsmb bridge) must refuse to detach while they own
+ * the bus, otherwise a caller sleeping inside iicbus_read()/write()
+ * resumes in freed module text and iicbus_softc->owner dangles.
+ */
+int
+iicbus_bus_owned(device_t bus, device_t dev)
+{
+	struct iicbus_softc *sc = (struct iicbus_softc *)device_get_softc(bus);
+
+	return (sc->owner == dev);
+}
+/*
  * iicbus_started()
  *
  * Test if the iicbus is started by the controller

--- a/sys/bus/iicbus/iiconf.h
+++ b/sys/bus/iicbus/iiconf.h
@@ -97,6 +97,7 @@
 extern int iicbus_release_bus(device_t, device_t);
 
 extern void iicbus_intr(device_t, int, char *);
+extern int	iicbus_bus_owned(device_t, device_t);
 
 extern int iicbus_null_repeated_start(device_t, u_char);
 extern int iicbus_null_callback(device_t, int, caddr_t);

--- a/sys/bus/iicbus/iicsmb.c
+++ b/sys/bus/iicbus/iicsmb.c
@@ -165,9 +165,23 @@
 iicsmb_detach(device_t dev)
 {
 	struct iicsmb_softc *sc = (struct iicsmb_softc *)device_get_softc(dev);
-
-	bus_generic_detach(dev);
-	if (sc->smbus) {
+	device_t parent = device_get_parent(dev);
+	int error;
+
+	/*
+	 * Refuse to detach (kldunload) while an SMBus client transaction
+	 * routed through this bridge is in flight: we are the iicbus owner
+	 * for the whole transaction, and the caller can be asleep inside
+	 * iicbus_read()/iicbus_write() below this bridge.  Completing the
+	 * unload frees the module text the sleeper returns into (UAF) and
+	 * leaves iicbus_softc->owner dangling for later iicbus_intr()s.
+	 */
+	if (parent != NULL && iicbus_bus_owned(parent, dev))
+		return (EBUSY);
+
+	if ((error = bus_generic_detach(dev)) != 0)
+		return (error);
+	if (sc->smbus != NULL) {
 		device_delete_child(dev, sc->smbus);
 	}
 

--- a/sys/bus/smbus/smbus.c
+++ b/sys/bus/smbus/smbus.c
@@ -106,6 +106,17 @@
 smbus_detach(device_t dev)
 {
 	int error;
+	struct smbus_softc *sc = (struct smbus_softc *)device_get_softc(dev);
+
+	/*
+	 * Refuse to detach while a client holds the bus: force-detaching here
+	 * (kldunload of a driver module registered on this bus, e.g. iicsmb's
+	 * smbus/iicsmb registration at iicsmb.c:510) frees the softc under a
+	 * caller that is asleep inside an SMBus transaction routed through
+	 * this bus, and detaches its device nodes.
+	 */
+	if (sc->owner != NULL)
+		return (EBUSY);
 
 	error = bus_generic_detach(dev);
 	if (error)