PERFORCE change 156320 for review

[Robert Watson]

http://perforce.freebsd.org/chv.cgi?CH=156320

Change 156320 by rwatson at rwatson_freebsd_capabilities on 2009/01/18 10:43:07

	Refine RFPROCDESC handling in fork1() a bit more: don't allow it
	to be used unless RFPROC is also used, since we need a process
	for there to be a descriptor.
	
	Close the process descriptor in the child immediately after the 
	fdcopy() so that fdclose() doesn't acquire the filedesc lock while
	the proc lock is held (this makes more sense anyway).
	
	It's possible for the process descriptor to not yet be allocated
	when we reach the fail label, so initialize to NULL initially and
	then check for non-NULL before dropping it on failure.

Affected files ...

.. //depot/projects/trustedbsd/capabilities/src/sys/kern/kern_fork.c#8 edit

Differences ...

==== //depot/projects/trustedbsd/capabilities/src/sys/kern/kern_fork.c#8 (text+ko) ====

@@ -250,7 +250,7 @@
 	struct sigacts *newsigacts;
 	struct vmspace *vm2;
 #ifdef PROCDESC
-	struct file *fp_procdesc;
+	struct file *fp_procdesc = NULL;
 	int fd_procdesc;
 #endif
 	int error;
@@ -259,6 +259,12 @@
 	if ((flags & (RFFDG|RFCFDG)) == (RFFDG|RFCFDG))
 		return (EINVAL);
 
+#ifdef PROCDESC
+	/* Can't not create a process yet get a process descriptor. */
+	if ((flags & RFPROCDESC) && ((flags & RFPROC) == 0))
+		return (EINVAL);
+#endif
+
 	p1 = td->td_proc;
 
 	/*
@@ -369,8 +375,6 @@
 	 * will abandon it if something goes wrong.  We don't finit() until
 	 * later.
 	 *
-	 * XXXRW: How best to make it not visible in the child?
-	 *
 	 * XXXRW: What errno to return?
 	 */
 	if (flags & RFPROCDESC) {
@@ -533,6 +537,16 @@
 	} else if (flags & RFFDG) {
 		fd = fdcopy(p1->p_fd);
 		fdtol = NULL;
+#ifdef PROCDESC
+		/*
+		 * If the file descriptor table is copied, we only want the
+		 * process descriptor to appear in the parent, so close in
+		 * the child.
+		 */
+		if (flags & RFPROCDESC)
+			fdclose(fd, fp_procdesc, fd_procdesc, td);
+#endif
+
 	} else {
 		fd = fdshare(p1->p_fd);
 		if (p1->p_fdtol == NULL)
@@ -600,15 +614,6 @@
 	else
 	        p2->p_sigparent = SIGCHLD;
 
-#ifdef PROCDESC
-	/*
-	 * We want only the parent to have a copy of the child's process
-	 * descriptor, so close in the child.
-	 */
-	if (flags & RFPROCDESC)
-		fdclose(fd, fp_procdesc, fd_procdesc, td);
-#endif
-
 	p2->p_textvp = p1->p_textvp;
 	p2->p_fd = fd;
 	p2->p_fdtol = fdtol;
@@ -862,7 +867,7 @@
 		vmspace_free(vm2);
 	uma_zfree(proc_zone, newproc);
 #ifdef PROCDESC
-	if (flags & RFPROCDESC)
+	if ((flags & RFPROCDESC) && (fp_procdesc != NULL))
 		fdrop(fp_procdesc, td);
 #endif
 	pause("fork", hz / 2);