lock filedesc before manipulating. avoids some rare races.

testing for quite some time by brad + otto

6 files changed, 90 insertions, 40 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 256e7501c41..5650bd03f38 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.66 2003/12/02 01:40:18 millert Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.67 2004/01/06 04:18:18 tedu Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -223,15 +223,21 @@ restart:
 	if ((fp = fd_getfile(fdp, old)) == NULL)
 		return (EBADF);
 	FREF(fp);
+	fdplock(fdp, p);
 	if ((error = fdalloc(p, 0, &new)) != 0) {
 		FRELE(fp);
 		if (error == ENOSPC) {
 			fdexpand(p);
+			fdpunlock(fdp);
 			goto restart;
 		}
-		return (error);
+		goto out;
 	}
-	return (finishdup(p, fp, old, new, retval));
+	error = finishdup(p, fp, old, new, retval);
+
+out:
+	fdpunlock(fdp);
+	return (error);
 }
 
 /*
@@ -269,20 +275,26 @@ restart:
 		return (0);
 	}
 	FREF(fp);
+	fdplock(fdp, p);
 	if (new >= fdp->fd_nfiles) {
 		if ((error = fdalloc(p, new, &i)) != 0) {
 			FRELE(fp);
 			if (error == ENOSPC) {
 				fdexpand(p);
+				fdpunlock(fdp);
 				goto restart;
 			}
-			return (error);
+			goto out;
 		}
 		if (new != i)
 			panic("dup2: fdalloc");
 	}
 	/* finishdup() does FRELE */
-	return (finishdup(p, fp, old, new, retval));
+	error = finishdup(p, fp, old, new, retval);
+
+out:
+	fdpunlock(fdp);
+	return (error);
 }
 
 /*
@@ -321,16 +333,23 @@ restart:
 			error = EINVAL;
 			break;
 		}
+		fdplock(fdp, p);
 		if ((error = fdalloc(p, newmin, &i)) != 0) {
 			if (error == ENOSPC) {
 				fdexpand(p);
 				FRELE(fp);
+				fdpunlock(fdp);
 				goto restart;
 			}
-			break;
 		}
 		/* finishdup will FRELE for us. */
-		return (finishdup(p, fp, fd, i, retval));
+		if (!error)
+			error = finishdup(p, fp, fd, i, retval);
+		else
+			FRELE(fp);
+
+		fdpunlock(fdp);
+		return (error);
 
 	case F_GETFD:
 		*retval = fdp->fd_ofileflags[fd] & UF_EXCLOSE ? 1 : 0;
@@ -577,12 +596,16 @@ sys_close(p, v, retval)
 	struct sys_close_args /* {
 		syscallarg(int) fd;
 	} */ *uap = v;
-	int fd = SCARG(uap, fd);
+	int fd = SCARG(uap, fd), error;
 	struct filedesc *fdp = p->p_fd;
 
 	if (fd_getfile(fdp, fd) == NULL)
 		return (EBADF);
-	return (fdrelease(p, fd));
+	fdplock(fdp, p);
+	error = fdrelease(p, fd);
+	fdpunlock(fdp);
+
+	return (error);
 }
 
 /*
@@ -732,13 +755,6 @@ fdexpand(p)
 	u_int *newhimap, *newlomap;
 
 	/*
-	 * If it's already expanding just wait for that expansion to finish
-	 * and return and let the caller retry the operation.
-	 */
-	if (lockmgr(&fdp->fd_lock, LK_EXCLUSIVE|LK_SLEEPFAIL, NULL, p))
-		return;
-
-	/*
 	 * No space in current array.
 	 */
 	if (fdp->fd_nfiles < NDEXTENT)
@@ -789,8 +805,6 @@ fdexpand(p)
 	fdp->fd_ofiles = newofile;
 	fdp->fd_ofileflags = newofileflags;
 	fdp->fd_nfiles = nfiles;	
-
-	lockmgr(&fdp->fd_lock, LK_RELEASE, NULL, p);
 }
 
 /*
@@ -866,7 +880,7 @@ fdinit(struct proc *p)
 		if (newfdp->fd_fd.fd_rdir)
 			VREF(newfdp->fd_fd.fd_rdir);
 	}
-	lockinit(&newfdp->fd_fd.fd_lock, PLOCK, "fdexpand", 0, 0);
+	rw_init(&newfdp->fd_fd.fd_lock);
 
 	/* Create the file descriptor table. */
 	newfdp->fd_fd.fd_refcnt = 1;
diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c
index 31643d56fc3..489198f9288 100644
--- a/sys/kern/sys_pipe.c
+++ b/sys/kern/sys_pipe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sys_pipe.c,v 1.45 2003/10/03 16:38:01 miod Exp $	*/
+/*	$OpenBSD: sys_pipe.c,v 1.46 2004/01/06 04:18:18 tedu Exp $	*/
 
 /*
  * Copyright (c) 1996 John S. Dyson
@@ -111,6 +111,8 @@ sys_opipe(p, v, retval)
 	struct pipe *rpipe, *wpipe;
 	int fd, error;
 
+	fdplock(fdp, p);
+
 	rpipe = pool_get(&pipe_pool, PR_WAITOK);
 	error = pipe_create(rpipe);
 	if (error != 0)
@@ -143,7 +145,10 @@ sys_opipe(p, v, retval)
 
 	FILE_SET_MATURE(rf);
 	FILE_SET_MATURE(wf);
+
+	fdpunlock(fdp);
 	return (0);
+
 free3:
 	fdremove(fdp, retval[0]);
 	closef(rf, p);
@@ -153,6 +158,7 @@ free2:
 free1:
 	if (rpipe != NULL)
 		(void)pipeclose(rpipe);
+	fdpunlock(fdp);
 	return (error);
 }
 
diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index c2bf148b725..e130c3eafb4 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_syscalls.c,v 1.56 2003/09/01 18:06:03 henning Exp $	*/
+/*	$OpenBSD: uipc_syscalls.c,v 1.57 2004/01/06 04:18:18 tedu Exp $	*/
 /*	$NetBSD: uipc_syscalls.c,v 1.19 1996/02/09 19:00:48 christos Exp $	*/
 
 /*
@@ -75,8 +75,10 @@ sys_socket(p, v, retval)
 	struct file *fp;
 	int fd, error;
 
+	fdplock(fdp, p);
+
 	if ((error = falloc(p, &fp, &fd)) != 0)
-		return (error);
+		goto out;
 	fp->f_flag = FREAD|FWRITE;
 	fp->f_type = DTYPE_SOCKET;
 	fp->f_ops = &socketops;
@@ -90,6 +92,8 @@ sys_socket(p, v, retval)
 		FILE_SET_MATURE(fp);
 		*retval = fd;
 	}
+out:
+	fdpunlock(fdp);
 	return (error);
 }
 
@@ -206,6 +210,7 @@ sys_accept(p, v, retval)
 	/* Take note if socket was non-blocking. */
 	nflag = (fp->f_flag & FNONBLOCK);
 
+	fdplock(p->p_fd, p);
 	if ((error = falloc(p, &fp, &tmpfd)) != 0) {
 		/*
 		 * Probably ran out of file descriptors. Put the
@@ -249,6 +254,7 @@ sys_accept(p, v, retval)
 	}
 	m_freem(nam);
 bad:
+	fdpunlock(p->p_fd);
 	splx(s);
 	FRELE(headfp);
 	return (error);
@@ -337,6 +343,8 @@ sys_socketpair(p, v, retval)
 			 SCARG(uap, protocol));
 	if (error)
 		goto free1;
+
+	fdplock(fdp, p);
 	if ((error = falloc(p, &fp1, &fd)) != 0)
 		goto free2;
 	sv[0] = fd;
@@ -364,7 +372,8 @@ sys_socketpair(p, v, retval)
 	if (error == 0) {
 		FILE_SET_MATURE(fp1);
 		FILE_SET_MATURE(fp2);
-		return (error);
+		fdpunlock(fdp);
+		return (0);
 	}
 free4:
 	fdremove(fdp, sv[1]);
@@ -377,6 +386,7 @@ free3:
 free2:
 	if (so2 != NULL)
 		(void)soclose(so2);
+	fdpunlock(fdp);
 free1:
 	if (so1 != NULL)
 		(void)soclose(so1);
@@ -931,8 +941,10 @@ sys_pipe(struct proc *p, void *v, register_t *retval)
 	error = copyout((caddr_t)fds, (caddr_t)SCARG(uap, fdp),
 	    2 * sizeof (int));
 	if (error) {
+		fdplock(p->p_fd, p);
 		fdrelease(p, fds[0]);
 		fdrelease(p, fds[1]);
+		fdpunlock(p->p_fd);
 	}
 	return (error);
 }
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 7ac3a42729c..af41b16a6ac 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_usrreq.c,v 1.24 2003/08/17 22:59:42 tedu Exp $	*/
+/*	$OpenBSD: uipc_usrreq.c,v 1.25 2004/01/06 04:18:18 tedu Exp $	*/
 /*	$NetBSD: uipc_usrreq.c,v 1.18 1996/02/09 19:00:50 christos Exp $	*/
 
 /*
@@ -656,6 +656,7 @@ unp_externalize(rights)
 #endif
 
 restart:
+	fdplock(p->p_fd, p);
 	if (error != 0) {
 		rp = ((struct file **)CMSG_DATA(cm));
 		for (i = 0; i < nfds; i++) {
@@ -696,6 +697,7 @@ restart:
 				 */
 				error = EMSGSIZE;
 			}
+			fdpunlock(p->p_fd);
 			goto restart;
 		}
 
