From dc399801d5c5a8186948b30d54856b4091647d85 Mon Sep 17 00:00:00 2001 From: visa Date: Mon, 27 Jun 2022 13:35:21 +0000 Subject: [PATCH] kqueue: Clear task when closing kqueue When closing a kqueue, block until any pending wakeup task has finished. Otherwise, if a pending task progressed slowly, the kqueue could stay alive longer than the associated file descriptor table, causing a use-after-free in KQRELE(). This also fixes a failed assertion "p->p_kq->kq_refcnt.r_refs == 1" in kqpoll_exit(). The use-after-free bug had existed since the introduction of kqueue_task() (the bug could occur if fdplock() blocked in KQRELE()). However, the issue became worse when the task was allowed to run without the kernel lock in sys/kern/kern_event.c r1.187. Prompted by a report from Mikhail on bugs@. OK mpi@ Reported-by: syzbot+fca7e4fa773c90886819@syzkaller.appspotmail.com --- sys/kern/kern_event.c | 17 +++++++++++------ sys/sys/eventvar.h | 3 ++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c index 235f1a57005..5db97df588f 100644 --- a/sys/kern/kern_event.c +++ b/sys/kern/kern_event.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_event.c,v 1.190 2022/06/20 01:39:44 visa Exp $ */ +/* $OpenBSD: kern_event.c,v 1.191 2022/06/27 13:35:21 visa Exp $ */ /*- * Copyright (c) 1999,2000,2001 Jonathan Lemon @@ -1559,6 +1559,7 @@ void kqueue_terminate(struct proc *p, struct kqueue *kq) { struct knote *kn; + int state; mtx_enter(&kq->kq_lock); @@ -1571,11 +1572,17 @@ kqueue_terminate(struct proc *p, struct kqueue *kq) KASSERT(kn->kn_filter == EVFILT_MARKER); kq->kq_state |= KQ_DYING; + state = kq->kq_state; kqueue_wakeup(kq); mtx_leave(&kq->kq_lock); + /* + * Any knotes that were attached to this kqueue were deleted + * by knote_fdclose() when this kqueue's file descriptor was closed. + */ KASSERT(klist_empty(&kq->kq_sel.si_note)); - task_del(systqmp, &kq->kq_task); + if (state & KQ_TASK) + taskq_del_barrier(systqmp, &kq->kq_task); } int @@ -1601,7 +1608,6 @@ kqueue_task(void *arg) mtx_enter(&kqueue_klist_lock); KNOTE(&kq->kq_sel.si_note, 0); mtx_leave(&kqueue_klist_lock); - KQRELE(kq); } void @@ -1615,9 +1621,8 @@ kqueue_wakeup(struct kqueue *kq) } if (!klist_empty(&kq->kq_sel.si_note)) { /* Defer activation to avoid recursion. */ - KQREF(kq); - if (!task_add(systqmp, &kq->kq_task)) - KQRELE(kq); + kq->kq_state |= KQ_TASK; + task_add(systqmp, &kq->kq_task); } } diff --git a/sys/sys/eventvar.h b/sys/sys/eventvar.h index 1e20fb2cf53..e93b1494767 100644 --- a/sys/sys/eventvar.h +++ b/sys/sys/eventvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: eventvar.h,v 1.15 2022/06/20 01:39:44 visa Exp $ */ +/* $OpenBSD: eventvar.h,v 1.16 2022/06/27 13:35:21 visa Exp $ */ /*- * Copyright (c) 1999,2000 Jonathan Lemon @@ -67,6 +67,7 @@ struct kqueue { int kq_state; /* [q] */ #define KQ_SLEEP 0x02 #define KQ_DYING 0x04 +#define KQ_TASK 0x08 }; #endif /* !_SYS_EVENTVAR_H_ */ -- 2.20.1