Fix a file descriptor close race in kqueue_register()

After inserting a knote, check that the associated file descriptor
still references the same file. Remove the knote if the descriptor
has changed because otherwise the kqueue becomes inconsistent with
the file descriptor table.

There is an analogous race in fcntl(F_SETLK). It is already handled,
but the code can be simplified by using the same check as in
kqueue_register().

Fix inspired by DragonFly BSD

OK mpi@, anton@

2 files changed, 35 insertions, 7 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index d967d543b38..a9adcb7a83e 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.192 2019/08/05 08:35:59 anton Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.193 2020/01/03 05:37:00 visa Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -262,6 +262,18 @@ fd_getfile_mode(struct filedesc *fdp, int fd, int mode)
 	return (fp);
 }
 
+int
+fd_checkclosed(struct filedesc *fdp, int fd, struct file *fp)
+{
+	int closed;
+
+	mtx_enter(&fdp->fd_fplock);
+	KASSERT(fd < fdp->fd_nfiles);
+	closed = (fdp->fd_ofiles[fd] != fp);
+	mtx_leave(&fdp->fd_fplock);
+	return (closed);
+}
+
 /*
  * System calls on descriptors.
  */
@@ -396,7 +408,7 @@ sys_fcntl(struct proc *p, void *v, register_t *retval)
 	} */ *uap = v;
 	int fd = SCARG(uap, fd);
 	struct filedesc *fdp = p->p_fd;
-	struct file *fp, *fp2;
+	struct file *fp;
 	struct vnode *vp;
 	int i, tmp, newmin, flg = F_POSIX;
 	struct flock fl;
@@ -570,8 +582,7 @@ restart:
 			goto out;
 		}
 
-		fp2 = fd_getfile(fdp, fd);
-		if (fp != fp2) {
+		if (fd_checkclosed(fdp, fd, fp)) {
 			/*
 			 * We have lost the race with close() or dup2();
 			 * unlock, pretend that we've won the race and that
@@ -583,8 +594,6 @@ restart:
 			VOP_ADVLOCK(vp, fdp, F_UNLCK, &fl, F_POSIX);
 			fl.l_type = F_UNLCK;
 		}
-		if (fp2 != NULL)
-			FRELE(fp2, p);
 		goto out;
 
 
diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index c634353caa5..3141af589ce 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_event.c,v 1.113 2019/12/31 14:09:56 visa Exp $	*/
+/*	$OpenBSD: kern_event.c,v 1.114 2020/01/03 05:37:00 visa Exp $	*/
 
 /*-
  * Copyright (c) 1999,2000,2001 Jonathan Lemon <jlemon@FreeBSD.org>
@@ -724,6 +724,25 @@ again:
 				knote_drop(kn, p);
 				goto done;
 			}
+
+			/*
+			 * If this is a file descriptor filter, check if
+			 * fd was closed while the knote was being added.
+			 * knote_fdclose() has missed kn if the function
+			 * ran before kn appeared in kq_knlist.
+			 */
+			if (fops->f_isfd &&
+			    fd_checkclosed(fdp, kev->ident, kn->kn_fp)) {
+				/*
+				 * Drop the knote silently without error
+				 * because another thread might already have
+				 * seen it. This corresponds to the insert
+				 * happening in full before the close.
+				 */
+				kn->kn_fop->f_detach(kn);
+				knote_drop(kn, p);
+				goto done;
+			}
 		} else {
 			/*
 			 * The user may change some filter values after the