[CFR] [PATCH] jail mount/unmount patch

[Martin Matuska]

Dňa 28. 7. 2011 19:38, Pawel Jakub Dawidek wrote / napísal(a):
> On Thu, Jul 28, 2011 at 08:25:39PM +0300, Kostik Belousov wrote:
>> On Thu, Jul 28, 2011 at 06:07:52PM +0200, Pawel Jakub Dawidek wrote:
>>> In you patch you depend on fact that full path to mount directory is
>>> passed to the nmount(2) system call. This doesn't have to be true.
>>> I changed mount(8) to call realpath(3) in mount directory, but I see no
>>> reason someone calling nmount(2) directly with "./foo" mount dir.
>>>
>>> I think the proper way is to build full path from within the kernel
>>> using vn_fullpath_global().
>> It indeed may work if the supplied vnode is a directory, since it
>> will fall back to the dotdot lookup loop if the namecache is purged.
> Exactly. Mount directory by definition is always a directory, so it
> should be reliable.
>
I have reworked and tested the patch to use vn_fullpath_global(), patch
attached.
The vp to global path translation is done in vfs_domount_first() (static
function) that doesn't take a fspath argument anymore.

Please review, comment and/or test my attached patch.

-- 
Martin Matuska
FreeBSD committer
http://blog.vx.sk

-------------- next part --------------
Index: sys/kern/kern_jail.c
===================================================================
--- sys/kern/kern_jail.c	(revision 224471)
+++ sys/kern/kern_jail.c	(working copy)
@@ -3858,7 +3858,8 @@
 	case PRIV_VFS_UNMOUNT:
 	case PRIV_VFS_MOUNT_NONUSER:
 	case PRIV_VFS_MOUNT_OWNER:
-		if (cred->cr_prison->pr_allow & PR_ALLOW_MOUNT)
+		if (cred->cr_prison->pr_allow & PR_ALLOW_MOUNT &&
+			cred->cr_prison->pr_enforce_statfs != 2)
 			return (0);
 		else
 			return (EPERM);
Index: sys/kern/vfs_mount.c
===================================================================
--- sys/kern/vfs_mount.c	(revision 224471)
+++ sys/kern/vfs_mount.c	(working copy)
@@ -745,7 +745,6 @@
 vfs_domount_first(
 	struct thread *td,		/* Calling thread. */
 	struct vfsconf *vfsp,		/* File system type. */
-	char *fspath,			/* Mount path. */
 	struct vnode *vp,		/* Vnode to be covered. */
 	int fsflags,			/* Flags common to all filesystems. */
 	struct vfsoptlist **optlist	/* Options local to the filesystem. */
@@ -755,11 +754,23 @@
 	struct mount *mp;
 	struct vnode *newdp;
 	int error;
+	char *fspath, *fbuf;
 
 	mtx_assert(&Giant, MA_OWNED);
 	ASSERT_VOP_ELOCKED(vp, __func__);
 	KASSERT((fsflags & MNT_UPDATE) == 0, ("MNT_UPDATE shouldn't be here"));
 
+	/* Construct global filesystem path from vnode */
+	error = vn_fullpath_global(td, vp, &fspath, &fbuf);
+	if (error == 0 && strlen(fspath) >= MNAMELEN)
+		error = ENAMETOOLONG;
+	if (error != 0) {
+		vput(vp);
+		if (fbuf != NULL)
+			free(fbuf, M_TEMP);
+		return(error);
+	}
+
 	/*
 	 * If the user is not root, ensure that they own the directory
 	 * onto which we are attempting to mount.
@@ -781,12 +792,14 @@
 	}
 	if (error != 0) {
 		vput(vp);
+		free(fbuf, M_TEMP);
 		return (error);
 	}
 	VOP_UNLOCK(vp, 0);
 
 	/* Allocate and initialize the filesystem. */
 	mp = vfs_mount_alloc(vp, vfsp, fspath, td->td_ucred);
+	free(fbuf, M_TEMP);
 	/* XXXMAC: pass to vfs_mount_alloc? */
 	mp->mnt_optnew = *optlist;
 	/* Set the mount level flags. */
@@ -1070,7 +1083,7 @@
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	vp = nd.ni_vp;
 	if ((fsflags & MNT_UPDATE) == 0) {
-		error = vfs_domount_first(td, vfsp, fspath, vp, fsflags,
+		error = vfs_domount_first(td, vfsp, vp, fsflags,
 		    optlist);
 	} else {
 		error = vfs_domount_update(td, vp, fsflags, optlist);
@@ -1105,7 +1118,7 @@
 	} */ *uap;
 {
 	struct mount *mp;
-	char *pathbuf;
+	char *pathbuf, *rpathbuf;
 	int error, id0, id1;
 
 	AUDIT_ARG_VALUE(uap->flags);
@@ -1139,13 +1152,28 @@
 		}
 		mtx_unlock(&mountlist_mtx);
 	} else {
-		AUDIT_ARG_UPATH1(td, pathbuf);
+		/*
+		 * If we are jailed and enforce_statfs == 1
+		 * construct real filesystem path
+		 */
+		rpathbuf = malloc(MNAMELEN, M_TEMP, M_WAITOK);
+		if (jailed(td->td_ucred) &&
+		    td->td_ucred->cr_prison->pr_enforce_statfs == 1 &&
+		    strcmp(td->td_ucred->cr_prison->pr_path, "/") != 0) {
+			strlcpy(rpathbuf, td->td_ucred->cr_prison->pr_path,
+			    MNAMELEN);
+			strlcat(rpathbuf, pathbuf, MNAMELEN);
+		} else {
+			strlcpy(rpathbuf, pathbuf, MNAMELEN);
+		}
+		AUDIT_ARG_UPATH1(td, rpathbuf);
 		mtx_lock(&mountlist_mtx);
 		TAILQ_FOREACH_REVERSE(mp, &mountlist, mntlist, mnt_list) {
-			if (strcmp(mp->mnt_stat.f_mntonname, pathbuf) == 0)
+			if (strcmp(mp->mnt_stat.f_mntonname, rpathbuf) == 0)
 				break;
 		}
 		mtx_unlock(&mountlist_mtx);
+		free(rpathbuf, M_TEMP);
 	}
 	free(pathbuf, M_TEMP);
 	if (mp == NULL) {