On Fri, Jan 03, 2020 at 11:37:22PM -0800, Greg Steuck wrote: > While playing with chromium u2f support[1] I managed to induce kernel > crashes in filt_uhidrdetach. It takes a few attempts of plugging/unplugging > the fido key while trying to authenticate at demo.yubico.com/playground. > Eventually the kernel panics with this stack trace (retyped from [2]): > > filt_uhidrdetach+0x33: movq 0x8(%rcx), rcx > kqueue_close > drop > closef > fdfree > exit1 > single_thread_check > userret > intr_user_exit
The kernel is not very good at revoking knotes when a device is detached. There is already some code for doing a cleanish detachment, but it is not used everywhere. Does the following diff help with this problem with uhid(4)? Index: dev/usb/uhid.c =================================================================== RCS file: src/sys/dev/usb/uhid.c,v retrieving revision 1.75 diff -u -p -r1.75 uhid.c --- dev/usb/uhid.c 31 Dec 2019 13:48:31 -0000 1.75 +++ dev/usb/uhid.c 6 Jan 2020 17:33:17 -0000 @@ -168,6 +168,10 @@ uhid_detach(struct device *self, int fla mn = self->dv_unit; vdevgone(maj, mn, mn, VCHR); + s = splusb(); + klist_invalidate(&sc->sc_rsel.si_note); + splx(s); + return (0); } Index: kern/kern_event.c =================================================================== RCS file: src/sys/kern/kern_event.c,v retrieving revision 1.117 diff -u -p -r1.117 kern_event.c --- kern/kern_event.c 6 Jan 2020 10:28:32 -0000 1.117 +++ kern/kern_event.c 6 Jan 2020 17:33:17 -0000 @@ -464,6 +464,27 @@ seltrue_kqfilter(dev_t dev, struct knote return (0); } +static int +filt_dead(struct knote *kn, long hint) +{ + kn->kn_flags |= (EV_EOF | EV_ONESHOT); + kn->kn_data = 0; + return (1); +} + +static void +filt_deaddetach(struct knote *kn) +{ + /* Nothing to do */ +} + +static const struct filterops dead_filtops = { + .f_isfd = 1, + .f_attach = NULL, + .f_detach = filt_deaddetach, + .f_event = filt_dead, +}; + int sys_kqueue(struct proc *p, void *v, register_t *retval) { @@ -1304,8 +1325,18 @@ klist_invalidate(struct klist *list) { struct knote *kn; - SLIST_FOREACH(kn, list, kn_selnext) { - kn->kn_status |= KN_DETACHED; - kn->kn_flags |= EV_EOF | EV_ONESHOT; + /* + * NET_LOCK() must not be held because it can block another thread + * in f_event with a knote acquired. + */ + NET_ASSERT_UNLOCKED(); + + while ((kn = SLIST_FIRST(list)) != NULL) { + if (knote_acquire(kn) == 0) + continue; + kn->kn_fop->f_detach(kn); + kn->kn_fop = &dead_filtops; + knote_activate(kn); + knote_release(kn); } }