From: visa <visa@openbsd.org>
Date: Mon, 27 Jun 2022 13:35:21 +0000 (+0000)
Subject: kqueue: Clear task when closing kqueue
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=dc399801d5c5a8186948b30d54856b4091647d85;p=openbsd

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 235f1a57005..5db97df588f 100644
--- 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 1e20fb2cf53..e93b1494767 100644
--- 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_ */