This patchset fixes a large number of file descriptor race conditions.

    Pre-rfork code assumed inherent locking of a process's file descriptor
    array.  However, with the advent of rfork() the file descriptor table
    could be shared between processes.  This patch closes over a dozen
    serious race conditions related to one thread manipulating the table
    (e.g. closing or dup()ing a descriptor) while another is blocked in
    an open(), close(), fcntl(), read(), write(), etc...

PR: kern/11629
Discussed with: Alexander Viro <viro@math.psu.edu>

1 files changed, 37 insertions, 15 deletions

diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c
index 43d75a5..6328ff7 100644
--- a/sys/vm/vm_mmap.c
+++ b/sys/vm/vm_mmap.c
@@ -188,7 +188,7 @@ mmap(p, uap)
 	register struct mmap_args *uap;
 {
 	register struct filedesc *fdp = p->p_fd;
-	register struct file *fp;
+	register struct file *fp = NULL;
 	struct vnode *vp;
 	vm_offset_t addr;
 	vm_size_t size, pageoff;
@@ -283,6 +283,12 @@ mmap(p, uap)
 			return (EBADF);
 		if (fp->f_type != DTYPE_VNODE)
 			return (EINVAL);
+
+		/*
+		 * don't let the descriptor disappear on us if we block
+		 */
+		fhold(fp);
+
 		/*
 		 * POSIX shared-memory objects are defined to have
 		 * kernel persistence, and are not defined to support
@@ -332,8 +338,10 @@ mmap(p, uap)
 			else
 				disablexworkaround = suser(p);
 			if (vp->v_type == VCHR && disablexworkaround &&
-				(flags & (MAP_PRIVATE|MAP_COPY)))
-				 return (EINVAL);
+			    (flags & (MAP_PRIVATE|MAP_COPY))) {
+				error = EINVAL;
+				goto done;
+			}
 			/*
 			 * Ensure that file and memory protections are
 			 * compatible.  Note that we only worry about
@@ -344,10 +352,12 @@ mmap(p, uap)
 			 * proc does a setuid?
 			 */
 			maxprot = VM_PROT_EXECUTE;	/* ??? */
-			if (fp->f_flag & FREAD)
+			if (fp->f_flag & FREAD) {
 				maxprot |= VM_PROT_READ;
-			else if (prot & PROT_READ)
-				return (EACCES);
+			} else if (prot & PROT_READ) {
+				error = EACCES;
+				goto done;
+			}
 			/*
 			 * If we are sharing potential changes (either via
 			 * MAP_SHARED or via the implicit sharing of character
@@ -364,17 +374,23 @@ mmap(p, uap)
 					struct vattr va;
 					if ((error =
 					    VOP_GETATTR(vp, &va,
-						        p->p_ucred, p)))
-						return (error);
+						        p->p_ucred, p))) {
+						goto done;
+					}
 					if ((va.va_flags &
-					   (SF_SNAPSHOT|IMMUTABLE|APPEND)) == 0)
+					   (SF_SNAPSHOT|IMMUTABLE|APPEND)) == 0) {
 						maxprot |= VM_PROT_WRITE;
-					else if (prot & PROT_WRITE)
-						return (EPERM);
-				} else if ((prot & PROT_WRITE) != 0)
-					return (EACCES);
-			} else
+					} else if (prot & PROT_WRITE) {
+						error = EPERM;
+						goto done;
+					}
+				} else if ((prot & PROT_WRITE) != 0) {
+					error = EACCES;
+					goto done;
+				}
+			} else {
 				maxprot |= VM_PROT_WRITE;
+			}
 
 			handle = (void *)vp;
 		}
@@ -387,13 +403,17 @@ mmap(p, uap)
 	 */
 	if (max_proc_mmap && 
 	    vms->vm_map.nentries >= max_proc_mmap * vms->vm_refcnt) {
-		return (ENOMEM);
+		error = ENOMEM;
+		goto done;
 	}
 
 	error = vm_mmap(&vms->vm_map, &addr, size, prot, maxprot,
 	    flags, handle, pos);
 	if (error == 0)
 		p->p_retval[0] = (register_t) (addr + pageoff);
+done:
+	if (fp)
+		fdrop(fp, p);
 	return (error);
 }
 
@@ -576,6 +596,7 @@ munmap(p, uap)
 	return (0);
 }
 
+#if 0
 void
 munmapfd(p, fd)
 	struct proc *p;
@@ -586,6 +607,7 @@ munmapfd(p, fd)
 	 */
 	p->p_fd->fd_ofileflags[fd] &= ~UF_MAPPED;
 }
+#endif
 
 #ifndef _SYS_SYSPROTO_H_
 struct mprotect_args {