Work around a race condition where an interrupt handler can be removed from

an interrupt thread while the interrupt thread is blocked on Giant waiting
to execute the interrupt handler being removed.  The result was that the
intrhand structure would be free'd, and we would call 0xdeadc0de.  The work
around is to check to see if the interrupt thread is idle when removing a
handler.  If not, then we mark the interrupt handler as being dead using
the new IH_DEAD flag and don't remove it from the interrupt threads' list
of handlers.  When the interrupt thread resumes, it will see a dead handler
while traversing the list of handlers and will remove the handler then.

2 files changed, 35 insertions, 3 deletions

diff --git a/sys/kern/kern_intr.c b/sys/kern/kern_intr.c
index 1cd558e..1c37284 100644
--- a/sys/kern/kern_intr.c
+++ b/sys/kern/kern_intr.c
@@ -301,11 +301,29 @@ ithread_remove_handler(void *cookie)
 	    ih->ih_name, ithread->it_name);
 ok:
 #endif
-	TAILQ_REMOVE(&ithread->it_handlers, handler, ih_next);
-	ithread_update(ithread);
+	/*
+	 * If the interrupt thread is already running, then just mark this
+	 * handler as being dead and let the ithread do the actual removal.
+	 */
+	mtx_lock_spin(&sched_lock);
+	if (ithread->it_proc->p_stat != SWAIT) {
+		handler->ih_flags |= IH_DEAD;
+
+		/*
+		 * Ensure that the thread will process the handler list
+		 * again and remove this handler if it has already passed
+		 * it on the list.
+		 */
+		ithread->it_need = 1;
+	} else {
+		TAILQ_REMOVE(&ithread->it_handlers, handler, ih_next);
+		ithread_update(ithread);
+	}
+	mtx_unlock_spin(&sched_lock);
 	mtx_unlock_spin(&ithread_list_lock);
 
-	free(handler, M_ITHREAD);
+	if ((handler->ih_flags & IH_DEAD) == 0)
+		free(handler, M_ITHREAD);
 	return (0);
 }
 
@@ -468,6 +486,7 @@ ithread_loop(void *arg)
 			 * another pass.
 			 */
 			atomic_store_rel_int(&ithd->it_need, 0);
+restart:
 			TAILQ_FOREACH(ih, &ithd->it_handlers, ih_next) {
 				if (ithd->it_flags & IT_SOFT && !ih->ih_need)
 					continue;
@@ -480,6 +499,18 @@ ithread_loop(void *arg)
 
 				if ((ih->ih_flags & IH_MPSAFE) == 0)
 					mtx_lock(&Giant);
+				if ((ih->ih_flags & IH_DEAD) != 0) {
+					mtx_lock_spin(&ithread_list_lock);
+					TAILQ_REMOVE(&ithd->it_handlers, ih,
+					    ih_next);
+					ithread_update(ithd);
+					mtx_unlock_spin(&ithread_list_lock);
+					if (!mtx_owned(&Giant))
+						mtx_lock(&Giant);
+					free(ih, M_ITHREAD);
+					mtx_unlock(&Giant);
+					goto restart;
+				}
 				ih->ih_handler(ih->ih_argument);
 				if ((ih->ih_flags & IH_MPSAFE) == 0)
 					mtx_unlock(&Giant);
diff --git a/sys/sys/interrupt.h b/sys/sys/interrupt.h
index 1bc48d6..9c44d81 100644
--- a/sys/sys/interrupt.h
+++ b/sys/sys/interrupt.h
@@ -50,6 +50,7 @@ struct	intrhand {
 #define	IH_FAST		0x00000001	/* Fast interrupt. */
 #define	IH_EXCLUSIVE	0x00000002	/* Exclusive interrupt. */
 #define	IH_ENTROPY	0x00000004	/* Device is a good entropy source. */
+#define	IH_DEAD		0x00000008	/* Handler should be removed. */
 #define	IH_MPSAFE	0x80000000	/* Handler does not need Giant. */
 
 /*