From 5969ccd0a5db96ab0203a5511600e0f38cc0edd6 Mon Sep 17 00:00:00 2001
From: claudio <claudio@openbsd.org>
Date: Thu, 11 Feb 2021 12:08:21 +0000
Subject: [PATCH] In the various open functions reduce the fdplock() to only
 span over the function which need the lock (falloc, fdinsert, fdremove). In
 most cases it is not correct to hold the lock while calling VFS functions or
 e.g. closef since those aquire or release long lived VFS locks. OK visa@ mvs@

---
 sys/kern/vfs_syscalls.c | 43 ++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index 3bab69b15ac..840ea5453e1 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.348 2020/10/02 15:45:22 deraadt Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.349 2021/02/11 12:08:21 claudio Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -1104,8 +1104,10 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 	cloexec = (oflags & O_CLOEXEC) ? UF_EXCLOSE : 0;
 
 	fdplock(fdp);
-	if ((error = falloc(p, &fp, &indx)) != 0)
-		goto out;
+	if ((error = falloc(p, &fp, &indx)) != 0) {
+		fdpunlock(fdp);
+		return (error);
+	}
 	fdpunlock(fdp);
 
 	flags = FFLAGS(oflags);
@@ -1139,15 +1141,17 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 		    p->p_dupfd >= 0 &&			/* XXX from fdopen */
 		    (error =
 			dupfdopen(p, indx, flags)) == 0) {
+			fdpunlock(fdp);
 			closef(fp, p);
 			*retval = indx;
-			goto out;
+			return (error);
 		}
 		if (error == ERESTART)
 			error = EINTR;
 		fdremove(fdp, indx);
+		fdpunlock(fdp);
 		closef(fp, p);
-		goto out;
+		return (error);
 	}
 	p->p_dupfd = 0;
 	vp = nd.ni_vp;
@@ -1172,8 +1176,9 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 			fdplock(fdp);
 			/* closef will vn_close the file for us. */
 			fdremove(fdp, indx);
+			fdpunlock(fdp);
 			closef(fp, p);
-			goto out;
+			return (error);
 		}
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 		atomic_setbits_int(&fp->f_iflags, FIF_HASLOCK);
@@ -1195,17 +1200,17 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 			fdplock(fdp);
 			/* closef will close the file for us. */
 			fdremove(fdp, indx);
+			fdpunlock(fdp);
 			closef(fp, p);
-			goto out;
+			return (error);
 		}
 	}
 	VOP_UNLOCK(vp);
 	*retval = indx;
 	fdplock(fdp);
 	fdinsert(fdp, indx, cloexec, fp);
-	FRELE(fp, p);
-out:
 	fdpunlock(fdp);
+	FRELE(fp, p);
 	return (error);
 }
 
@@ -1235,8 +1240,10 @@ sys___tmpfd(struct proc *p, void *v, register_t *retval)
 	cloexec = (oflags & O_CLOEXEC) ? UF_EXCLOSE : 0;
 
 	fdplock(fdp);
-	if ((error = falloc(p, &fp, &indx)) != 0)
-		goto out;
+	if ((error = falloc(p, &fp, &indx)) != 0) {
+		fdpunlock(fdp);
+		return (error);
+	}
 	fdpunlock(fdp);
 
 	flags = FFLAGS(oflags);
@@ -1250,12 +1257,13 @@ sys___tmpfd(struct proc *p, void *v, register_t *retval)
 	cmode = 0600;
 	NDINITAT(&nd, 0, KERNELPATH, UIO_SYSSPACE, AT_FDCWD, path, p);
 	if ((error = vn_open(&nd, flags, cmode)) != 0) {
-		fdplock(fdp);
 		if (error == ERESTART)
 			error = EINTR;
+		fdplock(fdp);
 		fdremove(fdp, indx);
+		fdpunlock(fdp);
 		closef(fp, p);
-		goto out;
+		return (error);
 	}
 	vp = nd.ni_vp;
 	fp->f_flag = flags & FMASK;
@@ -1266,6 +1274,7 @@ sys___tmpfd(struct proc *p, void *v, register_t *retval)
 	*retval = indx;
 	fdplock(fdp);
 	fdinsert(fdp, indx, cloexec, fp);
+	fdpunlock(fdp);
 	FRELE(fp, p);
 
 	/* unlink it */
@@ -1288,8 +1297,6 @@ sys___tmpfd(struct proc *p, void *v, register_t *retval)
 		}
 	}
 
-out:
-	fdpunlock(fdp);
 	return (error);
 }
 
@@ -1370,9 +1377,11 @@ sys_fhopen(struct proc *p, void *v, register_t *retval)
 
 	fdplock(fdp);
 	if ((error = falloc(p, &fp, &indx)) != 0) {
+		fdpunlock(fdp);
 		fp = NULL;
 		goto bad;
 	}
+	fdpunlock(fdp);
 
 	if ((error = copyin(SCARG(uap, fhp), &fh, sizeof(fhandle_t))) != 0)
 		goto bad;
@@ -1449,6 +1458,7 @@ sys_fhopen(struct proc *p, void *v, register_t *retval)
 	}
 	VOP_UNLOCK(vp);
 	*retval = indx;
+	fdplock(fdp);
 	fdinsert(fdp, indx, cloexec, fp);
 	fdpunlock(fdp);
 	FRELE(fp, p);
@@ -1456,12 +1466,13 @@ sys_fhopen(struct proc *p, void *v, register_t *retval)
 
 bad:
 	if (fp) {
+		fdplock(fdp);
 		fdremove(fdp, indx);
+		fdpunlock(fdp);
 		closef(fp, p);
 		if (vp != NULL)
 			vput(vp);
 	}
-	fdpunlock(fdp);
 	return (error);
 }
 
-- 
2.20.1