Date: Sun, 18 Jan 2009 10:43:31 GMT From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 156320 for review Message-ID: <200901181043.n0IAhVX7042995@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=156320 Change 156320 by rwatson@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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200901181043.n0IAhVX7042995>