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);
}
}
