⬢ DragonFlyBSD Kernel Audit
DF-5564 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/iicbus/iicbus.h
+++ b/sys/bus/iicbus/iicbus.h
@@ -30,6 +30,8 @@
 #ifndef __IICBUS_H
 #define __IICBUS_H
 
+#include <sys/spinlock.h>	/* struct spinlock sc_lock (DF-5563 fix) */
+
 #define IICBUS_IVAR(d) (struct iicbus_ivar *) device_get_ivars(d)
 #define IICBUS_SOFTC(d) (struct iicbus_softc *) device_get_softc(d)
 
@@ -37,6 +39,9 @@
 {
 	device_t dev;		/* Myself */
 	device_t owner;		/* iicbus owner device structure */
+	int dying;		/* detach in progress: fail new requests */
+	int waiters;		/* threads sleeping inside iicbus_request_bus() */
+	struct spinlock sc_lock;	/* protects owner/dying/waiters */
 	u_char started;		/* address of the 'started' slave
 				 * 0 if no start condition succeeded */
 };
--- a/sys/bus/iicbus/iiconf.c
+++ b/sys/bus/iicbus/iiconf.c
@@ -29,7 +29,9 @@
  */
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/kernel.h>
 #include <sys/malloc.h>
+#include <sys/spinlock2.h>
 #include <sys/module.h>
 #include <sys/bus.h>
 #include <sys/thread2.h>
@@ -60,18 +62,33 @@
 
 	switch (how) {
 	case IIC_WAIT | IIC_INTR:
-		error = tsleep(sc, PCATCH, "iicreq", 0);
+		/*
+		 * Always PCATCH and always bounded (DF-5563, DF-5564): an
+		 * unkillable, unbounded sleep on the softc cannot be
+		 * drained at detach time, and ignores SIGKILL.  The hz
+		 * bound also self-heals any lost wakeup from
+		 * iicbus_release_bus()'s EACCES path.
+		 */
+		error = tsleep(sc, PCATCH, "iicreq", hz);
 		break;
 
 	case IIC_WAIT | IIC_NOINTR:
-		error = tsleep(sc, 0, "iicreq", 0);
+		error = tsleep(sc, PCATCH, "iicreq", hz);
 		break;
 
 	default:
 		return (EWOULDBLOCK);
-		break;
 	}
 
+	/*
+	 * A tsleep timeout (EWOULDBLOCK) is treated as a spurious
+	 * wakeup: the caller re-checks the bus and sleeps again, keeping
+	 * the original "wait indefinitely" contract of IIC_WAIT while
+	 * remaining killable (PCATCH) and bounded for detach draining.
+	 */
+	if (error == EWOULDBLOCK)
+		error = 0;
+
 	return (error);
 }
 
@@ -87,32 +104,75 @@
 {
 	struct iicbus_softc *sc = (struct iicbus_softc *)device_get_softc(bus);
 	int error = 0;
+	int dead = 0;
+	int acquired = 0;
+	int drain;
+
+	spin_lock(&sc->sc_lock);
+	if (sc->dying) {
+		spin_unlock(&sc->sc_lock);
+		return (ENXIO);
+	}
+	sc->waiters++;
+	spin_unlock(&sc->sc_lock);
 
 	/* first, ask the underlying layers if the request is ok */
-	do {
+	for (;;) {
 		error = IICBUS_CALLBACK(device_get_parent(bus),
 						IIC_REQUEST_BUS, (caddr_t)&how);
-		if (error)
-			error = iicbus_poll(sc, how);
-	} while (error == EWOULDBLOCK);
+		if (error == 0) {
+			acquired = 1;
+			break;
+		}
+		/*
+		 * DONTWAIT: one attempt only -- iicbus_poll() returns
+		 * EWOULDBLOCK and we must NOT retry in a tight loop
+		 * (DF-5565).  WAIT: sleep (killable, bounded) and retry.
+		 */
+		error = iicbus_poll(sc, how);
+		if (error != 0)
+			goto out;	/* bridge not acquired: nothing to free */
+		spin_lock(&sc->sc_lock);
+		dead = sc->dying;
+		spin_unlock(&sc->sc_lock);
+		if (dead) {
+			error = ENXIO;
+			goto out;
+		}
+	}
 
-	while (!error) {
-		crit_enter();
-		if (sc->owner && sc->owner != dev) {
-			crit_exit();
+	for (;;) {
+		spin_lock(&sc->sc_lock);
+		dead = sc->dying;
+		if (!dead && sc->owner && sc->owner != dev) {
+			spin_unlock(&sc->sc_lock);
 			error = iicbus_poll(sc, how);
+			if (error != 0)
+				goto out;
 		} else {
-			sc->owner = dev;
-			crit_exit();
-			return (0);
+			if (!dead)
+				sc->owner = dev;
+			spin_unlock(&sc->sc_lock);
+			if (dead)
+				error = ENXIO;
+			goto out;
 		}
-
-		/* free any allocated resource */
-		if (error)
-			IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS,
-					(caddr_t)&how);
 	}
 
+out:
+	spin_lock(&sc->sc_lock);
+	sc->waiters--;
+	drain = (sc->dying && sc->waiters == 0);
+	spin_unlock(&sc->sc_lock);
+	if (drain)
+		wakeup(sc);
+
+	/* free any allocated resource */
+	if (error && acquired && !dead)
+		IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS,
+				(caddr_t)&how);
+	/* (dead: the parent chain is being torn down -- do not poke it) */
+
 	return (error);
 }
 
--- 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/spinlock2.h>
 #include <sys/module.h>
 #include <sys/bus.h>
 
@@ -118,10 +119,13 @@
 static int
 iicbus_attach(device_t dev)
 {
+	struct iicbus_softc *sc = (struct iicbus_softc *)device_get_softc(dev);
 #if SCAN_IICBUS
 	unsigned char addr;
 #endif
 
+	spin_init(&sc->sc_lock, "iicbus");
+
 	iicbus_reset(dev, IIC_FASTEST, 0, NULL);
 
 	/* device probing is meaningless since the bus is supposed to be
@@ -156,6 +160,28 @@
 static int
 iicbus_detach(device_t dev)
 {
+	struct iicbus_softc *sc = IICBUS_SOFTC(dev);
+
+	/*
+	 * Fail and drain iicbus_request_bus() waiters before the softc
+	 * (their sleep channel) and the module text go away (DF-5563).
+	 * The bounded/PCATCH poll makes every waiter observable within
+	 * one second even without a colliding wakeup.
+	 */
+	spin_lock(&sc->sc_lock);
+	sc->dying = 1;
+	spin_unlock(&sc->sc_lock);
+	wakeup(sc);
+	for (;;) {
+		spin_lock(&sc->sc_lock);
+		if (sc->waiters == 0) {
+			spin_unlock(&sc->sc_lock);
+			break;
+		}
+		spin_unlock(&sc->sc_lock);
+		tsleep(sc, PCATCH, "iicdrn", hz);
+	}
+
 	iicbus_reset(dev, IIC_FASTEST, 0, NULL);
 	bus_generic_detach(dev);
 	return (0);