split falloc into two functions, the allocation and the publication.

the first part is likely to fail, but we can generally prevent the second
part from failing.
use these two functions to drop the fdp lock around vn_open in sys_open,
but still maintain the original semantics of allocate then open.
the goal here is to fix the blocked fifo deadlock in a threaded program.

testing volunteers in short supply, resorting to conscription

3 files changed, 65 insertions, 21 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 3be945653b6..12473a37993 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.104 2013/09/14 01:35:00 guenther Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.105 2014/03/08 22:54:29 tedu Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -820,20 +820,28 @@ fdexpand(struct proc *p)
 int
 falloc(struct proc *p, struct file **resultfp, int *resultfd)
 {
-	struct file *fp, *fq;
-	int error, i;
+	int error;
 
-	fdpassertlocked(p->p_fd);
-restart:
-	if ((error = fdalloc(p, 0, &i)) != 0) {
-		if (error == ENOSPC) {
-			fdexpand(p);
-			goto restart;
-		}
+	if ((error = fcreate(p, resultfp)))
 		return (error);
-	}
+	if ((error = fpublish(p, *resultfp, resultfd)))
+		closef(*resultfp, p);
+	return (error);
+}
+
+int
+fcreate(struct proc *p, struct file **resultfp)
+{
+	struct filedesc *fdp = p->p_fd;
+	struct file *fp, *fq;
+	int lim;
+
+	fdpassertlocked(fdp);
+	lim = min((int)p->p_rlimit[RLIMIT_NOFILE].rlim_cur, maxfiles);
+	if (fdp->fd_openfd >= lim)
+		return (EMFILE);
+
 	if (nfiles >= maxfiles) {
-		fd_unused(p->p_fd, i);
 		tablefull("file");
 		return (ENFILE);
 	}
@@ -843,6 +851,7 @@ restart:
 	 * of open files at that point, otherwise put it at the front of
 	 * the list of open files.
 	 */
+	fdp->fd_openfd++;
 	nfiles++;
 	fp = pool_get(&file_pool, PR_WAITOK|PR_ZERO);
 	fp->f_iflags = FIF_LARVAL;
@@ -851,15 +860,35 @@ restart:
 	} else {
 		LIST_INSERT_HEAD(&filehead, fp, f_list);
 	}
-	p->p_fd->fd_ofiles[i] = fp;
 	fp->f_count = 1;
 	fp->f_cred = p->p_ucred;
 	crhold(fp->f_cred);
 	if (resultfp)
 		*resultfp = fp;
+	FREF(fp);
+	return (0);
+}
+
+int
+fpublish(struct proc *p, struct file *fp, int *resultfd)
+{
+	struct filedesc *fdp = p->p_fd;
+	int i, error;
+
+	fdpassertlocked(fdp);
+	fdp->fd_openfd--; /* undo previous increase by fcreate */
+restart:
+	if ((error = fdalloc(p, 0, &i)) != 0) {
+		if (error == ENOSPC) {
+			fdexpand(p);
+			goto restart;
+		}
+		return (error);
+	}
+
+	fdp->fd_ofiles[i] = fp;
 	if (resultfd)
 		*resultfd = i;
-	FREF(fp);
 	return (0);
 }
 
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index e76f391cabd..88928920569 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.202 2014/01/21 01:48:45 tedu Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.203 2014/03/08 22:54:30 tedu Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -841,17 +841,15 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 	struct vnode *vp;
 	struct vattr vattr;
 	int flags, cmode;
-	int type, indx, error, localtrunc = 0;
+	int type, indx, error, fpuberr, localtrunc = 0;
 	struct flock lf;
 	struct nameidata nd;
 
 	fdplock(fdp);
 
-	if ((error = falloc(p, &fp, &indx)) != 0)
+	if ((error = fcreate(p, &fp)) != 0)
 		goto out;
 	flags = FFLAGS(oflags);
-	if (flags & O_CLOEXEC)
-		fdp->fd_ofileflags[indx] |= UF_EXCLOSE;
 
 	cmode = ((mode &~ fdp->fd_cmask) & ALLPERMS) &~ S_ISTXT;
 	NDINITAT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, fd, path, p);
@@ -860,7 +858,22 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 		localtrunc = 1;
 		flags &= ~O_TRUNC;	/* Must do truncate ourselves */
 	}
-	if ((error = vn_open(&nd, flags, cmode)) != 0) {
+
+	fdpunlock(fdp);
+	error = vn_open(&nd, flags, cmode);
+	fdplock(fdp);
+
+	if ((fpuberr = fpublish(p, fp, &indx)) != 0) {
+		/* should never happen; just in case */
+		if (error == 0)
+			vn_close(nd.ni_vp, flags & FMASK, p->p_ucred, p);
+		closef(fp, p);
+		error = fpuberr;
+		goto out;
+	}
+	if (flags & O_CLOEXEC)
+		fdp->fd_ofileflags[indx] |= UF_EXCLOSE;
+	if (error != 0) {
 		if (error == ENODEV &&
 		    p->p_dupfd >= 0 &&			/* XXX from fdopen */
 		    (error =
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index 03e8d36d88b..8a61e41ff1c 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: filedesc.h,v 1.25 2012/07/11 23:07:19 guenther Exp $	*/
+/*	$OpenBSD: filedesc.h,v 1.26 2014/03/08 22:54:30 tedu Exp $	*/
 /*	$NetBSD: filedesc.h,v 1.14 1996/04/09 20:55:28 cgd Exp $	*/
 
 /*
@@ -125,6 +125,8 @@ int	dupfdopen(struct filedesc *, int, int, int);
 int	fdalloc(struct proc *p, int want, int *result);
 void	fdexpand(struct proc *);
 int	falloc(struct proc *p, struct file **resultfp, int *resultfd);
+int	fcreate(struct proc *p, struct file **resultfp);
+int	fpublish(struct proc *p, struct file *fp, int *resultfd);
 struct	filedesc *fdinit(struct proc *p);
 struct	filedesc *fdshare(struct proc *p);
 struct	filedesc *fdcopy(struct proc *p);