Hold struct filedesc's fd_lock when writing to the fd_ofiles, fd_ofileflags,

or fd_{lo,hi}maps members, or when doing a read for a write.  Fixes hangs
when an rthreaded processes sleeps while copying the fd table for fork()
and catches another thread with the lock.

ok jsing@ tedu@

8 files changed, 65 insertions, 24 deletions

diff --git a/sys/crypto/cryptodev.c b/sys/crypto/cryptodev.c
index d502f3dde9c..a6fd1be0ccb 100644
--- a/sys/crypto/cryptodev.c
+++ b/sys/crypto/cryptodev.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cryptodev.c,v 1.77 2011/01/11 16:06:40 deraadt Exp $	*/
+/*	$OpenBSD: cryptodev.c,v 1.78 2012/02/15 04:26:27 guenther Exp $	*/
 
 /*
  * Copyright (c) 2001 Theo de Raadt
@@ -34,6 +34,7 @@
 #include <sys/systm.h>
 #include <sys/malloc.h>
 #include <sys/mbuf.h>
+#include <sys/proc.h>
 #include <sys/file.h>
 #include <sys/filedesc.h>
 #include <sys/errno.h>
@@ -666,7 +667,9 @@ cryptoioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p)
 		TAILQ_INIT(&fcr->csessions);
 		fcr->sesn = 0;
 
+		fdplock(p->p_fd);
 		error = falloc(p, &f, &fd);
+		fdpunlock(p->p_fd);
 		if (error) {
 			free(fcr, M_XDATA);
 			return (error);
diff --git a/sys/dev/systrace.c b/sys/dev/systrace.c
index 09147c791a8..f48ffdec652 100644
--- a/sys/dev/systrace.c
+++ b/sys/dev/systrace.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: systrace.c,v 1.60 2011/09/18 23:24:14 matthew Exp $	*/
+/*	$OpenBSD: systrace.c,v 1.61 2012/02/15 04:26:27 guenther Exp $	*/
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * All rights reserved.
@@ -527,7 +527,9 @@ systraceioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p)
 		fst->p_ruid = p->p_cred->p_ruid;
 		fst->p_rgid = p->p_cred->p_rgid;
 
+		fdplock(p->p_fd);
 		error = falloc(p, &f, &fd);
+		fdpunlock(p->p_fd);
 		if (error) {
 			free(fst, M_XDATA);
 			return (error);
diff --git a/sys/kern/exec_script.c b/sys/kern/exec_script.c
index dc1cc381b84..a6d730f8ece 100644
--- a/sys/kern/exec_script.c
+++ b/sys/kern/exec_script.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: exec_script.c,v 1.26 2011/07/07 23:45:00 matthew Exp $	*/
+/*	$OpenBSD: exec_script.c,v 1.27 2012/02/15 04:26:27 guenther Exp $	*/
 /*	$NetBSD: exec_script.c,v 1.13 1996/02/04 02:15:06 christos Exp $	*/
 
 /*
@@ -187,7 +187,10 @@ check_shell:
 			panic("exec_script_makecmds: epp already has a fd");
 #endif
 
-		if ((error = falloc(p, &fp, &epp->ep_fd)))
+		fdplock(p->p_fd);
+		error = falloc(p, &fp, &epp->ep_fd);
+		fdpunlock(p->p_fd);
+		if (error)
 			goto fail;
 
 		epp->ep_flags |= EXEC_HASFD;
@@ -298,7 +301,9 @@ fail:
 	/* kill the opened file descriptor, else close the file */
 	if (epp->ep_flags & EXEC_HASFD) {
 		epp->ep_flags &= ~EXEC_HASFD;
+		fdplock(p->p_fd);
 		(void) fdrelease(p, epp->ep_fd);
+		fdpunlock(p->p_fd);
 	} else
 		vn_close(scriptvp, FREAD, p->p_ucred, p);
 
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index cb4b4143361..519074b6340 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.88 2011/07/08 21:26:27 matthew Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.89 2012/02/15 04:26:27 guenther Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -322,21 +322,19 @@ restart:
 		}
 		fdplock(fdp);
 		if ((error = fdalloc(p, newmin, &i)) != 0) {
+			FRELE(fp);
 			if (error == ENOSPC) {
 				fdexpand(p);
-				FRELE(fp);
 				fdpunlock(fdp);
 				goto restart;
 			}
-		}
-		/* finishdup will FRELE for us. */
-		if (!error)
+		} else {
+			/* finishdup will FRELE for us. */
 			error = finishdup(p, fp, fd, i, retval);
-		else
-			FRELE(fp);
 
-		if (!error && SCARG(uap, cmd) == F_DUPFD_CLOEXEC)
-			fdp->fd_ofileflags[i] |= UF_EXCLOSE;
+			if (!error && SCARG(uap, cmd) == F_DUPFD_CLOEXEC)
+				fdp->fd_ofileflags[i] |= UF_EXCLOSE;
+		}
 
 		fdpunlock(fdp);
 		return (error);
