Date: Fri, 27 Mar 2026 00:30:54 +0000 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: 8f3227f52756 - main - kqueue: Fix a race when adding an fd-based knote to a queue Message-ID: <69c5cfbe.45a44.22e5dbbb@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=8f3227f527567aef53da845ab78da8e16d9051c1 commit 8f3227f527567aef53da845ab78da8e16d9051c1 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2026-03-27 00:24:18 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2026-03-27 00:24:18 +0000 kqueue: Fix a race when adding an fd-based knote to a queue When registering a new kevent backed by a file descriptor, we first look up the file description with fget(), then lock the kqueue, then see if a corresponding knote is already registered. If not, and KN_ADD is specified, we add the knote to the kqueue. closefp_impl() interlocks with this process by calling knote_fdclose(), which locks each kqueue and checks to see if the fd is registered with a knote. But, if userspace closes an fd while a different thread is registering it, i.e., after fget() succeeds but before the kqueue is locked, then we may end up with a mismatch in the knote table, where the knote kn_fp field points to a different file description than the knote ident. Fix the problem by double-checking before registering a knote. Add a new fget_noref_unlocked() helper for this purpose. It is a clone of fget_noref(). We could simply use fget_noref(), but I like having an explicit unlocked variant. PR: 293382 Reviewed by: kib MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D55852 --- sys/kern/kern_event.c | 14 +++++++++++++- sys/sys/filedesc.h | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c index e8e670d39d09..f984161bfcd6 100644 --- a/sys/kern/kern_event.c +++ b/sys/kern/kern_event.c @@ -28,7 +28,6 @@ * SUCH DAMAGE. */ -#include <sys/cdefs.h> #include "opt_ktrace.h" #include "opt_kqueue.h" @@ -1803,6 +1802,19 @@ findkn: error = ENOMEM; goto done; } + + /* + * Now that the kqueue is locked, make sure the fd + * didn't change out from under us. + */ + if (fops->f_isfd && + fget_noref_unlocked(td->td_proc->p_fd, + kev->ident) != fp) { + KQ_UNLOCK(kq); + tkn = kn; + error = EBADF; + goto done; + } kn->kn_fp = fp; kn->kn_kq = kq; kn->kn_fop = fops; diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h index 4817855443af..c6499a18b884 100644 --- a/sys/sys/filedesc.h +++ b/sys/sys/filedesc.h @@ -212,6 +212,8 @@ struct filedesc_to_leader { #ifdef _KERNEL +#include <machine/atomic.h> + /* Operation types for kern_dup(). */ enum { FDDUP_NORMAL, /* dup() behavior. */ @@ -303,6 +305,21 @@ int fget_only_user(struct filedesc *fdp, int fd, MPASS(refcount_load(&fp->f_count) > 0); \ }) +/* + * Look up a file description without requiring a lock. In general the result + * may be immediately invalidated after the function returns, the caller must + * handle this. + */ +static inline struct file * +fget_noref_unlocked(struct filedesc *fdp, int fd) +{ + if (__predict_false( + (u_int)fd >= (u_int)atomic_load_int(&fdp->fd_nfiles))) + return (NULL); + + return (atomic_load_ptr(&fdp->fd_ofiles[fd].fde_file)); +} + /* Requires a FILEDESC_{S,X}LOCK held and returns without a ref. */ static __inline struct file * fget_noref(struct filedesc *fdp, int fd)home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?69c5cfbe.45a44.22e5dbbb>