PERFORCE change 156317 for review

[Robert Watson]

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

Change 156317 by rwatson at rwatson_freebsd_capabilities on 2009/01/18 09:32:53

	Create the process descriptor a bit later so we have to handle
	fewer failure modes.
	
	Always fdrop() unconditionally in the parent process.
	
	fdclose() in the process descriptor in the child since we want the
	parent to have the only reference unless otherwise set up by the
	two processes.

Affected files ...

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

Differences ...

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

@@ -261,21 +261,6 @@
 
 	p1 = td->td_proc;
 
-#ifdef PROCDESC
-	/*
-	 * If required, create a process descriptor in the parent first; we
-	 * will abandon it if something goes wrong.  We don't finit() until
-	 * later.
-	 *
-	 * XXXRW: How best to make it not visible in the child?
-	 */
-	if (flags & RFPROCDESC) {
-		error = falloc(td, &fp_procdesc, &fd_procdesc);
-		if (error)
-			return (error);
-	}
-#endif
-
 	/*
 	 * Here we don't create a new process, but we divorce
 	 * certain parts of a process from itself.
@@ -286,9 +271,6 @@
 			PROC_LOCK(p1);
 			if (thread_single(SINGLE_BOUNDARY)) {
 				PROC_UNLOCK(p1);
-#ifdef PROCDESC
-				fdrop(fp_procdesc, td);
-#endif
 				return (ERESTART);
 			}
 			PROC_UNLOCK(p1);
@@ -322,9 +304,6 @@
 			PROC_UNLOCK(p1);
 		}
 		*procp = NULL;
-#ifdef PROCDESC
-		fdrop(fp_procdesc, td);
-#endif
 		return (error);
 	}
 
@@ -384,7 +363,24 @@
 		goto fail;
 	}
 
+#ifdef PROCDESC
 	/*
+	 * If required, create a process descriptor in the parent first; we
+	 * 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) {
+		error = falloc(td, &fp_procdesc, &fd_procdesc);
+		if (error)
+			goto fail;
+	}
+#endif
+
+	/*
 	 * Increment the count of procs running with this uid. Don't allow
 	 * a nonprivileged user to exceed their current limit.
 	 *
@@ -604,6 +600,15 @@
 	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;
@@ -772,6 +777,7 @@
 	/*
 	 * Associate the process descriptor with the process before anything
 	 * can happen that might cause that process to need the descriptor.
+	 * However, don't do this until after fork(2) can no longer fail.
 	 */
 	if (flags & RFPROCDESC)
 		procdesc_new(p2, fp_procdesc);
@@ -836,8 +842,10 @@
 	 * number, rather than the chid pid, will be returned.
 	 */
 #ifdef PROCDESC
-	if (flags & RFPROCDESC)
+	if (flags & RFPROCDESC) {
 		td->td_retval[0] = fd_procdesc;
+		fdrop(fp_procdesc, td);
+	}
 #endif
 	return (0);
 fail:
@@ -854,7 +862,8 @@
 		vmspace_free(vm2);
 	uma_zfree(proc_zone, newproc);
 #ifdef PROCDESC
-	fdrop(fp_procdesc, td);
+	if (flags & RFPROCDESC)
+		fdrop(fp_procdesc, td);
 #endif
 	pause("fork", hz / 2);
 	return (error);