Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 1 Sep 2021 18:43:59 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: c511383de7a0 - main - kevent: Fix races between timer detach and kqtimer_proc_continue()
Message-ID:  <202109011843.181Ihx8e072768@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=c511383de7a0325a80b9c5d2b8678b438db146dc

commit c511383de7a0325a80b9c5d2b8678b438db146dc
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-09-01 18:18:58 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-09-01 18:18:58 +0000

    kevent: Fix races between timer detach and kqtimer_proc_continue()
    
    - When detaching a knote, we need to double check the enqueued flag
      after acquiring the process lock, as kqtimer_proc_continue() may have
      toggled it.
    - kqtimer_proc_continue() could in principle reschedule a stopped
      callout after filt_timerdetach() drains the callout.  So, we need to
      re-check.
    
    Reported by:    syzbot+4a4cebb3ec07892cb040@syzkaller.appspotmail.com
    Reported by:    syzbot+a9c04bc76078a3b7dd8d@syzkaller.appspotmail.com
    Reviewed by:    kib
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31772
---
 sys/kern/kern_event.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 63b28cd6024f..859248569f76 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -859,14 +859,24 @@ filt_timerdetach(struct knote *kn)
 {
 	struct kq_timer_cb_data *kc;
 	unsigned int old __unused;
+	bool pending;
 
 	kc = kn->kn_ptr.p_v;
-	callout_drain(&kc->c);
-	if ((kc->flags & KQ_TIMER_CB_ENQUEUED) != 0) {
+	do {
+		callout_drain(&kc->c);
+
+		/*
+		 * kqtimer_proc_continue() might have rescheduled this callout.
+		 * Double-check, using the process mutex as an interlock.
+		 */
 		PROC_LOCK(kc->p);
-		TAILQ_REMOVE(&kc->p->p_kqtim_stop, kc, link);
+		if ((kc->flags & KQ_TIMER_CB_ENQUEUED) != 0) {
+			kc->flags &= ~KQ_TIMER_CB_ENQUEUED;
+			TAILQ_REMOVE(&kc->p->p_kqtim_stop, kc, link);
+		}
+		pending = callout_pending(&kc->c);
 		PROC_UNLOCK(kc->p);
-	}
+	} while (pending);
 	free(kc, M_KQUEUE);
 	old = atomic_fetchadd_int(&kq_ncallouts, -1);
 	KASSERT(old > 0, ("Number of callouts cannot become negative"));



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202109011843.181Ihx8e072768>