@@ -346,10 +344,12 @@ restart:
 		break;
 
 	case F_SETFD:
+		fdplock(fdp);
 		if ((long)SCARG(uap, arg) & 1)
 			fdp->fd_ofileflags[fd] |= UF_EXCLOSE;
 		else
 			fdp->fd_ofileflags[fd] &= ~UF_EXCLOSE;
+		fdpunlock(fdp);
 		break;
 
 	case F_GETFL:
@@ -523,6 +523,7 @@ finishdup(struct proc *p, struct file *fp, int old, int new, register_t *retval)
 	struct file *oldfp;
 	struct filedesc *fdp = p->p_fd;
 
+	fdpassertlocked(fdp);
 	if (fp->f_count == LONG_MAX-2) {
 		FRELE(fp);
 		return (EDEADLK);
@@ -556,6 +557,7 @@ finishdup(struct proc *p, struct file *fp, int old, int new, register_t *retval)
 void
 fdremove(struct filedesc *fdp, int fd)
 {
+	fdpassertlocked(fdp);
 	fdp->fd_ofiles[fd] = NULL;
 	fd_unused(fdp, fd);
 }
@@ -566,6 +568,8 @@ fdrelease(struct proc *p, int fd)
 	struct filedesc *fdp = p->p_fd;
 	struct file **fpp, *fp;
 
+	fdpassertlocked(fdp);
+
 	/*
 	 * Don't fd_getfile here. We want to closef LARVAL files and closef
 	 * can deal with that.
@@ -749,6 +753,8 @@ fdexpand(struct proc *p)
 	char *newofileflags;
 	u_int *newhimap, *newlomap;
 
+	fdpassertlocked(fdp);
+
 	/*
 	 * No space in current array.
 	 */
@@ -812,6 +818,7 @@ falloc(struct proc *p, struct file **resultfp, int *resultfd)
 	struct file *fp, *fq;
 	int error, i;
 
+	fdpassertlocked(p->p_fd);
 restart:
 	if ((error = fdalloc(p, 0, &i)) != 0) {
 		if (error == ENOSPC) {
@@ -908,6 +915,7 @@ fdcopy(struct proc *p)
 	struct file **fpp;
 	int i;
 
+	fdplock(fdp);
 	newfdp = pool_get(&fdesc_pool, PR_WAITOK);
 	bcopy(fdp, newfdp, sizeof(struct filedesc));
 	if (newfdp->fd_cdir)
@@ -915,6 +923,7 @@ fdcopy(struct proc *p)
 	if (newfdp->fd_rdir)
 		vref(newfdp->fd_rdir);
 	newfdp->fd_refcnt = 1;
+	rw_init(&newfdp->fd_lock, "fdlock");
 
 	/*
 	 * If the number of open files fits in the internal arrays
@@ -955,10 +964,12 @@ fdcopy(struct proc *p)
 	bcopy(fdp->fd_ofileflags, newfdp->fd_ofileflags, i * sizeof(char));
 	bcopy(fdp->fd_himap, newfdp->fd_himap, NDHISLOTS(i) * sizeof(u_int));
 	bcopy(fdp->fd_lomap, newfdp->fd_lomap, NDLOSLOTS(i) * sizeof(u_int));
+	fdpunlock(fdp);
 
 	/*
 	 * kq descriptors cannot be copied.
 	 */
+	fdplock(newfdp);
 	if (newfdp->fd_knlistsize != -1) {
 		fpp = newfdp->fd_ofiles;
 		for (i = 0; i <= newfdp->fd_lastfile; i++, fpp++)
@@ -983,6 +994,7 @@ fdcopy(struct proc *p)
 			else
 				(*fpp)->f_count++;
 		}
+	fdpunlock(newfdp);
 	return (newfdp);
 }
 
@@ -1197,6 +1209,8 @@ dupfdopen(struct filedesc *fdp, int indx, int dfd, int mode, int error)
 {
 	struct file *wfp;
 
+	fdpassertlocked(fdp);
+
 	/*
 	 * Assume that the filename was user-specified; applications do
 	 * not tend to open /dev/fd/# when they can just call dup()
@@ -1277,9 +1291,11 @@ fdcloseexec(struct proc *p)
 	struct filedesc *fdp = p->p_fd;
 	int fd;
 
+	fdplock(fdp);
 	for (fd = 0; fd <= fdp->fd_lastfile; fd++)
 		if (fdp->fd_ofileflags[fd] & UF_EXCLOSE)
 			(void) fdrelease(p, fd);
+	fdpunlock(fdp);
 }
 
 int
diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index d43c013d4c2..d247058b9b0 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_event.c,v 1.41 2011/07/02 22:20:08 nicm Exp $	*/
+/*	$OpenBSD: kern_event.c,v 1.42 2012/02/15 04:26:27 guenther Exp $	*/
 
 /*-
  * Copyright (c) 1999,2000,2001 Jonathan Lemon <jlemon@FreeBSD.org>
@@ -423,7 +423,9 @@ sys_kqueue(struct proc *p, void *v, register_t *retval)
 	struct file *fp;
 	int fd, error;
 
+	fdplock(fdp);
 	error = falloc(p, &fp, &fd);
+	fdpunlock(fdp);
 	if (error)
 		return (error);
 	fp->f_flag = FREAD | FWRITE;
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index c72d2fcdead..1b1ac0d84d2 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_exec.c,v 1.122 2011/12/14 07:32:16 guenther Exp $	*/
+/*	$OpenBSD: kern_exec.c,v 1.123 2012/02/15 04:26:27 guenther Exp $	*/
 /*	$NetBSD: kern_exec.c,v 1.75 1996/02/09 18:59:28 christos Exp $	*/
 
 /*-
@@ -529,6 +529,8 @@ sys_execve(struct proc *p, void *v, register_t *retval)
 		 * For set[ug]id processes, a few caveats apply to
 		 * stdin, stdout, and stderr.
 		 */
+		error = 0;
+		fdplock(p->p_fd);
 		for (i = 0; i < 3; i++) {
 			struct file *fp = NULL;
 
@@ -562,7 +564,7 @@ sys_execve(struct proc *p, void *v, register_t *retval)
 				int indx;
 
 				if ((error = falloc(p, &fp, &indx)) != 0)
-					goto exec_abort;
+					break;
 #ifdef DIAGNOSTIC
 				if (indx != i)
 					panic("sys_execve: falloc indx != i");
@@ -570,13 +572,13 @@ sys_execve(struct proc *p, void *v, register_t *retval)
 				if ((error = cdevvp(getnulldev(), &vp)) != 0) {
 					fdremove(p->p_fd, indx);
 					closef(fp, p);
-					goto exec_abort;
+					break;
 				}
 				if ((error = VOP_OPEN(vp, flags, p->p_ucred, p)) != 0) {
 					fdremove(p->p_fd, indx);
 					closef(fp, p);
 					vrele(vp);
-					goto exec_abort;
+					break;
 				}
 				if (flags & FWRITE)
 					vp->v_writecount++;
@@ -587,6 +589,9 @@ sys_execve(struct proc *p, void *v, register_t *retval)
 				FILE_SET_MATURE(fp);
 			}
 		}
+		fdpunlock(p->p_fd);
+		if (error)
+			goto exec_abort;
 	} else
 		atomic_clearbits_int(&pr->ps_flags, PS_SUGID);
 	p->p_cred->p_svuid = p->p_ucred->cr_uid;
@@ -695,7 +700,9 @@ bad:
 	/* kill any opened file descriptor, if necessary */
 	if (pack.ep_flags & EXEC_HASFD) {
 		pack.ep_flags &= ~EXEC_HASFD;
+		fdplock(p->p_fd);
 		(void) fdrelease(p, pack.ep_fd);
+		fdpunlock(p->p_fd);
 	}
 	if (pack.ep_interp != NULL)
 		pool_put(&namei_pool, pack.ep_interp);
diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
index 962645dba50..4fbc127dae0 100644
--- a/sys/kern/sys_generic.c
+++ b/sys/kern/sys_generic.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sys_generic.c,v 1.73 2011/11/06 12:10:04 guenther Exp $	*/
+/*	$OpenBSD: sys_generic.c,v 1.74 2012/02/15 04:26:27 guenther Exp $	*/
 /*	$NetBSD: sys_generic.c,v 1.24 1996/03/29 00:25:32 cgd Exp $	*/
 
 /*
@@ -408,10 +408,13 @@ sys_ioctl(struct proc *p, void *v, register_t *retval)
 
 	switch (com = SCARG(uap, com)) {
 	case FIONCLEX:
-		fdp->fd_ofileflags[SCARG(uap, fd)] &= ~UF_EXCLOSE;
-		return (0);
 	case FIOCLEX:
-		fdp->fd_ofileflags[SCARG(uap, fd)] |= UF_EXCLOSE;
+		fdplock(fdp);
+		if (com == FIONCLEX)
+			fdp->fd_ofileflags[SCARG(uap, fd)] &= ~UF_EXCLOSE;
+		else
+			fdp->fd_ofileflags[SCARG(uap, fd)] |= UF_EXCLOSE;
+		fdpunlock(fdp);
 		return (0);
 	}
 
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index 792129b1cb8..876b3bb6b14 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: filedesc.h,v 1.20 2008/09/19 12:24:55 art Exp $	*/
+/*	$OpenBSD: filedesc.h,v 1.21 2012/02/15 04:26:27 guenther Exp $	*/
 /*	$NetBSD: filedesc.h,v 1.14 1996/04/09 20:55:28 cgd Exp $	*/
 
 /*
@@ -68,7 +68,9 @@ struct filedesc {
 	int	fd_freefile;		/* approx. next free file */
 	u_short	fd_cmask;		/* mask for file creation */
 	u_short	fd_refcnt;		/* reference count */
-	struct rwlock fd_lock;		/* lock for the file descs */
+	struct rwlock fd_lock;		/* lock for the file descs; must be */
+					/* held when writing to fd_ofiles, */
+					/* fd_ofileflags, or fd_{hi,lo}map */
 
 	int	fd_knlistsize;		/* size of knlist */
 	struct	klist *fd_knlist;	/* list of attached knotes */
@@ -137,4 +139,5 @@ int	getsock(struct filedesc *, int, struct file **);
 
 #define	fdplock(fdp)	rw_enter_write(&(fdp)->fd_lock)
 #define	fdpunlock(fdp)	rw_exit_write(&(fdp)->fd_lock)
+#define	fdpassertlocked(fdp)	rw_assert_wrlock(&(fdp)->fd_lock)
 #endif