DF-2777 / fix.diff
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 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 | --- 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); } |