⬢ DragonFlyBSD Kernel Audit
DF-2776 / fix.diff
← back to finding ↓ download raw
--- a/sys/kern/kern_intr.c
+++ b/sys/kern/kern_intr.c
@@ -137,10 +137,12 @@
 
 static struct systimer emergency_intr_timer[MAXCPU];
 static struct thread *emergency_intr_thread[MAXCPU];
+static int emergency_intr_gate[MAXCPU];
 
 #define ISTATE_NOTHREAD		0
 #define ISTATE_NORMAL		1
 #define ISTATE_LIVELOCKED	2
+#define ISTATE_CREATING		3
 
 static int livelock_limit = 40000;
 static int livelock_limit_hi = 120000;
@@ -305,7 +307,11 @@
      *
      * objcache may not be operational yet, use kmalloc().
      */
-    if (emergency_intr_thread[cpuid] == NULL) {
+    if (emergency_intr_thread[cpuid] == NULL &&
+	atomic_cmpset_int(&emergency_intr_gate[cpuid], 0, 1)) {
+	/* DF-2777: once-only creation -- two concurrent first
+	 * registrations on a cpu must not both create and arm.  The
+	 * loser just skips this block and registers normally. */
 	emergency_intr_thread[cpuid] = kmalloc(sizeof(struct thread), M_DEVBUF,
 					       M_INTWAIT | M_ZERO);
 	lwkt_create(ithread_emergency, NULL, NULL,
@@ -321,19 +327,30 @@
     /*
      * Create an interrupt thread if necessary, leave it in an unscheduled
      * state.
+     *
+     * DF-2777: the creation window spans blocking allocations (M_INTWAIT
+     * kmalloc, lwkt_create).  Claim the transition with a cmpset, build
+     * the thread into a local, and publish i_thread BEFORE i_state so no
+     * concurrent register_int() -- and no interrupt arriving through
+     * sched_ithd_intern() -- can observe ISTATE_NORMAL with a NULL
+     * i_thread.
      */
-    if (info->i_state == ISTATE_NOTHREAD) {
-	info->i_state = ISTATE_NORMAL;
-	info->i_thread = kmalloc(sizeof(struct thread), M_DEVBUF,
-				 M_INTWAIT | M_ZERO);
+    if (info->i_state == ISTATE_NOTHREAD &&
+	atomic_cmpset_int(&info->i_state, ISTATE_NOTHREAD, ISTATE_CREATING)) {
+	struct thread *ithd;
+
+	ithd = kmalloc(sizeof(struct thread), M_DEVBUF, M_INTWAIT | M_ZERO);
 	lwkt_create(ithread_handler, (void *)(intptr_t)intr, NULL,
-		    info->i_thread, TDF_NOSTART | TDF_INTTHREAD, cpuid,
+		    ithd, TDF_NOSTART | TDF_INTTHREAD, cpuid,
 		    "ithread%d %d", intr, cpuid);
 	if (intr >= FIRST_SOFTINT)
-	    lwkt_setpri(info->i_thread, TDPRI_SOFT_NORM);
+	    lwkt_setpri(ithd, TDPRI_SOFT_NORM);
 	else
-	    lwkt_setpri(info->i_thread, TDPRI_INT_MED);
-	info->i_thread->td_preemptable = lwkt_preempt;
+	    lwkt_setpri(ithd, TDPRI_INT_MED);
+	ithd->td_preemptable = lwkt_preempt;
+	info->i_thread = ithd;			/* publish thread first */
+	cpu_sfence();
+	info->i_state = ISTATE_NORMAL;		/* then the state */
     }
 
     /*
@@ -346,8 +363,11 @@
 	kprintf("interrupt uses mplock: %s\n", name);
     }
     if (intr_flags & INTR_CLOCK) {
-	atomic_set_int(&info->i_thread->td_flags, TDF_CLKTHREAD);
 	++info->i_fast;
+	/* DF-2777: a concurrent registration may still be creating the
+	 * interrupt thread; the creator sets the flag itself. */
+	if (info->i_thread != NULL)
+	    atomic_set_int(&info->i_thread->td_flags, TDF_CLKTHREAD);
     } else {
 	++info->i_slow;
     }
@@ -486,8 +506,10 @@
 	kfree(rec->name, M_DEVBUF);
 	kfree(rec, M_DEVBUF);
     } else {
-	kprintf("warning: unregister_int: int %d handler for %s not found\n",
-		intr, ((intrec_t)id)->name);
+	/* DF-2776: `id' is caller-supplied and may be stale/freed;
+	 * never dereference it here.  (The intr load at entry is
+	 * unavoidable: the cookie is the only way to find the list.) */
+	kprintf("warning: unregister_int: int handler %p not found\n", id);
     }
 }
 
@@ -585,6 +607,9 @@
     if (info->i_state != ISTATE_NOTHREAD) {
 	if (info->i_reclist == NULL) {
 	    report_stray_interrupt(info, "sched_ithd");
+	} else if (info->i_thread == NULL) {
+	    /* DF-2777: interrupt thread creation in progress */
+	    report_stray_interrupt(info, "sched_ithd");
 	} else {
 	    if (info->i_thread->td_gd == mycpu) {
 		if (info->i_running == 0) {
@@ -1090,16 +1115,32 @@
 static int
 sysctl_intrnames(SYSCTL_HANDLER_ARGS)
 {
-    struct intr_info *info;
-    intrec_t rec;
     int error = 0;
-    int len;
     int intr, cpuid;
-    char buf[64];
+    size_t tot;
+    char *out, *p;
 
+    /*
+     * DF-2775: i_reclist chains are unlinked and freed by
+     * unregister_int() without any lock the sysctl reader holds, so
+     * walking them while copying out to userland can dereference a
+     * freed intrec (UAF read -> heap disclosure / panic).  Snapshot
+     * the rendered names under the per-cpu migrate+crit protocol used
+     * for the unlink, and copy the snapshot out afterwards.
+     */
+    tot = (size_t)ncpus * MAX_INTS * 64;
+    out = kmalloc(tot, M_INTRMNG, M_INTWAIT | M_ZERO);
+    p = out;
     for (cpuid = 0; cpuid < ncpus; ++cpuid) {
-	for (intr = 0; error == 0 && intr < MAX_INTS; ++intr) {
-	    info = &intr_block->ary[cpuid][intr];
+	int orig;
+
+	int_moveto_destcpu(&orig, cpuid);
+	crit_enter();
+	for (intr = 0; intr < MAX_INTS; ++intr) {
+	    struct intr_info *info = &intr_block->ary[cpuid][intr];
+	    intrec_t rec;
+	    char buf[64];
+	    int len;
 
 	    len = 0;
 	    buf[0] = 0;
@@ -1112,9 +1153,14 @@
 		ksnprintf(buf, sizeof(buf), "irq%d", intr);
 		len = strlen(buf);
 	    }
-	    error = SYSCTL_OUT(req, buf, len + 1);
+	    bcopy(buf, p, len + 1);
+	    p += len + 1;
 	}
+	crit_exit();
+	int_moveto_origcpu(orig, cpuid);
     }
+    error = SYSCTL_OUT(req, out, p - out);
+    kfree(out, M_INTRMNG);
     return (error);
 }