Fix dangling knote references.

kqueue_close() does not take into account that the kqueue instance may
have queued knotes. This can cause a use-after-free if new knotes are
enqueued on the kqueue as a result of file closing.

Correct the error by dequeueing each knote before freeing it.
Since r1.93 of kern_event.c, each kqueue instance has its knotes
in nonshared lists kq_knhash and kq_knlist, so kqueue_close() does
not have to skip other kqueues' knotes any longer. The code can be
simplified by using knote_remove() for clearing the knote lists.
The function uses knote_drop() which takes care of knote dequeueing.

Found and initial analysis by anton@
OK anton@, mpi@

1 files changed, 5 insertions, 34 deletions

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 2ea8dbf45f8..8ee92e8d60d 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_event.c,v 1.94 2018/06/18 09:15:05 mpi Exp $	*/
+/*	$OpenBSD: kern_event.c,v 1.95 2018/08/07 12:38:15 visa Exp $	*/
 
 /*-
  * Copyright (c) 1999,2000,2001 Jonathan Lemon <jlemon@FreeBSD.org>
@@ -905,42 +905,13 @@ int
 kqueue_close(struct file *fp, struct proc *p)
 {
 	struct kqueue *kq = fp->f_data;
-	struct knote **knp, *kn, *kn0;
 	int i;
 
-	for (i = 0; i < kq->kq_knlistsize; i++) {
-		knp = &SLIST_FIRST(&kq->kq_knlist[i]);
-		kn = *knp;
-		while (kn != NULL) {
-			kn0 = SLIST_NEXT(kn, kn_link);
-			if (kq == kn->kn_kq) {
-				kn->kn_fop->f_detach(kn);
-				FRELE(kn->kn_fp, p);
-				knote_free(kn);
-				*knp = kn0;
-			} else {
-				knp = &SLIST_NEXT(kn, kn_link);
-			}
-			kn = kn0;
-		}
-	}
+	for (i = 0; i < kq->kq_knlistsize; i++)
+		knote_remove(p, &kq->kq_knlist[i]);
 	if (kq->kq_knhashmask != 0) {
-		for (i = 0; i < kq->kq_knhashmask + 1; i++) {
-			knp = &SLIST_FIRST(&kq->kq_knhash[i]);
-			kn = *knp;
-			while (kn != NULL) {
-				kn0 = SLIST_NEXT(kn, kn_link);
-				if (kq == kn->kn_kq) {
-					kn->kn_fop->f_detach(kn);
-		/* XXX non-fd release of kn->kn_ptr */
-					knote_free(kn);
-					*knp = kn0;
-				} else {
-					knp = &SLIST_NEXT(kn, kn_link);
-				}
-				kn = kn0;
-			}
-		}
+		for (i = 0; i < kq->kq_knhashmask + 1; i++)
+			knote_remove(p, &kq->kq_knhash[i]);
 	}
 	fp->f_data = NULL;