Index | Thread | Search

From:
Tim Leslie <tleslie@protonmail.com>
Subject:
Remove fd_checkclosed
To:
"tech@openbsd.org" <tech@openbsd.org>
Date:
Mon, 24 Nov 2025 05:21:19 +0000

Download raw body.

Thread
All,

fd_checkclosed() is only used in two places, both as a best‑effort race detector rather than a strong synchronization point. In sys_fcntl()’s F_SETLK/F_SETLKW path, it is called after VOP_ADVLOCK() to see if close() or dup2() won the race and then issue a compensating F_UNLCK if so. In kqueue_kqfilter(), fd filters use it to check whether knote_fdclose() might have missed the knote while it was being attached.

In both cases, the caller already holds a reference on the struct file obtained earlier from fd_getfile(). This patch converts the check to a direct atomic grab and then removing the helper.​

There is no intended semantic change beyond avoiding an extra mutex acquisition around what is already documented as a racy snapshot in both call sites.​

—
Tim Leslie

diff --git a/share/man/man9/file.9 b/share/man/man9/file.9
--- a/share/man/man9/file.9
+++ b/share/man/man9/file.9
@@ -32,7 +32,6 @@
 .Nm FRELE ,
 .Nm fd_getfile ,
 .Nm fd_getfile_mode ,
-.Nm fd_checkclosed ,
 .Nm getsock ,
 .Nm getvnode
 .Nd an overview of file descriptor handling
@@ -52,8 +51,6 @@
 .Ft struct file *
 .Fn fd_getfile_mode "struct filedesc *fdp" "int fd" "int mode"
 .Ft int
-.Fn fd_checkclosed "struct filedesc *fdp" "int fd" "struct file *fp"
-.Ft int
 .Fn getsock "struct proc *p" "int fd" "struct file **fpp"
 .In sys/file.h
 .In sys/filedesc.h
@@ -112,13 +109,6 @@ is like
 .Fn fd_getfile
 but also checks if the file has been opened with the given mode.
 .Pp
-.Fn fd_checkclosed
-checks if file descriptor
-.Fa fd
-has been closed and no longer points to file
-.Fa fp .
-The file must have been retrieved from the descriptor previously.
-.Pp
 The files are extracted from the process context using the
 function
 .Fn getsock
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -262,18 +262,6 @@ 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.
  */
@@ -571,7 +559,7 @@ restart:
 			goto out;
 		}

-		if (fd_checkclosed(fdp, fd, fp)) {
+		if (READ_ONCE(fdp->fd_ofiles[fd]) != fp) {
 			/*
 			 * We have lost the race with close() or dup2();
 			 * unlock, pretend that we've won the race and that
diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -1501,7 +1501,7 @@ again:
 			 * ran before kn appeared in kq_knlist.
 			 */
 			if ((fops->f_flags & FILTEROP_ISFD) &&
-			    fd_checkclosed(fdp, kev->ident, kn->kn_fp)) {
+			    READ_ONCE(fdp->fd_ofiles[kev->ident]) != kn->kn_fp) {
 				/*
 				 * Drop the knote silently without error
 				 * because another thread might already have
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -148,7 +148,6 @@ void	fdprepforexec(struct proc *);
 struct file *fd_iterfile(struct file *, struct proc *);
 struct file *fd_getfile(struct filedesc *, int);
 struct file *fd_getfile_mode(struct filedesc *, int, int);
-int	fd_checkclosed(struct filedesc *, int, struct file *);

 int	closef(struct file *, struct proc *);
 int	getsock(struct proc *, int, struct file **);