[PATCH 1/2] Move chdir/chroot-related fdp manipulation to kern_descrip.c
Mateusz Guzik
mjguzik at gmail.com
Sat Jul 11 13:40:30 UTC 2015
On Sat, Jul 11, 2015 at 03:43:35PM +0300, Konstantin Belousov wrote:
> On Sat, Jul 11, 2015 at 01:08:03AM +0200, Mateusz Guzik wrote:
> > From: Mateusz Guzik <mjg at freebsd.org>
> >
> > Prefix exported functions with pwd_.
> >
> > This also adds a helper which sets fd_cdir which deduplicates some code.
> Patch is fine, I like the cleanup. Please use the opportunity to fix
> minor existing issues, mostly with style, see below.
>
> > + oldvp = fdp->fd_rdir;
> VREF(vp);
> > + fdp->fd_rdir = vp;
> > + VREF(fdp->fd_rdir);
> Remove this line.
>
> > + if (!fdp->fd_jdir) {
> if (fdp->fd_jdir == NULL) {
>
> > + fdp->fd_jdir = vp;
> > + VREF(fdp->fd_jdir);
> Again, swap the order: do VREF(vp), then assign.
>
Done.
> > + }
> > + 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);
> > + oldvp = fdp->fd_cdir;
> > + fdp->fd_cdir = vp;
> Assert that vp v_usecount > 0, as the inaccurate but still useful check.
> We have no way to ensure that the reference is owned by the code, but
> we do know that there must be such reference.
>
Done.
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:
--
Mateusz Guzik <mjguzik gmail.com>
More information about the freebsd-fs
mailing list