Date: Wed, 8 Feb 2006 16:27:32 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 91395 for review Message-ID: <200602081627.k18GRWGX001821@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200602081627.k18GRWGX001821>