From owner-p4-projects@FreeBSD.ORG Sun Jan 18 09:33:20 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 27A361065695; Sun, 18 Jan 2009 09:33:20 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D39381065693 for ; Sun, 18 Jan 2009 09:33:19 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id B6B5C8FC08 for ; Sun, 18 Jan 2009 09:33:19 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n0I9XJuh036841 for ; Sun, 18 Jan 2009 09:33:19 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n0I9XJb1036839 for perforce@freebsd.org; Sun, 18 Jan 2009 09:33:19 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Sun, 18 Jan 2009 09:33:19 GMT Message-Id: <200901180933.n0I9XJb1036839@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Cc: Subject: PERFORCE change 156317 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Jan 2009 09:33:22 -0000 http://perforce.freebsd.org/chv.cgi?CH=156317 Change 156317 by rwatson@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);