From e24b948e7688956f2d1894c08ea3246b660d7c38 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Tue, 16 Mar 2004 17:53:53 +0000 Subject: [PATCH 1/1] Separate chroot() into kern_chroot(). Rename change_dir() to checkvp_chdir() and reorganize the code to avoid doing weird things to the passed vnode's lock and ref count in deep subroutines (which lead to buggy code). Fix a bug in chdir()/kern_chdir() (the namei data was not being freed in all cases), and also fix a bug in symlink() (missing zfree in error case). Submitted-by: Paul Herman Additional-work-by: dillon --- sys/kern/vfs_syscalls.c | 123 ++++++++++++++++++++++++---------------- sys/sys/kern_syscall.h | 3 +- sys/sys/namei.h | 4 +- 3 files changed, 77 insertions(+), 53 deletions(-) diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 104a2ddcb8..f5df25bfc0 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -37,7 +37,7 @@ * * @(#)vfs_syscalls.c 8.13 (Berkeley) 4/15/94 * $FreeBSD: src/sys/kern/vfs_syscalls.c,v 1.151.2.18 2003/04/04 20:35:58 tegge Exp $ - * $DragonFly: src/sys/kern/vfs_syscalls.c,v 1.29 2004/03/01 06:33:17 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_syscalls.c,v 1.30 2004/03/16 17:53:53 dillon Exp $ */ #include @@ -71,7 +71,7 @@ #include -static int change_dir (struct nameidata *ndp, struct thread *td); +static int checkvp_chdir (struct vnode *vn, struct thread *td); static void checkdirs (struct vnode *olddp); static int chroot_refuse_vdir_fds (struct filedesc *fdp); static int getutimes (const struct timeval *, struct timespec *); @@ -790,13 +790,15 @@ kern_chdir(struct nameidata *nd) struct filedesc *fdp = p->p_fd; int error; - error = change_dir(nd, td); - if (error) + if ((error = namei(nd)) != 0) return (error); - NDFREE(nd, NDF_ONLY_PNBUF); - vrele(fdp->fd_cdir); - fdp->fd_cdir = nd->ni_vp; - return (0); + if ((error = checkvp_chdir(nd->ni_vp, td)) == 0) { + vrele(fdp->fd_cdir); + fdp->fd_cdir = nd->ni_vp; + VREF(fdp->fd_cdir); + } + NDFREE(nd, ~(NDF_NO_FREE_PNBUF | NDF_NO_VP_PUT)); + return (error); } /* @@ -857,6 +859,51 @@ static int chroot_allow_open_directories = 1; SYSCTL_INT(_kern, OID_AUTO, chroot_allow_open_directories, CTLFLAG_RW, &chroot_allow_open_directories, 0, ""); +/* + * Chroot to the specified vnode. vp must be locked and referenced on + * call, and will be left locked and referenced on return. This routine + * may acquire additional refs on the vnode when associating it with + * the process's root and/or jail dirs. + */ +int +kern_chroot(struct vnode *vp) +{ + struct thread *td = curthread; + struct proc *p = td->td_proc; + struct filedesc *fdp = p->p_fd; + int error; + + /* + * Only root can chroot + */ + if ((error = suser_cred(p->p_ucred, PRISON_ROOT)) != 0) + return (error); + + /* + * Disallow open directory descriptors (fchdir() breakouts). + */ + if (chroot_allow_open_directories == 0 || + (chroot_allow_open_directories == 1 && fdp->fd_rdir != rootvnode)) { + if ((error = chroot_refuse_vdir_fds(fdp)) != 0) + return (error); + } + + /* + * Check the validity of vp as a directory to change to and + * associate it with rdir/jdir. + */ + if ((error = checkvp_chdir(vp, td)) == 0) { + vrele(fdp->fd_rdir); + fdp->fd_rdir = vp; + VREF(fdp->fd_rdir); + if (fdp->fd_jdir == NULL) { + fdp->fd_jdir = vp; + VREF(fdp->fd_jdir); + } + } + return (error); +} + /* * chroot_args(char *path) * @@ -867,55 +914,33 @@ int chroot(struct chroot_args *uap) { struct thread *td = curthread; - struct proc *p = td->td_proc; - struct filedesc *fdp = p->p_fd; - int error; struct nameidata nd; + int error; - KKASSERT(p); - error = suser_cred(p->p_ucred, PRISON_ROOT); - if (error) - return (error); - if (chroot_allow_open_directories == 0 || - (chroot_allow_open_directories == 1 && fdp->fd_rdir != rootvnode)) - error = chroot_refuse_vdir_fds(fdp); - if (error) - return (error); + KKASSERT(td->td_proc); NDINIT(&nd, NAMEI_LOOKUP, CNP_FOLLOW | CNP_LOCKLEAF, UIO_USERSPACE, - SCARG(uap, path), td); - if ((error = change_dir(&nd, td)) != 0) - return (error); - NDFREE(&nd, NDF_ONLY_PNBUF); - vrele(fdp->fd_rdir); - fdp->fd_rdir = nd.ni_vp; - if (!fdp->fd_jdir) { - fdp->fd_jdir = nd.ni_vp; - VREF(fdp->fd_jdir); + SCARG(uap, path), td); + if ((error = namei(&nd)) == 0) { + error = kern_chroot(nd.ni_vp); + NDFREE(&nd, ~(NDF_NO_FREE_PNBUF | NDF_NO_VP_PUT)); } - return (0); + return (error); } /* - * Common routine for chroot and chdir. + * Common routine for chroot and chdir. Given a locked, referenced vnode, + * determine whether it is legal to chdir to the vnode. The vnode's state + * is not changed by this call. */ -static int -change_dir(struct nameidata *ndp, struct thread *td) +int +checkvp_chdir(struct vnode *vp, struct thread *td) { - struct vnode *vp; int error; - error = namei(ndp); - if (error) - return (error); - vp = ndp->ni_vp; if (vp->v_type != VDIR) error = ENOTDIR; else - error = VOP_ACCESS(vp, VEXEC, ndp->ni_cnd.cn_cred, td); - if (error) - vput(vp); - else - VOP_UNLOCK(vp, NULL, 0, td); + error = VOP_ACCESS(vp, VEXEC, td->td_proc->p_ucred, td); return (error); } @@ -1337,13 +1362,11 @@ symlink(struct symlink_args *uap) path = zalloc(namei_zone); error = copyinstr(uap->path, path, MAXPATHLEN, NULL); - if (error) - return (error); - NDINIT(&nd, NAMEI_CREATE, CNP_LOCKPARENT | CNP_NOOBJ, UIO_USERSPACE, - uap->link, td); - - error = kern_symlink(path, &nd); - + if (error == 0) { + NDINIT(&nd, NAMEI_CREATE, CNP_LOCKPARENT | CNP_NOOBJ, + UIO_USERSPACE, uap->link, td); + error = kern_symlink(path, &nd); + } zfree(namei_zone, path); return (error); } diff --git a/sys/sys/kern_syscall.h b/sys/sys/kern_syscall.h index a2db31a1a1..72d05a7641 100644 --- a/sys/sys/kern_syscall.h +++ b/sys/sys/kern_syscall.h @@ -25,7 +25,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/sys/kern_syscall.h,v 1.14 2003/11/14 20:54:07 daver Exp $ + * $DragonFly: src/sys/sys/kern_syscall.h,v 1.15 2004/03/16 17:53:51 dillon Exp $ */ #ifndef _SYS_KERN_SYSCALL_H_ @@ -120,6 +120,7 @@ int kern_access(struct nameidata *nd, int aflags); int kern_chdir(struct nameidata *nd); int kern_chmod(struct nameidata *nd, int mode); int kern_chown(struct nameidata *nd, int uid, int gid); +int kern_chroot(struct vnode *vp); int kern_fstatfs(int fd, struct statfs *buf); int kern_ftruncate(int fd, off_t length); int kern_futimes(int fd, struct timeval *tptr); diff --git a/sys/sys/namei.h b/sys/sys/namei.h index 76ac1c793b..dda7540405 100644 --- a/sys/sys/namei.h +++ b/sys/sys/namei.h @@ -32,7 +32,7 @@ * * @(#)namei.h 8.5 (Berkeley) 1/9/95 * $FreeBSD: src/sys/sys/namei.h,v 1.29.2.2 2001/09/30 21:12:54 luigi Exp $ - * $DragonFly: src/sys/sys/namei.h,v 1.9 2003/10/13 21:16:10 dillon Exp $ + * $DragonFly: src/sys/sys/namei.h,v 1.10 2004/03/16 17:53:51 dillon Exp $ */ #ifndef _SYS_NAMEI_H_ @@ -195,7 +195,7 @@ NDINIT2(struct nameidata *ndp, u_long op, u_long flags, enum uio_seg segflg, #define NDF_NO_DVP_PUT 0x00000003 #define NDF_NO_VP_RELE 0x00000004 #define NDF_NO_VP_UNLOCK 0x00000008 -#define NDF_NO_VP_PUT 0x0000000c +#define NDF_NO_VP_PUT (NDF_NO_VP_RELE|NDF_NO_VP_UNLOCK) #define NDF_NO_STARTDIR_RELE 0x00000010 #define NDF_NO_FREE_PNBUF 0x00000020 #define NDF_NO_DNCP_RELE 0x00000040 -- 2.41.0