⬢ DragonFlyBSD Kernel Audit
DF-5635 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/iicbus/iicbus.c
+++ b/sys/bus/iicbus/iicbus.c
@@ -33,6 +33,7 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
+#include <sys/malloc.h>
 #include <sys/module.h>
 #include <sys/bus.h>
 
@@ -41,7 +42,8 @@
 
 #include "iicbus_if.h"
 
-#define DEVTOIICBUS(dev) ((struct iicbus_device*)device_get_ivars(dev))
+/* DEVTOIICBUS removed: it referenced struct iicbus_device, which no
+ * longer exists anywhere in the tree (dead macro since the ivar rework) */
 
 devclass_t iicbus_devclass;
 
@@ -54,7 +56,11 @@
 static int iicbus_probe(device_t);
 static int iicbus_attach(device_t);
 static int iicbus_detach(device_t);
-static int iicbus_add_child(device_t dev, int order, const char *name, int unit);
+static device_t iicbus_add_child(device_t bus, device_t parent, int order,
+		   const char *name, int unit);
+static void iicbus_child_detached(device_t bus, device_t child);
+static int iicbus_read_ivar(device_t, device_t, int, uintptr_t *);
+static int iicbus_write_ivar(device_t, device_t, int, uintptr_t);
 
 static device_method_t iicbus_methods[] = {
         /* device interface */
@@ -66,6 +72,9 @@
         DEVMETHOD(bus_add_child,	iicbus_add_child),
 	DEVMETHOD(bus_driver_added,	bus_generic_driver_added),
         DEVMETHOD(bus_print_child,      bus_generic_print_child),
+        DEVMETHOD(bus_child_detached,   iicbus_child_detached),
+        DEVMETHOD(bus_read_ivar,        iicbus_read_ivar),
+        DEVMETHOD(bus_write_ivar,       iicbus_write_ivar),
 
 	DEVMETHOD(iicbus_transfer,	iicbus_transfer_gen),
 
@@ -118,11 +127,14 @@
 static int
 iicbus_attach(device_t dev)
 {
+	int error;
 #if SCAN_IICBUS
 	unsigned char addr;
 #endif
 
-	iicbus_reset(dev, IIC_FASTEST, 0, NULL);
+	error = iicbus_reset(dev, IIC_FASTEST, 0, NULL);
+	if (error)
+		return (error);
 
 	/* device probing is meaningless since the bus is supposed to be
 	 * hot-plug. Moreover, some I2C chips do not appreciate random
@@ -141,9 +153,20 @@
 	kprintf("\n");
 #endif
 
-	device_add_child(dev, "ic", -1);
-	device_add_child(dev, "iic", -1);
-	device_add_child(dev, "iicsmb", -1);
+	/*
+	 * Create the well-known rendezvous children, GUARDED: children
+	 * survive driver detach (devclass_delete_driver() only detaches
+	 * them), so an unguarded re-attach leaks a struct bsd_device
+	 * trio per unload/reload cycle and mints duplicate iicN/iicsmbN
+	 * units (duplicate /dev/iicN nodes on one physical bus).
+	 * Route through our own bus_add_child so ivars are allocated.
+	 */
+	if (device_find_child(dev, "ic", -1) == NULL)
+		BUS_ADD_CHILD(dev, dev, 0, "ic", -1);
+	if (device_find_child(dev, "iic", -1) == NULL)
+		BUS_ADD_CHILD(dev, dev, 0, "iic", -1);
+	if (device_find_child(dev, "iicsmb", -1) == NULL)
+		BUS_ADD_CHILD(dev, dev, 0, "iicsmb", -1);
 #if 0
 	/* attach any known device */
 	device_add_child(dev, "iic", -1);
@@ -156,18 +179,93 @@
 static int
 iicbus_detach(device_t dev)
 {
-	iicbus_reset(dev, IIC_FASTEST, 0, NULL);
-	bus_generic_detach(dev);
-	return (0);
+	int error;
+
+	/*
+	 * DF-5635: a failing child detach must fail OUR detach, else the
+	 * bus is left marked detached + driverless with its softc freed
+	 * (device_set_driver(NULL) kfrees dev->softc) while the child is
+	 * still attached -- the child then derefs
+	 * device_get_softc(bus) == NULL in iicbus_request_bus().
+	 *
+	 * DF-5563 note: the waiter wake/drain this detach also owes
+	 * (dying flag + wakeup(sc) + drain loop) is carried by that
+	 * finding's fix; both patches compose trivially here.
+	 */
+	error = iicbus_reset(dev, IIC_FASTEST, 0, NULL);
+	if (error)
+		return (error);
+	error = bus_generic_detach(dev);
+	return (error);
+}
+
+/*
+ * DF-5632: DragonFly's bus_add_child interface is
+ *	device_t add_child(device_t bus, device_t parent, int order,
+ *			   const char *name, int unit)
+ * (sys/kern/bus_if.m:128-134).  The old 4-arg/int-registered prototype
+ * made every dispatch misread its arguments (order <- child pointer,
+ * name <- order int, unit <- name pointer) and return NULL to callers
+ * expecting the new device.  DF-5634: allocate the ivar the generated
+ * accessors (iicbus.h:53-56) read/write; free it in child_detached.
+ */
+static device_t
+iicbus_add_child(device_t bus, device_t parent, int order,
+		  const char *name, int unit)
+{
+	struct iicbus_ivar *ivar;
+	device_t child;
+
+	child = device_add_child_ordered(parent, order, name, unit);
+	if (child == NULL)
+		return (NULL);
+	ivar = kmalloc(sizeof(*ivar), M_DEVBUF, M_INTWAIT | M_ZERO);
+	if (ivar == NULL) {
+		device_delete_child(bus, child);
+		return (NULL);
+	}
+	device_set_ivars(child, ivar);
+	return (child);
+}
+
+static void
+iicbus_child_detached(device_t bus, device_t child)
+{
+	struct iicbus_ivar *ivar = IICBUS_IVAR(child);
+
+	if (ivar != NULL) {
+		device_set_ivars(child, NULL);
+		kfree(ivar, M_DEVBUF);
+	}
 }
 
 static int
-iicbus_add_child(device_t dev, int order, const char *name, int unit)
+iicbus_read_ivar(device_t dev, device_t child, int index, uintptr_t *result)
 {
+	struct iicbus_ivar *ivar = IICBUS_IVAR(child);
 
-	device_add_child_ordered(dev, order, name, unit);
-	bus_generic_attach(dev);
-	return (0);
+	switch (index) {
+	case IICBUS_IVAR_ADDR:
+		*result = (ivar != NULL) ? ivar->addr : 0;
+		return (0);
+	default:
+		return (ENOENT);
+	}
+}
+
+static int
+iicbus_write_ivar(device_t dev, device_t child, int index, uintptr_t value)
+{
+	struct iicbus_ivar *ivar = IICBUS_IVAR(child);
+
+	switch (index) {
+	case IICBUS_IVAR_ADDR:
+		if (ivar != NULL)
+			ivar->addr = (uint32_t)value;
+		return (0);
+	default:
+		return (ENOENT);
+	}
 }
 
 int