Release the file descriptor table lock before calling closef()

in finishdup(). This makes the order of operations similar to that
of fdrelease() and removes a case where lock ordering might cause
problems.

OK anton@, mpi@

1 files changed, 23 insertions, 21 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 0d9ebf5b30b..059a62120a0 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.199 2020/02/18 03:47:18 visa Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.200 2020/02/26 13:54:52 visa Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -307,14 +307,11 @@ restart:
 			fdpunlock(fdp);
 			goto restart;
 		}
-		goto out;
+		fdpunlock(fdp);
+		return (error);
 	}
 	/* No need for FRELE(), finishdup() uses current ref. */
-	error = finishdup(p, fp, old, new, retval, 0);
-
-out:
-	fdpunlock(fdp);
-	return (error);
+	return (finishdup(p, fp, old, new, retval, 0));
 }
 
 /*
@@ -382,7 +379,8 @@ restart:
 				fdpunlock(fdp);
 				goto restart;
 			}
-			goto out;
+			fdpunlock(fdp);
+			return (error);
 		}
 		if (new != i)
 			panic("dup2: fdalloc");
@@ -394,11 +392,7 @@ restart:
 		dupflags |= DUPF_CLOEXEC;
 
 	/* No need for FRELE(), finishdup() uses current ref. */
-	error = finishdup(p, fp, old, new, retval, dupflags);
-
-out:
-	fdpunlock(fdp);
-	return (error);
+	return (finishdup(p, fp, old, new, retval, dupflags));
 }
 
 /*
@@ -445,6 +439,7 @@ restart:
 				fdpunlock(fdp);
 				goto restart;
 			}
+			fdpunlock(fdp);
 		} else {
 			int dupflags = 0;
 
@@ -454,8 +449,6 @@ restart:
 			/* No need for FRELE(), finishdup() uses current ref. */
 			error = finishdup(p, fp, fd, i, retval, dupflags);
 		}
-
-		fdpunlock(fdp);
 		return (error);
 
 	case F_GETFD:
@@ -657,23 +650,24 @@ finishdup(struct proc *p, struct file *fp, int old, int new,
 {
 	struct file *oldfp;
 	struct filedesc *fdp = p->p_fd;
+	int error;
 
 	fdpassertlocked(fdp);
 	KASSERT(fp->f_iflags & FIF_INSERTED);
 
 	if (fp->f_count >= FDUP_MAX_COUNT) {
-		FRELE(fp, p);
-		return (EDEADLK);
+		error = EDEADLK;
+		goto fail;
 	}
 
 	oldfp = fd_getfile(fdp, new);
 	if ((dupflags & DUPF_DUP2) && oldfp == NULL) {
 		if (fd_inuse(fdp, new)) {
- 			FRELE(fp, p);
- 			return (EBUSY);
- 		}
+			error = EBUSY;
+			goto fail;
+		}
 		fd_used(fdp, new);
- 	}
+	}
 
 	/*
 	 * Use `fd_fplock' to synchronize with fd_getfile() so that
@@ -690,10 +684,18 @@ finishdup(struct proc *p, struct file *fp, int old, int new,
 
 	if (oldfp != NULL) {
 		knote_fdclose(p, new);
+		fdpunlock(fdp);
 		closef(oldfp, p);
+	} else {
+		fdpunlock(fdp);
 	}
 
 	return (0);
+
+fail:
+	fdpunlock(fdp);
+	FRELE(fp, p);
+	return (error);
 }
 
 void