From 4c035d0786d583e0761a97aa87b27f8463a98730 Mon Sep 17 00:00:00 2001 From: visa Date: Tue, 7 Aug 2018 12:38:15 +0000 Subject: [PATCH] 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@ --- sys/kern/kern_event.c | 39 +++++---------------------------------- 1 file 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 @@ -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; -- 2.20.1