Revert the blocked FIFO open fix, as there's either a race in the

ref-count handling under handling, or blambert@ is messing with my head

3 files changed, 26 insertions, 47 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index cd6385c1066..bafc9d0e305 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.97 2012/07/08 10:55:10 guenther Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.98 2012/07/11 23:07:19 guenther Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -1196,11 +1196,9 @@ filedescopen(dev_t dev, int mode, int type, struct proc *p)
  * Duplicate the specified descriptor to a free descriptor.
  */
 int
-dupfdopen(struct proc *p, int indx, struct file *fp, int mode)
+dupfdopen(struct filedesc *fdp, int indx, int dfd, int mode)
 {
-	struct filedesc *fdp = p->p_fd;
 	struct file *wfp;
-	int dfd = p->p_dupfd;
 
 	fdpassertlocked(fdp);
 
@@ -1208,10 +1206,10 @@ dupfdopen(struct proc *p, int indx, struct file *fp, int mode)
 	 * Assume that the filename was user-specified; applications do
 	 * not tend to open /dev/fd/# when they can just call dup()
 	 */
-	if ((p->p_p->ps_flags & (PS_SUGIDEXEC | PS_SUGID))) {
-		if (p->p_descfd == 255)
+	if ((curproc->p_p->ps_flags & (PS_SUGIDEXEC | PS_SUGID))) {
+		if (curproc->p_descfd == 255)
 			return (EPERM);
-		if (p->p_descfd != dfd)
+		if (curproc->p_descfd != curproc->p_dupfd)
 			return (EPERM);
 	}
 
@@ -1234,23 +1232,10 @@ dupfdopen(struct proc *p, int indx, struct file *fp, int mode)
 	if (wfp->f_count == LONG_MAX-2)
 		return (EDEADLK);
 
-	FREF(wfp);
-
-	/*
-	 * Check whether the larval fd was close behind our back.
-	 * If it was, we act like the open() completed before the
-	 * close(): fake up a closef(), to provide POSIX lock semantics,
-	 * then return success.
-	 */
-	if (fdp->fd_ofiles[indx] != fp) {
-		FREF(wfp);	/* yes, again: closef() decrements it twice */
-		closef(wfp, p);
-		return (0);
-	}
-
 	fdp->fd_ofiles[indx] = wfp;
 	fdp->fd_ofileflags[indx] = (fdp->fd_ofileflags[indx] & UF_EXCLOSE) |
 	    (fdp->fd_ofileflags[dfd] & ~UF_EXCLOSE);
+	wfp->f_count++;
 	fd_used(fdp, indx);
 	return (0);
 }
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index c6462898d5e..b8ccd2b94d6 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.186 2012/07/10 17:00:55 guenther Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.187 2012/07/11 23:07:19 guenther Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -856,14 +856,12 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 	struct nameidata nd;
 
 	fdplock(fdp);
-	if ((error = falloc(p, &fp, &indx)) != 0) {
-		fdpunlock(fdp);
-		return (error);
-	}
+
+	if ((error = falloc(p, &fp, &indx)) != 0)
+		goto out;
 	flags = FFLAGS(oflags);
 	if (flags & O_CLOEXEC)
 		fdp->fd_ofileflags[indx] |= UF_EXCLOSE;
-	fdpunlock(fdp);
 
 	cmode = ((mode &~ fdp->fd_cmask) & ALLPERMS) &~ S_ISTXT;
 	NDINITAT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, fd, path, p);
@@ -873,16 +871,19 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 		flags &= ~O_TRUNC;	/* Must do truncate ourselves */
 	}
 	if ((error = vn_open(&nd, flags, cmode)) != 0) {
-		fdplock(fdp);
 		if (error == ENODEV &&
 		    p->p_dupfd >= 0 &&			/* XXX from fdopen */
-		    (error = dupfdopen(p, indx, fp, flags)) == 0) {
+		    (error =
+			dupfdopen(fdp, indx, p->p_dupfd, flags)) == 0) {
+			closef(fp, p);
 			*retval = indx;
 			goto out;
 		}
 		if (error == ERESTART)
 			error = EINTR;
-		goto err;
+		fdremove(fdp, indx);
+		closef(fp, p);
+		goto out;
 	}
 	p->p_dupfd = 0;
 	vp = nd.ni_vp;
@@ -904,9 +905,10 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 		VOP_UNLOCK(vp, 0, p);
 		error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type);
 		if (error) {
-			/* FRELE will vn_close the file for us. */
-			fdplock(fdp);
-			goto err;
+			/* closef will vn_close the file for us. */
+			fdremove(fdp, indx);
+			closef(fp, p);
+			goto out;
 		}
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
 		fp->f_flag |= FHASLOCK;
@@ -925,25 +927,17 @@ doopenat(struct proc *p, int fd, const char *path, int oflags, mode_t mode,
 		}
 		if (error) {
 			VOP_UNLOCK(vp, 0, p);
-			/* FRELE will close the file for us. */
-			fdplock(fdp);
-			goto err;
+			/* closef will close the file for us. */
+			fdremove(fdp, indx);
+			closef(fp, p);
+			goto out;
 		}
 	}
 	VOP_UNLOCK(vp, 0, p);
 	*retval = indx;
 	FILE_SET_MATURE(fp, p);
-	return (0);
-
-err:
-	/* only remove the fd if the file didn't change behind out back */
-	if (fdp->fd_ofiles[indx] == fp) {
-		fdremove(fdp, indx);
-		FRELE(fp, p);
-	}
 out:
 	fdpunlock(fdp);
-	FRELE(fp, p);
 	return (error);
 }
 
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index 6a4061aedff..03e8d36d88b 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: filedesc.h,v 1.24 2012/07/08 10:55:10 guenther Exp $	*/
+/*	$OpenBSD: filedesc.h,v 1.25 2012/07/11 23:07:19 guenther Exp $	*/
 /*	$NetBSD: filedesc.h,v 1.14 1996/04/09 20:55:28 cgd Exp $	*/
 
 /*
@@ -121,7 +121,7 @@ struct filedesc0 {
  * Kernel global variables and routines.
  */
 void	filedesc_init(void);
-int	dupfdopen(struct proc *, int, struct file *, int);
+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);