@@ -726,6 +728,7 @@ restart:
 	cm->cmsg_len = CMSG_LEN(nfds * sizeof(int));
 	rights->m_len = CMSG_SPACE(nfds * sizeof(int));
  out:
+	fdpunlock(p->p_fd);
 	free(fdp, M_TEMP);
 	return (error);
 }
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index bf738cf8aef..93e38884320 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.107 2003/09/01 18:06:03 henning Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.108 2004/01/06 04:18:18 tedu Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -892,8 +892,10 @@ sys_open(p, v, retval)
 	struct flock lf;
 	struct nameidata nd;
 
+	fdplock(fdp, p);
+
 	if ((error = falloc(p, &fp, &indx)) != 0)
-		return (error);
+		goto out;
 
 	flags = FFLAGS(SCARG(uap, flags));
 	cmode = ((SCARG(uap, mode) &~ fdp->fd_cmask) & ALLPERMS) &~ S_ISTXT;
@@ -910,13 +912,13 @@ sys_open(p, v, retval)
 			dupfdopen(fdp, indx, p->p_dupfd, flags, error)) == 0) {
 			closef(fp, p);
 			*retval = indx;
-			return (0);
+			goto out;
 		}
 		if (error == ERESTART)
 			error = EINTR;
 		fdremove(fdp, indx);
 		closef(fp, p);
-		return (error);
+		goto out;
 	}
 	p->p_dupfd = 0;
 	vp = nd.ni_vp;
@@ -941,7 +943,7 @@ sys_open(p, v, retval)
 			/* closef will vn_close the file for us. */
 			fdremove(fdp, indx);
 			closef(fp, p);
-			return (error);
+			goto out;
 		}
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
 		fp->f_flag |= FHASLOCK;
@@ -964,13 +966,15 @@ sys_open(p, v, retval)
 			/* closef will close the file for us. */
 			fdremove(fdp, indx);
 			closef(fp, p);
-			return (error);
+			goto out;
 		}
 	}
 	VOP_UNLOCK(vp, 0, p);
 	*retval = indx;
 	FILE_SET_MATURE(fp);
-	return (0);
+out:
+	fdpunlock(fdp);
+	return (error);
 }
 
 /*
@@ -1052,8 +1056,11 @@ sys_fhopen(p, v, retval)
 	if ((flags & O_CREAT))
 		return (EINVAL);
 
-	if ((error = falloc(p, &fp, &indx)) != 0)
-		return (error);
+	fdplock(fdp, p);
+	if ((error = falloc(p, &fp, &indx)) != 0) {
+		fp = NULL;
+		goto bad;
+	}
 
 	if ((error = copyin(SCARG(uap, fhp), &fh, sizeof(fhandle_t))) != 0)
 		goto bad;
@@ -1132,13 +1139,18 @@ sys_fhopen(p, v, retval)
 	VOP_UNLOCK(vp, 0, p);
 	*retval = indx;
 	FILE_SET_MATURE(fp);
+
+	fdpunlock(fdp);
 	return (0);
 
 bad:
-	fdremove(fdp, indx);
-	closef(fp, p);
-	if (vp != NULL)
-		vput(vp);
+	if (fp) {
+		fdremove(fdp, indx);
+		closef(fp, p);
+		if (vp != NULL)
+			vput(vp);
+	}
+	fdpunlock(fdp);
 	return (error);
 }
 
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index 32fd3922538..d9471e74c56 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: filedesc.h,v 1.16 2003/06/02 23:28:21 millert Exp $	*/
+/*	$OpenBSD: filedesc.h,v 1.17 2004/01/06 04:18:18 tedu Exp $	*/
 /*	$NetBSD: filedesc.h,v 1.14 1996/04/09 20:55:28 cgd Exp $	*/
 
 /*
@@ -32,7 +32,7 @@
  *	@(#)filedesc.h	8.1 (Berkeley) 6/2/93
  */
 
-#include <sys/lock.h>
+#include <sys/rwlock.h>
 /*
  * This structure is used for the management of descriptors.  It may be
  * shared by multiple processes.
@@ -68,7 +68,7 @@ struct filedesc {
 	int	fd_freefile;		/* approx. next free file */
 	u_short	fd_cmask;		/* mask for file creation */
 	u_short	fd_refcnt;		/* reference count */
-	struct lock fd_lock;		/* lock for growing the structure */
+	struct rwlock fd_lock;		/* lock for the file descs */
 
 	int	fd_knlistsize;		/* size of knlist */
 	struct	klist *fd_knlist;	/* list of attached knotes */
@@ -127,4 +127,7 @@ struct file *fd_getfile(struct filedesc *, int fd);
 
 int	closef(struct file *, struct proc *);
 int	getsock(struct filedesc *, int, struct file **);
+
+#define	fdplock(fdp, p)	rw_enter_write(&(fdp)->fd_lock, p)
+#define	fdpunlock(fdp)	rw_exit_write(&(fdp)->fd_lock)
 #endif