kqueue: Clear task when closing kqueue

When closing a kqueue, block until any pending wakeup task has finished.
Otherwise, if a pending task progressed slowly, the kqueue could stay
alive longer than the associated file descriptor table, causing
a use-after-free in KQRELE().

This also fixes a failed assertion "p->p_kq->kq_refcnt.r_refs == 1" in
kqpoll_exit().

The use-after-free bug had existed since the introduction of
kqueue_task() (the bug could occur if fdplock() blocked in KQRELE()).
However, the issue became worse when the task was allowed to run without
the kernel lock in sys/kern/kern_event.c r1.187.

Prompted by a report from Mikhail on bugs@.

OK mpi@

Reported-by: syzbot+fca7e4fa773c90886819@syzkaller.appspotmail.com

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 235f1a5..5db97df 100644 (file)
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_event.c,v 1.190 2022/06/20 01:39:44 visa Exp $   */
+/*     $OpenBSD: kern_event.c,v 1.191 2022/06/27 13:35:21 visa Exp $   */
 
 /*-
  * Copyright (c) 1999,2000,2001 Jonathan Lemon <jlemon@FreeBSD.org>
@@ -1559,6 +1559,7 @@ void
 kqueue_terminate(struct proc *p, struct kqueue *kq)
 {
        struct knote *kn;
+       int state;
 
        mtx_enter(&kq->kq_lock);
 
@@ -1571,11 +1572,17 @@ kqueue_terminate(struct proc *p, struct kqueue *kq)
                KASSERT(kn->kn_filter == EVFILT_MARKER);
 
        kq->kq_state |= KQ_DYING;
+       state = kq->kq_state;
        kqueue_wakeup(kq);
        mtx_leave(&kq->kq_lock);
 
+       /*
+        * Any knotes that were attached to this kqueue were deleted
+        * by knote_fdclose() when this kqueue's file descriptor was closed.
+        */
        KASSERT(klist_empty(&kq->kq_sel.si_note));
-       task_del(systqmp, &kq->kq_task);
+       if (state & KQ_TASK)
+               taskq_del_barrier(systqmp, &kq->kq_task);
 }
 
 int
@@ -1601,7 +1608,6 @@ kqueue_task(void *arg)
        mtx_enter(&kqueue_klist_lock);
        KNOTE(&kq->kq_sel.si_note, 0);
        mtx_leave(&kqueue_klist_lock);
-       KQRELE(kq);
 }
 
 void
@@ -1615,9 +1621,8 @@ kqueue_wakeup(struct kqueue *kq)
        }
        if (!klist_empty(&kq->kq_sel.si_note)) {
                /* Defer activation to avoid recursion. */
-               KQREF(kq);
-               if (!task_add(systqmp, &kq->kq_task))
-                       KQRELE(kq);
+               kq->kq_state |= KQ_TASK;
+               task_add(systqmp, &kq->kq_task);
        }
 }
 

diff --git a/sys/sys/eventvar.h b/sys/sys/eventvar.h
index 1e20fb2..e93b149 100644 (file)
--- a/sys/sys/eventvar.h
+++ b/sys/sys/eventvar.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: eventvar.h,v 1.15 2022/06/20 01:39:44 visa Exp $      */
+/*     $OpenBSD: eventvar.h,v 1.16 2022/06/27 13:35:21 visa Exp $      */
 
 /*-
  * Copyright (c) 1999,2000 Jonathan Lemon <jlemon@FreeBSD.org>
@@ -67,6 +67,7 @@ struct kqueue {
        int             kq_state;               /* [q] */
 #define KQ_SLEEP       0x02
 #define KQ_DYING       0x04
+#define KQ_TASK                0x08
 };
 
 #endif /* !_SYS_EVENTVAR_H_ */