Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 11 Jul 2015 16:53:03 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Mateusz Guzik <mjguzik@gmail.com>
Cc:        freebsd-fs@freebsd.org
Subject:   Re: [PATCH 1/2] Move chdir/chroot-related fdp manipulation to kern_descrip.c
Message-ID:  <20150711135303.GH2080@kib.kiev.ua>
In-Reply-To: <20150711134021.GA1433@dft-labs.eu>
References:  <1436569684-3939-1-git-send-email-mjguzik@gmail.com> <1436569684-3939-2-git-send-email-mjguzik@gmail.com> <20150711124335.GF2080@kib.kiev.ua> <20150711134021.GA1433@dft-labs.eu>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Jul 11, 2015 at 03:40:21PM +0200, Mateusz Guzik wrote:
> diff --git a/sys/compat/svr4/svr4_misc.c b/sys/compat/svr4/svr4_misc.c
> index ec4504e..5e53874 100644
> --- a/sys/compat/svr4/svr4_misc.c
> +++ b/sys/compat/svr4/svr4_misc.c
> @@ -643,7 +643,7 @@ svr4_sys_fchroot(td, uap)
>  		goto fail;
>  #endif
>  	VOP_UNLOCK(vp, 0);
> -	error = change_root(vp, td);
> +	error = pwd_chroot(td, vp);
>  	vrele(vp);
>  	return (error);
>  fail:
> diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
> index e8f0014..51e8b3c 100644
> --- a/sys/kern/kern_descrip.c
> +++ b/sys/kern/kern_descrip.c
> @@ -2875,6 +2875,96 @@ dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode,
>  }
>  
>  /*
> + * This sysctl determines if we will allow a process to chroot(2) if it
> + * has a directory open:
> + *	0: disallowed for all processes.
> + *	1: allowed for processes that were not already chroot(2)'ed.
> + *	2: allowed for all processes.
> + */
> +
> +static int chroot_allow_open_directories = 1;
> +
> +SYSCTL_INT(_kern, OID_AUTO, chroot_allow_open_directories, CTLFLAG_RW,
> +    &chroot_allow_open_directories, 0,
> +    "Allow a process to chroot(2) if it has a directory open");
> +
> +/*
> + * Helper function for raised chroot(2) security function:  Refuse if
> + * any filedescriptors are open directories.
> + */
> +static int
> +chroot_refuse_vdir_fds(struct filedesc *fdp)
> +{
> +	struct vnode *vp;
> +	struct file *fp;
> +	int fd;
> +
> +	FILEDESC_LOCK_ASSERT(fdp);
> +
> +	for (fd = 0; fd <= fdp->fd_lastfile; fd++) {
> +		fp = fget_locked(fdp, fd);
> +		if (fp == NULL)
> +			continue;
> +		if (fp->f_type == DTYPE_VNODE) {
> +			vp = fp->f_vnode;
> +			if (vp->v_type == VDIR)
> +				return (EPERM);
> +		}
> +	}
> +	return (0);
> +}
> +
> +/*
> + * Common routine for kern_chroot() and jail_attach().  The caller is
> + * responsible for invoking priv_check() and mac_vnode_check_chroot() to
> + * authorize this operation.
> + */
> +int
> +pwd_chroot(struct thread *td, struct vnode *vp)
> +{
> +	struct filedesc *fdp;
> +	struct vnode *oldvp;
> +	int error;
> +
> +	fdp = td->td_proc->p_fd;
> +	FILEDESC_XLOCK(fdp);
> +	if (chroot_allow_open_directories == 0 ||
> +	    (chroot_allow_open_directories == 1 && fdp->fd_rdir != rootvnode)) {
> +		error = chroot_refuse_vdir_fds(fdp);
> +		if (error != 0) {
> +			FILEDESC_XUNLOCK(fdp);
> +			return (error);
> +		}
> +	}
> +	oldvp = fdp->fd_rdir;
> +	VREF(vp);
> +	fdp->fd_rdir = vp;
> +	if (fdp->fd_jdir == NULL) {
> +		VREF(vp);
> +		fdp->fd_jdir = vp;
> +	}
> +	FILEDESC_XUNLOCK(fdp);
> +	vrele(oldvp);
> +	return (0);
> +}
> +
> +void
> +pwd_chdir(struct thread *td, struct vnode *vp)
> +{
> +	struct filedesc *fdp;
> +	struct vnode *oldvp;
> +
> +	fdp = td->td_proc->p_fd;
> +	FILEDESC_XLOCK(fdp);
> +	VNASSERT(vp->v_usecount > 0, vp,
> +	    ("chdir to a vnode with zero usecount"));
> +	oldvp = fdp->fd_cdir;
> +	fdp->fd_cdir = vp;
> +	FILEDESC_XUNLOCK(fdp);
> +	vrele(oldvp);
> +}
> +
> +/*
>   * Scan all active processes and prisons to see if any of them have a current
>   * or root directory of `olddp'. If so, replace them with the new mount point.
>   */
> diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
> index c118d74..6bdddd7 100644
> --- a/sys/kern/kern_jail.c
> +++ b/sys/kern/kern_jail.c
> @@ -2432,7 +2432,7 @@ do_jail_attach(struct thread *td, struct prison *pr)
>  		goto e_unlock;
>  #endif
>  	VOP_UNLOCK(pr->pr_root, 0);
> -	if ((error = change_root(pr->pr_root, td)))
> +	if ((error = pwd_chroot(td, pr->pr_root)))
>  		goto e_revert_osd;
>  
>  	newcred = crget();
> diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
> index 9088017..258a1ef 100644
> --- a/sys/kern/vfs_syscalls.c
> +++ b/sys/kern/vfs_syscalls.c
> @@ -728,8 +728,7 @@ sys_fchdir(td, uap)
>  		int fd;
>  	} */ *uap;
>  {
> -	register struct filedesc *fdp = td->td_proc->p_fd;
> -	struct vnode *vp, *tdp, *vpold;
> +	struct vnode *vp, *tdp;
>  	struct mount *mp;
>  	struct file *fp;
>  	cap_rights_t rights;
> @@ -761,11 +760,7 @@ sys_fchdir(td, uap)
>  		return (error);
>  	}
>  	VOP_UNLOCK(vp, 0);
> -	FILEDESC_XLOCK(fdp);
> -	vpold = fdp->fd_cdir;
> -	fdp->fd_cdir = vp;
> -	FILEDESC_XUNLOCK(fdp);
> -	vrele(vpold);
> +	pwd_chdir(td, vp);
>  	return (0);
>  }
>  
> @@ -791,9 +786,7 @@ sys_chdir(td, uap)
>  int
>  kern_chdir(struct thread *td, char *path, enum uio_seg pathseg)
>  {
> -	register struct filedesc *fdp = td->td_proc->p_fd;
>  	struct nameidata nd;
> -	struct vnode *vp;
>  	int error;
>  
>  	NDINIT(&nd, LOOKUP, FOLLOW | LOCKSHARED | LOCKLEAF | AUDITVNODE1,
> @@ -807,56 +800,11 @@ kern_chdir(struct thread *td, char *path, enum uio_seg pathseg)
>  	}
>  	VOP_UNLOCK(nd.ni_vp, 0);
>  	NDFREE(&nd, NDF_ONLY_PNBUF);
> -	FILEDESC_XLOCK(fdp);
> -	vp = fdp->fd_cdir;
> -	fdp->fd_cdir = nd.ni_vp;
> -	FILEDESC_XUNLOCK(fdp);
> -	vrele(vp);
> -	return (0);
> -}
> -
> -/*
> - * Helper function for raised chroot(2) security function:  Refuse if
> - * any filedescriptors are open directories.
> - */
> -static int
> -chroot_refuse_vdir_fds(fdp)
> -	struct filedesc *fdp;
> -{
> -	struct vnode *vp;
> -	struct file *fp;
> -	int fd;
> -
> -	FILEDESC_LOCK_ASSERT(fdp);
> -
> -	for (fd = 0; fd <= fdp->fd_lastfile; fd++) {
> -		fp = fget_locked(fdp, fd);
> -		if (fp == NULL)
> -			continue;
> -		if (fp->f_type == DTYPE_VNODE) {
> -			vp = fp->f_vnode;
> -			if (vp->v_type == VDIR)
> -				return (EPERM);
> -		}
> -	}
> +	pwd_chdir(td, nd.ni_vp);
>  	return (0);
>  }
>  
>  /*
> - * This sysctl determines if we will allow a process to chroot(2) if it
> - * has a directory open:
> - *	0: disallowed for all processes.
> - *	1: allowed for processes that were not already chroot(2)'ed.
> - *	2: allowed for all processes.
> - */
> -
> -static int chroot_allow_open_directories = 1;
> -
> -SYSCTL_INT(_kern, OID_AUTO, chroot_allow_open_directories, CTLFLAG_RW,
> -     &chroot_allow_open_directories, 0,
> -     "Allow a process to chroot(2) if it has a directory open");
> -
> -/*
>   * Change notion of root (``/'') directory.
>   */
>  #ifndef _SYS_SYSPROTO_H_
> @@ -891,7 +839,7 @@ sys_chroot(td, uap)
>  		goto e_vunlock;
>  #endif
>  	VOP_UNLOCK(nd.ni_vp, 0);
> -	error = change_root(nd.ni_vp, td);
> +	error = pwd_chroot(td, nd.ni_vp);
>  	vrele(nd.ni_vp);
>  	NDFREE(&nd, NDF_ONLY_PNBUF);
>  	return (error);
> @@ -926,42 +874,6 @@ change_dir(vp, td)
>  	return (VOP_ACCESS(vp, VEXEC, td->td_ucred, td));
>  }
>  
> -/*
> - * Common routine for kern_chroot() and jail_attach().  The caller is
> - * responsible for invoking priv_check() and mac_vnode_check_chroot() to
> - * authorize this operation.
> - */
> -int
> -change_root(vp, td)
> -	struct vnode *vp;
> -	struct thread *td;
> -{
> -	struct filedesc *fdp;
> -	struct vnode *oldvp;
> -	int error;
> -
> -	fdp = td->td_proc->p_fd;
> -	FILEDESC_XLOCK(fdp);
> -	if (chroot_allow_open_directories == 0 ||
> -	    (chroot_allow_open_directories == 1 && fdp->fd_rdir != rootvnode)) {
> -		error = chroot_refuse_vdir_fds(fdp);
> -		if (error != 0) {
> -			FILEDESC_XUNLOCK(fdp);
> -			return (error);
> -		}
> -	}
> -	oldvp = fdp->fd_rdir;
> -	fdp->fd_rdir = vp;
> -	VREF(fdp->fd_rdir);
> -	if (!fdp->fd_jdir) {
> -		fdp->fd_jdir = vp;
> -		VREF(fdp->fd_jdir);
> -	}
> -	FILEDESC_XUNLOCK(fdp);
> -	vrele(oldvp);
> -	return (0);
> -}
> -
>  static __inline void
>  flags_to_rights(int flags, cap_rights_t *rightsp)
>  {
> diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
> index ab7ce9f..e569a3b 100644
> --- a/sys/sys/filedesc.h
> +++ b/sys/sys/filedesc.h
> @@ -205,6 +205,10 @@ fd_modified(struct filedesc *fdp, int fd, seq_t seq)
>  	return (!seq_consistent(fd_seq(fdp->fd_files, fd), seq));
>  }
>  
> +/* cdir/rdir/jdir manipulation functions. */
> +void	pwd_chdir(struct thread *td, struct vnode *vp);
> +int	pwd_chroot(struct thread *td, struct vnode *vp);
> +
>  #endif /* _KERNEL */
>  
>  #endif /* !_SYS_FILEDESC_H_ */
> diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
> index 36ef8af..6d5da32 100644
> --- a/sys/sys/vnode.h
> +++ b/sys/sys/vnode.h
> @@ -616,7 +616,6 @@ void	cache_purge(struct vnode *vp);
>  void	cache_purge_negative(struct vnode *vp);
>  void	cache_purgevfs(struct mount *mp);
>  int	change_dir(struct vnode *vp, struct thread *td);
> -int	change_root(struct vnode *vp, struct thread *td);
>  void	cvtstat(struct stat *st, struct ostat *ost);
>  void	cvtnstat(struct stat *sb, struct nstat *nsb);
>  int	getnewvnode(const char *tag, struct mount *mp, struct vop_vector *vops,
> diff --git a/sys/ufs/ffs/ffs_alloc.c b/sys/ufs/ffs/ffs_alloc.c
> index 2b9c334..c587dfb 100644
> --- a/sys/ufs/ffs/ffs_alloc.c
> +++ b/sys/ufs/ffs/ffs_alloc.c
> @@ -2748,13 +2748,12 @@ sysctl_ffs_fsck(SYSCTL_HANDLER_ARGS)
>  	struct thread *td = curthread;
>  	struct fsck_cmd cmd;
>  	struct ufsmount *ump;
> -	struct vnode *vp, *vpold, *dvp, *fdvp;
> +	struct vnode *vp, *dvp, *fdvp;
>  	struct inode *ip, *dp;
>  	struct mount *mp;
>  	struct fs *fs;
>  	ufs2_daddr_t blkno;
>  	long blkcnt, blksize;
> -	struct filedesc *fdp;
>  	struct file *fp, *vfp;
>  	cap_rights_t rights;
>  	int filetype, error;
> @@ -2968,12 +2967,7 @@ sysctl_ffs_fsck(SYSCTL_HANDLER_ARGS)
>  			break;
>  		}
>  		VOP_UNLOCK(vp, 0);
> -		fdp = td->td_proc->p_fd;
> -		FILEDESC_XLOCK(fdp);
> -		vpold = fdp->fd_cdir;
> -		fdp->fd_cdir = vp;
> -		FILEDESC_XUNLOCK(fdp);
> -		vrele(vpold);
> +		pwd_chdir(td, vp);
>  		break;
>  
>  	case FFS_SET_DOTDOT:

Looks good.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150711135303.GH2080>