Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Jul 2026 17:50:29 +0000
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 5a4222a1b225 - releng/15.1 - kqueue: Avoid enqueuing an already-enqueued knote
Message-ID:  <6a6a3d65.3bd96.2dcb7688@gitrepo.freebsd.org>

index | next in thread | raw e-mail

The branch releng/15.1 has been updated by markj:

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

commit 5a4222a1b225dc151d4262f1bb6e6ff5c1c0c79a
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-07-27 15:28:50 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-07-28 15:28:11 +0000

    kqueue: Avoid enqueuing an already-enqueued knote
    
    knotes with a non-trivial f_copy implementation may be activated before
    kqueue_fork_copy_knote() is finished.  In particular, it may be enqueued
    at the time that kqueue_fork_copy_knote() calls knote_enqueue().  Guard
    against this.
    
    Add a test case which triggers the race.
    
    Fix several other problems with the replication of knote state:
    - Make sure only the KN_ACTIVE and KN_DISABLED status flags are
      inherited, the rest should not be copied.
    - Ignore marker knotes.
    - Ignore knotes for kqueues.  They cannot be safely copied into the
      child without more work, as kqueues are inherently local to a process;
      on fork, we need to ensure that such knotes are patched to reference
      the new kqueue, not the original.
    - Try to keep knote state stable by holding the kqueue and knlist locks
      while copying.
    
    Approved by:    so
    Security:       FreeBSD-SA-26:50.kqueue
    Security:       CVE-2026-58083
    Reviewed by:    kib
    Reported by:    Hazley Samsudin of GovTech CSG
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D58223
---
 sys/kern/kern_event.c          | 43 ++++++++++++++++++++++++++++++++++--------
 sys/sys/event.h                |  1 +
 tests/sys/kqueue/kqueue_fork.c | 36 +++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 01bd22fbefd1..706f0e6d658e 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -3072,13 +3072,29 @@ kqueue_fork_alloc(struct filedesc *fdp, struct file *fp, struct file **fp1,
 }
 
 static void
