On Sat, May 07, 2022 at 07:47:12PM +1000, Joel Sing wrote:
> After upgrading an OpenBSD arm64 (pine64+ board) Go builder to 7.1, the
> following has been triggered twice (once every couple of days):
>
> panic: kernel diagnostic assertion "kn->kn_kq == kq" failed: file
> "/usr/src/sys/kern/kern_event.c", line 1811
> Stopped at panic+0x160: cmp w21, #0x0
> TID PID UID PRFLAGS PFLAGS CPU COMMAND
> 152330 84924 1001 0x3 0x4000000 3 http.test
> *427942 85055 1001 0x3 0x4000000 1 net.test
> 460437 53928 0 0x14000 0x200 2 tztq
> 197440 95098 0 0x14000 0x200 0 softnet
> db_enter() at panic+0x15c
> panic() at __assert+0x24
> panic() at knote_remove+0x2c4
> knote_remove() at knote_fdclose+0x88
> knote_fdclose() at fdrelease+0x94
> fdrelease() at svc_handler+0x2d4
> svc_handler() at do_el0_sync+0xa0
>
> This was stable with 7.0, hence seems to be a regression between 7.0 and 7.1
> (presumably unlocking related).
There is a race condition in knote_remove(). The function does not
take into account that another thread might resize kq_knlist[] while
knote_remove() clears a knote.
I think the flaw has been present since r1.109 of kern_event.c when I
made the subsystem allow sleeping inside event filters. I guess the
recent unlockings now enable such timings that trigger the error.
Joel, could you test the following patch?
For a speedier trial, the bug's incidence rate can be increased by
lowering the value of KQEXTENT in sys/sys/eventvar.h. Even a value
as low as 1 should work (though one can ask if this will actually
test the original issue).
Index: sys/kern/kern_event.c
===================================================================
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.187
diff -u -p -r1.187 kern_event.c
--- sys/kern/kern_event.c 6 May 2022 13:12:16 -0000 1.187
+++ sys/kern/kern_event.c 8 May 2022 04:31:21 -0000
@@ -121,8 +121,8 @@ void knote_dequeue(struct knote *kn);
int knote_acquire(struct knote *kn, struct klist *, int);
void knote_release(struct knote *kn);
void knote_activate(struct knote *kn);
-void knote_remove(struct proc *p, struct kqueue *kq, struct knlist *list,
- int purge);
+void knote_remove(struct proc *p, struct kqueue *kq, struct knlist **plist,
+ int idx, int purge);
void filt_kqdetach(struct knote *kn);
int filt_kqueue(struct knote *kn, long hint);
@@ -1563,10 +1563,10 @@ kqueue_purge(struct proc *p, struct kque
mtx_enter(&kq->kq_lock);
for (i = 0; i < kq->kq_knlistsize; i++)
- knote_remove(p, kq, &kq->kq_knlist[i], 1);
+ knote_remove(p, kq, &kq->kq_knlist, i, 1);
if (kq->kq_knhashmask != 0) {
for (i = 0; i < kq->kq_knhashmask + 1; i++)
- knote_remove(p, kq, &kq->kq_knhash[i], 1);
+ knote_remove(p, kq, &kq->kq_knhash, i, 1);
}
mtx_leave(&kq->kq_lock);
}
@@ -1789,13 +1789,15 @@ knote(struct klist *list, long hint)
* remove all knotes from a specified knlist
*/
void
-knote_remove(struct proc *p, struct kqueue *kq, struct knlist *list, int purge)
+knote_remove(struct proc *p, struct kqueue *kq, struct knlist **plist, int idx,
+ int purge)
{
struct knote *kn;
MUTEX_ASSERT_LOCKED(&kq->kq_lock);
- while ((kn = SLIST_FIRST(list)) != NULL) {
+ /* Always fetch array pointer as another thread can resize kq_knlist. */
+ while ((kn = SLIST_FIRST(*plist + idx)) != NULL) {
KASSERT(kn->kn_kq == kq);
if (!purge) {
@@ -1863,7 +1865,7 @@ knote_fdclose(struct proc *p, int fd)
LIST_FOREACH(kq, &fdp->fd_kqlist, kq_next) {
mtx_enter(&kq->kq_lock);
if (fd < kq->kq_knlistsize)
- knote_remove(p, kq, &kq->kq_knlist[fd], 0);
+ knote_remove(p, kq, &kq->kq_knlist, fd, 0);
mtx_leave(&kq->kq_lock);
}
}
