When I added the knote_acquire() call in kqueue_register(), I overlooked
the fact that the knote could be modified by a (soft) interrupt.
Interrupts have to be blocked when changing kn_status. Otherwise the
state could become confused.
The diff below adds splhigh() where kn_status is modified. This also
ensures that the steps of knote_activate() run as an uninterrupted
sequence. As a consequence, knote_enqueue() and knote_dequeue() can rely
on the caller to raise the spl.
OK?
Index: kern/kern_event.c
===================================================================
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.124
diff -u -p -r1.124 kern_event.c
--- kern/kern_event.c 14 Feb 2020 16:50:25 -0000 1.124
+++ kern/kern_event.c 15 Feb 2020 16:52:41 -0000
@@ -312,8 +312,11 @@ filt_proc(struct knote *kn, long hint)
*/
if (event == NOTE_EXIT) {
struct process *pr = kn->kn_ptr.p_process;
+ int s;
+ s = splhigh();
kn->kn_status |= KN_DETACHED;
+ splx(s);
kn->kn_flags |= (EV_EOF | EV_ONESHOT);
kn->kn_data = W_EXITCODE(pr->ps_xexit, pr->ps_xsig);
SLIST_REMOVE(&pr->ps_klist, kn, knote, kn_selnext);
@@ -712,13 +715,16 @@ again:
SLIST_FOREACH(kn, list, kn_link) {
if (kev->filter == kn->kn_filter &&
kev->ident == kn->kn_id) {
- if (knote_acquire(kn) == 0) {
+ s = splhigh();
+ if (!knote_acquire(kn)) {
+ splx(s);
if (fp != NULL) {
FRELE(fp, p);
fp = NULL;
}
goto again;
}
+ splx(s);
break;
}
}
@@ -820,7 +826,9 @@ again:
splx(s);
}
+ s = splhigh();
knote_release(kn);
+ splx(s);
done:
if (fp != NULL)
FRELE(fp, p);
@@ -1157,6 +1165,7 @@ kqueue_expand_list(struct kqueue *kq, in
int
knote_acquire(struct knote *kn)
{
+ splassert(IPL_HIGH);
KASSERT(kn->kn_filter != EVFILT_MARKER);
if (kn->kn_status & KN_PROCESSING) {
@@ -1175,6 +1184,7 @@ knote_acquire(struct knote *kn)
void
knote_release(struct knote *kn)
{
+ splassert(IPL_HIGH);
KASSERT(kn->kn_filter != EVFILT_MARKER);
KASSERT(kn->kn_status & KN_PROCESSING);
@@ -1192,9 +1202,13 @@ knote_release(struct knote *kn)
void
knote_activate(struct knote *kn)
{
+ int s;
+
+ s = splhigh();
kn->kn_status |= KN_ACTIVE;
if ((kn->kn_status & (KN_QUEUED | KN_DISABLED)) == 0)
knote_enqueue(kn);
+ splx(s);
}
/*
@@ -1217,10 +1231,15 @@ void
knote_remove(struct proc *p, struct klist *list)
{
struct knote *kn;
+ int s;
while ((kn = SLIST_FIRST(list)) != NULL) {
- if (!knote_acquire(kn))
+ s = splhigh();
+ if (!knote_acquire(kn)) {
+ splx(s);
continue;
+ }
+ splx(s);
kn->kn_fop->f_detach(kn);
knote_drop(kn, p);
}
@@ -1297,6 +1316,7 @@ knote_drop(struct knote *kn, struct proc
{
struct kqueue *kq = kn->kn_kq;
struct klist *list;
+ int s;
KASSERT(kn->kn_filter != EVFILT_MARKER);
@@ -1306,12 +1326,14 @@ knote_drop(struct knote *kn, struct proc
list = &kq->kq_knhash[KN_HASH(kn->kn_id, kq->kq_knhashmask)];
SLIST_REMOVE(list, kn, knote, kn_link);
+ s = splhigh();
if (kn->kn_status & KN_QUEUED)
knote_dequeue(kn);
if (kn->kn_status & KN_WAITING) {
kn->kn_status &= ~KN_WAITING;
wakeup(kn);
}
+ splx(s);
if (kn->kn_fop->f_isfd)
FRELE(kn->kn_fp, p);
pool_put(&knote_pool, kn);
@@ -1322,8 +1344,8 @@ void
knote_enqueue(struct knote *kn)
{
struct kqueue *kq = kn->kn_kq;
- int s = splhigh();
+ splassert(IPL_HIGH);
KASSERT(kn->kn_filter != EVFILT_MARKER);
KASSERT((kn->kn_status & KN_QUEUED) == 0);
@@ -1332,7 +1354,6 @@ knote_enqueue(struct knote *kn)
kn->kn_status |= KN_QUEUED;
kq->kq_count++;
kqueue_check(kq);
- splx(s);
kqueue_wakeup(kq);
}
@@ -1340,8 +1361,8 @@ void
knote_dequeue(struct knote *kn)
{
struct kqueue *kq = kn->kn_kq;
- int s = splhigh();
+ splassert(IPL_HIGH);
KASSERT(kn->kn_filter != EVFILT_MARKER);
KASSERT(kn->kn_status & KN_QUEUED);
@@ -1350,13 +1371,13 @@ knote_dequeue(struct knote *kn)
kn->kn_status &= ~KN_QUEUED;
kq->kq_count--;
kqueue_check(kq);
- splx(s);
}
void
klist_invalidate(struct klist *list)
{
struct knote *kn;
+ int s;
/*
* NET_LOCK() must not be held because it can block another thread
@@ -1364,12 +1385,16 @@ klist_invalidate(struct klist *list)
*/
NET_ASSERT_UNLOCKED();
+ s = splhigh();
while ((kn = SLIST_FIRST(list)) != NULL) {
- if (knote_acquire(kn) == 0)
+ if (!knote_acquire(kn))
continue;
+ splx(s);
kn->kn_fop->f_detach(kn);
kn->kn_fop = &dead_filtops;
knote_activate(kn);
+ s = splhigh();
knote_release(kn);
}
+ splx(s);
}
