15031 event ports impacted by FDINFO accesses

Reviewed by: Ryan Goodfellow <ryan.goodfellow@oxide.computer>
Reviewed by: Joshua M. Clulow <josh@sysmgr.org>
Approved by: Dan McDonald <danmcd@mnx.io>

3 files changed, 56 insertions, 4 deletions

diff --git a/usr/src/uts/common/fs/proc/prdata.h b/usr/src/uts/common/fs/proc/prdata.h
index 45219eccd5..a661478c50 100644
--- a/usr/src/uts/common/fs/proc/prdata.h
+++ b/usr/src/uts/common/fs/proc/prdata.h
@@ -352,6 +352,7 @@ extern	int	pr_set(proc_t *, long);
 extern	int	pr_unset(proc_t *, long);
 extern	void	pr_sethold(prnode_t *, sigset_t *);
 extern	file_t	*pr_getf(proc_t *, uint_t, short *);
+extern	void	pr_releasef(file_t *);
 extern	void	pr_setfault(proc_t *, fltset_t *);
 extern	int	prusrio(proc_t *, enum uio_rw, struct uio *, int);
 extern	int	prwritectl(vnode_t *, struct uio *, cred_t *);
diff --git a/usr/src/uts/common/fs/proc/prsubr.c b/usr/src/uts/common/fs/proc/prsubr.c
index da43f59092..5591ffd89b 100644
--- a/usr/src/uts/common/fs/proc/prsubr.c
+++ b/usr/src/uts/common/fs/proc/prsubr.c
@@ -24,6 +24,7 @@
  * Copyright 2019 Joyent, Inc.
  * Copyright 2020 OmniOS Community Edition (OmniOSce) Association.
  * Copyright 2022 MNX Cloud, Inc.
+ * Copyright 2022 Oxide Computer Company
  */
 
 /*	Copyright (c) 1984, 1986, 1987, 1988, 1989 AT&T	*/
@@ -1615,6 +1616,54 @@ retry:
 	return (fp);
 }
 
+
+/*
+ * Just as pr_getf() is a little unusual in how it goes about making the file_t
+ * safe for procfs consumers to access it, so too is pr_releasef() for safely
+ * releasing that "hold".  The "hold" is unlike normal file descriptor activity
+ * -- procfs is just an interloper here, wanting access to the vnode_t without
+ * risk of a racing close() disrupting the state.  Just as pr_getf() avoids some
+ * of the typical file_t behavior (such as auditing) when establishing its hold,
+ * so too should pr_releasef().  It should not go through the motions of
+ * closef() (since it is not a true close()) unless racing activity causes it to
+ * be the last actor holding the refcount above zero.
+ *
+ * Under normal circumstances, we expect to find file_t`f_count > 1 after
+ * the successful pr_getf() call.  We are, after all, accessing a resource
+ * already held by the process in question.  We would also expect to rarely race
+ * with a close() of the underlying fd, meaning that file_t`f_count > 1 would
+ * still holds at pr_releasef() time.  That would mean we only need to decrement
+ * f_count, leaving it to the process to later close the fd (thus triggering
+ * VOP_CLOSE(), etc).
+ *
+ * It is only when that process manages to close() the fd while we have it
+ * "held" in procfs that we must make a trip through the traditional closef()
+ * logic to ensure proper tear-down of the file_t.
+ */
+void
+pr_releasef(file_t *fp)
+{
+	mutex_enter(&fp->f_tlock);
+	if (fp->f_count > 1) {
+		/*
+		 * This is the most common case: The file is still held open by
+		 * the process, and we simply need to release our hold by
+		 * decrementing f_count
+		 */
+		fp->f_count--;
+		mutex_exit(&fp->f_tlock);
+	} else {
+		/*
+		 * A rare occasion: The process snuck a close() of this file
+		 * while we were doing our business in procfs.  Given that
+		 * f_count == 1, we are the only one with a reference to the
+		 * file_t and need to take a trip through closef() to free it.
+		 */
+		mutex_exit(&fp->f_tlock);
+		(void) closef(fp);
+	}
+}
+
 void
 pr_object_name(char *name, vnode_t *vp, struct vattr *vattr)
 {
diff --git a/usr/src/uts/common/fs/proc/prvnops.c b/usr/src/uts/common/fs/proc/prvnops.c
index f2a924095e..e535b1f647 100644
--- a/usr/src/uts/common/fs/proc/prvnops.c
+++ b/usr/src/uts/common/fs/proc/prvnops.c
@@ -25,6 +25,7 @@
  * Copyright (c) 2017 by Delphix. All rights reserved.
  * Copyright 2020 OmniOS Community Edition (OmniOSce) Association.
  * Copyright 2022 MNX Cloud, Inc.
+ * Copyright 2022 Oxide Computer Company
  */
 
 /*	Copyright (c) 1984,	 1986, 1987, 1988, 1989 AT&T	*/
@@ -886,7 +887,7 @@ pr_read_fdinfo(prnode_t *pnp, uio_t *uiop, cred_t *cr)
 
 	error = prgetfdinfo(p, fp->f_vnode, fdinfo, cr, fp->f_cred, &data);
 
-	(void) closef(fp);
+	pr_releasef(fp);
 
 out:
 	if (error == 0)
@@ -3207,7 +3208,7 @@ prgetattr(vnode_t *vp, vattr_t *vap, int flags, cred_t *cr,
 		prunlock(pnp);
 		vap->va_size = prgetfdinfosize(p, fp->f_vnode, cr);
 		vap->va_nblocks = (fsblkcnt64_t)btod(vap->va_size);
-		(void) closef(fp);
+		pr_releasef(fp);
 		return (0);
 	}
 	case PR_LWPDIR:
@@ -4306,8 +4307,9 @@ pr_lookup_fddir(vnode_t *dp, char *comp)
 	}
 
 	prunlock(dpnp);
-	if (fp != NULL)
-		(void) closef(fp);
+	if (fp != NULL) {
+		pr_releasef(fp);
+	}
 
 	if (vp == NULL) {
 		prfreenode(pnp);