From owner-p4-projects@FreeBSD.ORG Wed Feb 8 16:27:33 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 63E0916A423; Wed, 8 Feb 2006 16:27:33 +0000 (GMT) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0B28816A420 for ; Wed, 8 Feb 2006 16:27:33 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id B9DBD43D46 for ; Wed, 8 Feb 2006 16:27:32 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.1/8.13.1) with ESMTP id k18GRW6b001824 for ; Wed, 8 Feb 2006 16:27:32 GMT (envelope-from jhb@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.1/8.13.1/Submit) id k18GRWGX001821 for perforce@freebsd.org; Wed, 8 Feb 2006 16:27:32 GMT (envelope-from jhb@freebsd.org) Date: Wed, 8 Feb 2006 16:27:32 GMT Message-Id: <200602081627.k18GRWGX001821@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jhb@freebsd.org using -f From: John Baldwin To: Perforce Change Reviews Cc: Subject: PERFORCE change 91395 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: Wed, 08 Feb 2006 16:27:34 -0000 http://perforce.freebsd.org/chv.cgi?CH=91395 Change 91395 by jhb@jhb_slimer on 2006/02/08 16:26:57 Allow the caller of pfs_visible to pass in a pointer to a proc pointer. If they do and a process is found, return with the process locked. This closes some races. Also, treat processes with P_WEXIT set as being non-visible. Affected files ... .. //depot/projects/smpng/sys/fs/pseudofs/pseudofs_vnops.c#40 edit Differences ... ==== //depot/projects/smpng/sys/fs/pseudofs/pseudofs_vnops.c#40 (text+ko) ==== @@ -81,13 +81,14 @@ #endif /* - * Returns non-zero if given file is visible to given process + * Returns non-zero if given file is visible to given process. If the 'p' + * parameter is non-NULL, then it will hold a pointer to the process the + * given file belongs to on return and the process will be locked. */ static int -pfs_visible(struct thread *td, struct pfs_node *pn, pid_t pid) +pfs_visible(struct thread *td, struct pfs_node *pn, pid_t pid, struct proc **p) { struct proc *proc; - int r; PFS_TRACE(("%s (pid: %d, req: %d)", pn->pn_name, pid, td->td_proc->p_pid)); @@ -95,20 +96,27 @@ if (pn->pn_flags & PFS_DISABLED) PFS_RETURN (0); - r = 1; if (pid != NO_PID) { if ((proc = pfind(pid)) == NULL) PFS_RETURN (0); + if (proc->p_flag & P_WEXIT) { + PROC_UNLOCK(proc); + PFS_RETURN (0); + } if (p_cansee(td, proc) != 0 || - (pn->pn_vis != NULL && !(pn->pn_vis)(td, proc, pn))) - r = 0; - /* - * XXX: We might should return with the proc locked to - * avoid some races. - */ - PROC_UNLOCK(proc); - } - PFS_RETURN (r); + (pn->pn_vis != NULL && !(pn->pn_vis)(td, proc, pn))) { + PROC_UNLOCK(proc); + PFS_RETURN (0); + } + if (p) { + /* We return with the process locked to avoid races. */ + *p = proc; + } else + PROC_UNLOCK(proc); + } else + if (p) + *p = NULL; + PFS_RETURN (1); } /* @@ -180,8 +188,9 @@ PFS_TRACE((pn->pn_name)); - if (!pfs_visible(curthread, pn, pvd->pvd_pid)) + if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc)) PFS_RETURN (ENOENT); + VATTR_NULL(vap); vap->va_type = vn->v_type; @@ -210,9 +219,7 @@ break; } - if (pvd->pvd_pid != NO_PID) { - if ((proc = pfind(pvd->pvd_pid)) == NULL) - PFS_RETURN (ENOENT); + if (proc != NULL) { vap->va_uid = proc->p_ucred->cr_ruid; vap->va_gid = proc->p_ucred->cr_rgid; if (pn->pn_attr != NULL) @@ -235,7 +242,7 @@ struct vnode *vn = va->a_vp; struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data; struct pfs_node *pn = pvd->pvd_pn; - struct proc *proc = NULL; + struct proc *proc; int error; PFS_TRACE(("%s: %lx", pn->pn_name, va->a_command)); @@ -250,13 +257,10 @@ * This is necessary because process' privileges may * have changed since the open() call. */ - if (!pfs_visible(curthread, pn, pvd->pvd_pid)) + if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc)) PFS_RETURN (EIO); - /* XXX duplicates bits of pfs_visible() */ - if (pvd->pvd_pid != NO_PID) { - if ((proc = pfind(pvd->pvd_pid)) == NULL) - PFS_RETURN (EIO); + if (proc != NULL) { _PHOLD(proc); PROC_UNLOCK(proc); } @@ -278,13 +282,16 @@ struct vnode *vn = va->a_vp; struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data; struct pfs_node *pn = pvd->pvd_pn; - struct proc *proc = NULL; + struct proc *proc; int error; PFS_TRACE((pn->pn_name)); +#if 0 + /* Umm, no need to call this twice. */ if (!pfs_visible(curthread, pn, pvd->pvd_pid)) PFS_RETURN (ENOENT); +#endif if (pn->pn_getextattr == NULL) PFS_RETURN (EOPNOTSUPP); @@ -293,13 +300,10 @@ * This is necessary because either process' privileges may * have changed since the open() call. */ - if (!pfs_visible(curthread, pn, pvd->pvd_pid)) + if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc)) PFS_RETURN (EIO); - /* XXX duplicates bits of pfs_visible() */ - if (pvd->pvd_pid != NO_PID) { - if ((proc = pfind(pvd->pvd_pid)) == NULL) - PFS_RETURN (EIO); + if (proc != NULL) { _PHOLD(proc); PROC_UNLOCK(proc); } @@ -351,8 +355,8 @@ if (cnp->cn_namelen >= PFS_NAMELEN) PFS_RETURN (ENOENT); - /* check that parent directory is visisble... */ - if (!pfs_visible(curthread, pd, pvd->pvd_pid)) + /* check that parent directory is visible... */ + if (!pfs_visible(curthread, pd, pvd->pvd_pid, NULL)) PFS_RETURN (ENOENT); /* self */ @@ -408,7 +412,7 @@ got_pnode: if (pn != pd->pn_parent && !pn->pn_parent) pn->pn_parent = pd; - if (!pfs_visible(curthread, pn, pvd->pvd_pid)) { + if (!pfs_visible(curthread, pn, pvd->pvd_pid, NULL)) { error = ENOENT; goto failed; } @@ -451,7 +455,7 @@ * XXX and the only consequence of that race is an EIO further * XXX down the line. */ - if (!pfs_visible(va->a_td, pn, pvd->pvd_pid)) + if (!pfs_visible(va->a_td, pn, pvd->pvd_pid, NULL)) PFS_RETURN (ENOENT); /* check if the requested mode is permitted */ @@ -476,7 +480,7 @@ struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data; struct pfs_node *pn = pvd->pvd_pn; struct uio *uio = va->a_uio; - struct proc *proc = NULL; + struct proc *proc; struct sbuf *sb = NULL; int error; unsigned int buflen, offset, resid; @@ -496,13 +500,10 @@ * This is necessary because either process' privileges may * have changed since the open() call. */ - if (!pfs_visible(curthread, pn, pvd->pvd_pid)) + if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc)) PFS_RETURN (EIO); - /* XXX duplicates bits of pfs_visible() */ - if (pvd->pvd_pid != NO_PID) { - if ((proc = pfind(pvd->pvd_pid)) == NULL) - PFS_RETURN (EIO); + if (proc != NULL) { _PHOLD(proc); PROC_UNLOCK(proc); } @@ -558,7 +559,7 @@ pfs_iterate(struct thread *td, pid_t pid, struct pfs_node *pd, struct pfs_node **pn, struct proc **p) { - sx_assert(&allproc_lock, SX_LOCKED); + sx_assert(&allproc_lock, SX_SLOCKED); again: if (*pn == NULL) { /* first node */ @@ -581,7 +582,7 @@ if ((*pn) == NULL) return (-1); - if (!pfs_visible(td, *pn, *p ? (*p)->p_pid : pid)) + if (!pfs_visible(td, *pn, *p ? (*p)->p_pid : pid, NULL)) goto again; return (0); @@ -613,7 +614,7 @@ uio = va->a_uio; /* check if the directory is visible to the caller */ - if (!pfs_visible(curthread, pd, pid)) + if (!pfs_visible(curthread, pd, pid, NULL)) PFS_RETURN (ENOENT); /* only allow reading entire entries */ @@ -713,6 +714,10 @@ if (pvd->pvd_pid != NO_PID) { if ((proc = pfind(pvd->pvd_pid)) == NULL) PFS_RETURN (EIO); + if (proc->p_flag & P_WEXIT) { + PROC_UNLOCK(proc); + PFS_RETURN (EIO); + } _PHOLD(proc); PROC_UNLOCK(proc); } @@ -768,7 +773,7 @@ struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data; struct pfs_node *pn = pvd->pvd_pn; struct uio *uio = va->a_uio; - struct proc *proc = NULL; + struct proc *proc; struct sbuf sb; int error; @@ -787,13 +792,10 @@ * This is necessary because either process' privileges may * have changed since the open() call. */ - if (!pfs_visible(curthread, pn, pvd->pvd_pid)) + if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc)) PFS_RETURN (EIO); - /* XXX duplicates bits of pfs_visible() */ - if (pvd->pvd_pid != NO_PID) { - if ((proc = pfind(pvd->pvd_pid)) == NULL) - PFS_RETURN (EIO); + if (proc != NULL) { _PHOLD(proc); PROC_UNLOCK(proc); }