-kqueue_fork_copy_knote(struct kqueue *kq1, struct knote *kn, struct proc *p1,
-    struct filedesc *fdp)
+kqueue_fork_copy_knote(struct kqueue *kq, struct kqueue *kq1, struct knote *kn,
+    struct proc *p1, struct filedesc *fdp)
 {
 	struct knote *kn1;
+	struct knlist *knl;
 	const struct filterops *fop;
 	int error;
+	bool enqueue;
 
+	KASSERT(kn->kn_influx != 0,
+	    ("%s: knote %p not in flux", __func__, kn));
+	KASSERT((kn->kn_status & KN_DETACHED) == 0,
+	    ("%s: knote %p not detached", __func__, kn));
+
+	if ((kn->kn_status & KN_MARKER) != 0)
+		return;
+	if ((kn->kn_status & KN_KQUEUE) != 0) {
+		/*
+		 * We cannot hold references to a kqueue outside of the process
+		 * itself, kqueue_close() does not handle this possibility.
+		 */
+		return;
+	}
 	fop = kn->kn_fop;
 	if (fop->f_copy == NULL || (fop->f_isfd &&
 	    fdp->fd_files->fdt_ofiles[kn->kn_kevent.ident].fde_file == NULL))
@@ -3088,9 +3104,13 @@ kqueue_fork_copy_knote(struct kqueue *kq1, struct knote *kn, struct proc *p1,
 		return;
 
 	kn1 = knote_alloc(M_WAITOK);
+
+	knl = kn_list_lock(kn);
+	KQ_LOCK(kq);
 	*kn1 = *kn;
-	kn1->kn_status |= KN_DETACHED;
-	kn1->kn_status &= ~KN_QUEUED;
+	KQ_UNLOCK(kq);
+	kn_list_unlock(knl);
+	kn1->kn_status = KN_DETACHED | (kn1->kn_status & KN_CPONFORK);
 	kn1->kn_kq = kq1;
 	kn1->kn_knlist = NULL;
 	error = fop->f_copy(kn1, p1);
@@ -3105,12 +3125,19 @@ kqueue_fork_copy_knote(struct kqueue *kq1, struct knote *kn, struct proc *p1,
 		knote_free(kn1);
 		return;
 	}
-	if (kn->kn_knlist != NULL)
-		knlist_add(kn->kn_knlist, kn1, 0);
+	if (kn->kn_knlist != NULL) {
+		knl = kn_list_lock(kn);
+		knlist_add(kn->kn_knlist, kn1, 1);
+	} else {
+		knl = NULL;
+	}
+	enqueue = kn->kn_fop->f_event(kn1, 0) != 0;
+	kn_list_unlock(knl);
+
 	KQ_LOCK(kq1);
 	knote_attach(kn1, kq1);
 	kn1->kn_influx = 0;
-	if ((kn->kn_status & KN_QUEUED) != 0)
+	if (enqueue && (kn1->kn_status & KN_QUEUED) == 0)
 		knote_enqueue(kn1);
 	KQ_UNLOCK(kq1);
 }
@@ -3134,7 +3161,7 @@ kqueue_fork_copy_list(struct klist *knlist, struct knote *marker,
 		kn_enter_flux(kn);
 		SLIST_INSERT_AFTER(kn, marker, kn_link);
 		KQ_UNLOCK(kq);
-		kqueue_fork_copy_knote(kq1, kn, p1, fdp);
+		kqueue_fork_copy_knote(kq, kq1, kn, p1, fdp);
 		KQ_LOCK(kq);
 		kn_leave_flux(kn);
 		kn = SLIST_NEXT(marker, kn_link);
diff --git a/sys/sys/event.h b/sys/sys/event.h
index de24c9226e4a..a863cd20f45d 100644
--- a/sys/sys/event.h
+++ b/sys/sys/event.h
@@ -315,6 +315,7 @@ struct knote {
 #define KN_MARKER	0x20			/* ignore this knote */
 #define KN_KQUEUE	0x40			/* this knote belongs to a kq */
 #define	KN_SCAN		0x100			/* flux set in kqueue_scan() */
+#define	KN_CPONFORK	(KN_ACTIVE | KN_DISABLED) /* state preserved by fork */
 	int			kn_influx;
 	unsigned int		kn_sfflags;	/* saved filter flags */
 	int64_t			kn_sdata;	/* saved data field */
diff --git a/tests/sys/kqueue/kqueue_fork.c b/tests/sys/kqueue/kqueue_fork.c
index ad8f69056e07..03b831e4f77c 100644
--- a/tests/sys/kqueue/kqueue_fork.c
+++ b/tests/sys/kqueue/kqueue_fork.c
@@ -269,10 +269,46 @@ ATF_TC_BODY(cponfork_notes, tc)
 	cponfork_notes_mask_check(info.si_status, true);
 }
 
+/*
+ * Exercise a rare race: while the kernel is copying knotes during a fork, try
+ * to set things up so that a new knote is activated while the copy is still in
+ * progress.
+ */
+ATF_TC_WITHOUT_HEAD(cponfork_timer_race);
+ATF_TC_BODY(cponfork_timer_race, tc)
+{
+	struct kevent ev;
+	int error, kq, status;
+	pid_t pid;
+
+	for (int i = 0; i < 100; i++) {
+		kq = kqueuex(KQUEUE_CPONFORK);
+		ATF_REQUIRE(kq >= 0);
+
+		EV_SET(&ev, 0, EVFILT_TIMER, EV_ADD | EV_ENABLE, NOTE_NSECONDS,
+		    1, NULL);
+		error = kevent(kq, &ev, 1, NULL, 0, NULL);
+		ATF_REQUIRE(error == 0);
+
+		pid = fork();
+		ATF_REQUIRE(pid != -1);
+		if (pid == 0)
+			_exit(0);
+
+		error = waitpid(pid, &status, 0);
+		ATF_REQUIRE(error != -1);
+		ATF_REQUIRE(WIFEXITED(status));
+		ATF_REQUIRE_EQ(WEXITSTATUS(status), 0);
+
+		ATF_REQUIRE(close(kq) == 0);
+	}
+}
+
 ATF_TP_ADD_TCS(tp)
 {
 	ATF_TP_ADD_TC(tp, shared_table_filt_sig);
 	ATF_TP_ADD_TC(tp, cponfork_notes);
+	ATF_TP_ADD_TC(tp, cponfork_timer_race);
 
 	return (atf_no_error());
 }


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a6a3d65.3bd96.2dcb7688>