Module Name: src
Committed By: christos
Date: Sun Jan 31 04:40:01 UTC 2016
Modified Files:
src/sys/kern: kern_event.c
src/sys/sys: event.h
Log Message:
PR/50730: Benny Siegert: Go kqueue test panics kernel.
- use a marker knote from the stack instead of allocating and freeing on
each scan.
- add more KASSERTS
- introduce a KN_BUSY bit that indicates that the knote is currently being
scanned, so that knote_detach does not end up deleting it when the file
descriptor gets closed and we don't end up using/trashing free memory from
the scan.
To generate a diff of this commit:
cvs rdiff -u -r1.84 -r1.85 src/sys/kern/kern_event.c
cvs rdiff -u -r1.25 -r1.26 src/sys/sys/event.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_event.c
diff -u src/sys/kern/kern_event.c:1.84 src/sys/kern/kern_event.c:1.85
--- src/sys/kern/kern_event.c:1.84 Tue Dec 8 09:52:06 2015
+++ src/sys/kern/kern_event.c Sat Jan 30 23:40:01 2016
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_event.c,v 1.84 2015/12/08 14:52:06 christos Exp $ */
+/* $NetBSD: kern_event.c,v 1.85 2016/01/31 04:40:01 christos Exp $ */
/*-
* Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_event.c,v 1.84 2015/12/08 14:52:06 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_event.c,v 1.85 2016/01/31 04:40:01 christos Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -986,6 +986,7 @@ kqueue_register(struct kqueue *kq, struc
kev->data = 0;
kn->kn_kevent = *kev;
+ KASSERT(kn->kn_fop != NULL);
/*
* apply reference count to knote structure, and
* do not release it at the end of this routine.
@@ -1043,6 +1044,7 @@ kqueue_register(struct kqueue *kq, struc
* support events, and the attach routine is
* broken and does not return an error.
*/
+ KASSERT(kn->kn_fop != NULL);
KASSERT(kn->kn_fop->f_event != NULL);
KERNEL_LOCK(1, NULL); /* XXXSMP */
rv = (*kn->kn_fop->f_event)(kn, 0);
@@ -1150,7 +1152,7 @@ kqueue_scan(file_t *fp, size_t maxevents
struct kqueue *kq;
struct kevent *kevp;
struct timespec ats, sleepts;
- struct knote *kn, *marker;
+ struct knote *kn, *marker, morker;
size_t count, nkev, nevents;
int timeout, error, rv;
filedesc_t *fdp;
@@ -1178,7 +1180,8 @@ kqueue_scan(file_t *fp, size_t maxevents
timeout = 0;
}
- marker = kmem_zalloc(sizeof(*marker), KM_SLEEP);
+ memset(&morker, 0, sizeof(morker));
+ marker = &morker;
marker->kn_status = KN_MARKER;
mutex_spin_enter(&kq->kq_lock);
retry:
@@ -1219,30 +1222,35 @@ kqueue_scan(file_t *fp, size_t maxevents
kn = TAILQ_NEXT(kn, kn_tqe);
}
kq_check(kq);
- TAILQ_REMOVE(&kq->kq_head, kn, kn_tqe);
kq->kq_count--;
+ TAILQ_REMOVE(&kq->kq_head, kn, kn_tqe);
kn->kn_status &= ~KN_QUEUED;
+ kn->kn_status |= KN_BUSY;
kq_check(kq);
if (kn->kn_status & KN_DISABLED) {
+ kn->kn_status &= ~KN_BUSY;
/* don't want disabled events */
continue;
}
if ((kn->kn_flags & EV_ONESHOT) == 0) {
mutex_spin_exit(&kq->kq_lock);
+ KASSERT(kn->kn_fop != NULL);
KASSERT(kn->kn_fop->f_event != NULL);
KERNEL_LOCK(1, NULL); /* XXXSMP */
rv = (*kn->kn_fop->f_event)(kn, 0);
KERNEL_UNLOCK_ONE(NULL); /* XXXSMP */
mutex_spin_enter(&kq->kq_lock);
/* Re-poll if note was re-enqueued. */
- if ((kn->kn_status & KN_QUEUED) != 0)
+ if ((kn->kn_status & KN_QUEUED) != 0) {
+ kn->kn_status &= ~KN_BUSY;
continue;
+ }
if (rv == 0) {
/*
* non-ONESHOT event that hasn't
* triggered again, so de-queue.
*/
- kn->kn_status &= ~KN_ACTIVE;
+ kn->kn_status &= ~(KN_ACTIVE|KN_BUSY);
continue;
}
}
@@ -1253,22 +1261,24 @@ kqueue_scan(file_t *fp, size_t maxevents
/* delete ONESHOT events after retrieval */
mutex_spin_exit(&kq->kq_lock);
mutex_enter(&fdp->fd_lock);
+ kn->kn_status &= ~KN_BUSY;
knote_detach(kn, fdp, true);
mutex_spin_enter(&kq->kq_lock);
} else if (kn->kn_flags & EV_CLEAR) {
/* clear state after retrieval */
kn->kn_data = 0;
kn->kn_fflags = 0;
- kn->kn_status &= ~(KN_QUEUED|KN_ACTIVE);
+ kn->kn_status &= ~(KN_QUEUED|KN_ACTIVE|KN_BUSY);
} else if (kn->kn_flags & EV_DISPATCH) {
kn->kn_status |= KN_DISABLED;
- kn->kn_status &= ~(KN_QUEUED|KN_ACTIVE);
+ kn->kn_status &= ~(KN_QUEUED|KN_ACTIVE|KN_BUSY);
} else {
/* add event back on list */
kq_check(kq);
+ kn->kn_status |= KN_QUEUED;
+ kn->kn_status &= ~KN_BUSY;
TAILQ_INSERT_TAIL(&kq->kq_head, kn, kn_tqe);
kq->kq_count++;
- kn->kn_status |= KN_QUEUED;
kq_check(kq);
}
if (nkev == kevcnt) {
@@ -1292,8 +1302,6 @@ kqueue_scan(file_t *fp, size_t maxevents
}
done:
mutex_spin_exit(&kq->kq_lock);
- if (marker != NULL)
- kmem_free(marker, sizeof(*marker));
if (nkev != 0) {
/* copyout remaining events */
error = (*keops->keo_put_events)(keops->keo_private,
@@ -1514,6 +1522,7 @@ knote(struct klist *list, long hint)
struct knote *kn, *tmpkn;
SLIST_FOREACH_SAFE(kn, list, kn_selnext, tmpkn) {
+ KASSERT(kn->kn_fop != NULL);
KASSERT(kn->kn_fop->f_event != NULL);
if ((*kn->kn_fop->f_event)(kn, hint))
knote_activate(kn);
@@ -1555,8 +1564,10 @@ knote_detach(struct knote *kn, filedesc_
KASSERT((kn->kn_status & KN_MARKER) == 0);
KASSERT(mutex_owned(&fdp->fd_lock));
+ KASSERT(kn->kn_fop != NULL);
/* Remove from monitored object. */
if (dofop) {
+ KASSERT(kn->kn_fop->f_detach != NULL);
KERNEL_LOCK(1, NULL); /* XXXSMP */
(*kn->kn_fop->f_detach)(kn);
KERNEL_UNLOCK_ONE(NULL); /* XXXSMP */
@@ -1571,14 +1582,17 @@ knote_detach(struct knote *kn, filedesc_
SLIST_REMOVE(list, kn, knote, kn_link);
/* Remove from kqueue. */
- /* XXXAD should verify not in use by kqueue_scan. */
+again:
mutex_spin_enter(&kq->kq_lock);
if ((kn->kn_status & KN_QUEUED) != 0) {
kq_check(kq);
+ kq->kq_count--;
TAILQ_REMOVE(&kq->kq_head, kn, kn_tqe);
kn->kn_status &= ~KN_QUEUED;
- kq->kq_count--;
kq_check(kq);
+ } else if (kn->kn_status & KN_BUSY) {
+ mutex_spin_exit(&kq->kq_lock);
+ goto again;
}
mutex_spin_exit(&kq->kq_lock);
@@ -1607,8 +1621,8 @@ knote_enqueue(struct knote *kn)
}
if ((kn->kn_status & (KN_ACTIVE | KN_QUEUED)) == KN_ACTIVE) {
kq_check(kq);
- TAILQ_INSERT_TAIL(&kq->kq_head, kn, kn_tqe);
kn->kn_status |= KN_QUEUED;
+ TAILQ_INSERT_TAIL(&kq->kq_head, kn, kn_tqe);
kq->kq_count++;
kq_check(kq);
cv_broadcast(&kq->kq_cv);
@@ -1632,8 +1646,8 @@ knote_activate(struct knote *kn)
kn->kn_status |= KN_ACTIVE;
if ((kn->kn_status & (KN_QUEUED | KN_DISABLED)) == 0) {
kq_check(kq);
- TAILQ_INSERT_TAIL(&kq->kq_head, kn, kn_tqe);
kn->kn_status |= KN_QUEUED;
+ TAILQ_INSERT_TAIL(&kq->kq_head, kn, kn_tqe);
kq->kq_count++;
kq_check(kq);
cv_broadcast(&kq->kq_cv);
Index: src/sys/sys/event.h
diff -u src/sys/sys/event.h:1.25 src/sys/sys/event.h:1.26
--- src/sys/sys/event.h:1.25 Tue Dec 8 09:52:06 2015
+++ src/sys/sys/event.h Sat Jan 30 23:40:01 2016
@@ -1,4 +1,4 @@
-/* $NetBSD: event.h,v 1.25 2015/12/08 14:52:06 christos Exp $ */
+/* $NetBSD: event.h,v 1.26 2016/01/31 04:40:01 christos Exp $ */
/*-
* Copyright (c) 1999,2000,2001 Jonathan Lemon <jle...@freebsd.org>
@@ -196,6 +196,7 @@ struct knote {
#define KN_DISABLED 0x04U /* event is disabled */
#define KN_DETACHED 0x08U /* knote is detached */
#define KN_MARKER 0x10U /* is a marker */
+#define KN_BUSY 0x20U /* is being scanned */
#define kn_id kn_kevent.ident
#define kn_filter kn_kevent.filter
