[PATCH] jail mount/unmount patch
Please review my attached patch.
The patch fixes f_mntonname with mount/unmount inside a jail with allow.mount
enabled.
Filesystems mountable in a jail require the VFCF_JAIL flag (currently only ZFS).
With this patch, mount and unmount works both with enforce_statfs = 0 and
enforce_statfs = 1.
I suggest disabling mount/unmount for jails with enforce_statfs = 2, as this is
contradictory and does not play well with or without this patch.
I have successfully tested this patch with ZFS, nullfs and tmpfs.
To enable nullfs for a jail, you have to modify tmpfs/tmpfs_vfsops.c and
recompile the tmpfs module:
-VFS_SET(tmpfs_vfsops, tmpfs, 0);
+VFS_SET(tmpfs_vfsops, tmpfs, VFCF_JAIL);
To enable tmpfs for a jail, you have to modify nullfs/null_vfsops.c and
recompile the nullfs module:
-VFS_SET(null_vfsops, nullfs, VFCF_LOOPBACK);
+VFS_SET(null_vfsops, nullfs, VFCF_LOOPBACK | VFCF_JAIL);
The filesystems can be successfully mounted/unmounted inside a jail and also
unmounted from the parent host without problems.
The mount inside jail, a jail needs allow.mount=1 and enforce.statfs=0 or
enforce.statfs=1, for more information see jail(8)
I assume other filesystem not dealing with devices may work correctly with this
patch, too (e.g. nfs).
With jailed nullfs we can run tinderbox in a jail ;)
Please review, comment and/or test my attached patch.
Cheers,
mm
--
Martin Matuska
FreeBSD committer
http://blog.vx.sk
Index: src/sys/kern/kern_jail.c
===
--- src/sys/kern/kern_jail.c (revision 224297)
+++ src/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: src/sys/kern/vfs_mount.c
===
--- src/sys/kern/vfs_mount.c (revision 224297)
+++ src/sys/kern/vfs_mount.c (working copy)
@@ -1007,6 +1007,7 @@
struct vfsconf *vfsp;
struct nameidata nd;
struct vnode *vp;
+ char realfspath[MNAMELEN];
int error;
/*
@@ -1023,6 +1024,21 @@
}
/*
+ * If we are jailed, construct real filesystem path
+ */
+ if (jailed(td-td_ucred)
+ strcmp(td-td_ucred-cr_prison-pr_path, /) != 0) {
+ if (strlen(td-td_ucred-cr_prison-pr_path) +
+ strlen(fspath) = MNAMELEN)
+ return (ENAMETOOLONG);
+ strlcpy(realfspath, td-td_ucred-cr_prison-pr_path,
+ sizeof(realfspath));
+ strlcat(realfspath, fspath, sizeof(realfspath));
+ } else {
+ strlcpy(realfspath, fspath, sizeof(realfspath));
+ }
+
+ /*
* Do not allow NFS export or MNT_SUIDDIR by unprivileged users.
*/
if (fsflags MNT_EXPORTED) {
@@ -1070,7 +1086,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, realfspath, vp, fsflags,
optlist);
} else {
error = vfs_domount_update(td, vp, fsflags, optlist);
@@ -1107,6 +1123,7 @@
struct mount *mp;
char *pathbuf;
int error, id0, id1;
+ char realfspath[MNAMELEN];
AUDIT_ARG_VALUE(uap-flags);
if (jailed(td-td_ucred) || usermount == 0) {
@@ -1139,10 +1156,23 @@
}
mtx_unlock(mountlist_mtx);
} else {
- AUDIT_ARG_UPATH1(td, pathbuf);
+ /*
+ * If we are jailed and enforce_statfs == 1
+ * construct real filesystem path
+ */
+ if (jailed(td-td_ucred)
+ td-td_ucred-cr_prison-pr_enforce_statfs == 1
+ strcmp(td-td_ucred-cr_prison-pr_path, /) != 0) {
+ strlcpy(realfspath, td-td_ucred-cr_prison-pr_path,
+ sizeof(realfspath));
+ strlcat(realfspath, pathbuf, sizeof(realfspath));
+ } else {
+ strlcpy(realfspath, pathbuf, sizeof(realfspath));
+ }
+ AUDIT_ARG_UPATH1(td, realfspath);
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, realfspath) == 0)
break;
}
mtx_unlock(mountlist_mtx);
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [PATCH] jail mount/unmount patch
There's a curious asymmetry here: enforce_statfs==1 is checked for munging the name on unmounting, but not on mounting. I see the point on the unmount side, as statfs would give the full un-jailed pathname and an admin would naturally want to unmount what he sees mounted, but without the same logic on the mount side, it would mean the unmount path is different from the mount path which would make fstab not use for mounting inside a jail. But then, enforce_statfs==0 is a strange world to be in anyway. I'm not sure about enforce_statfs!=2 in the privilege check. It seems a reasonable response to a contradictory set of permissions, but then so does the strange case if being able to mount a filesystem and then not being able to see it in statfs. - Jamie On 07/28/11 08:59, Martin Matuska wrote: Please review my attached patch. The patch fixes f_mntonname with mount/unmount inside a jail with allow.mount enabled. Filesystems mountable in a jail require the VFCF_JAIL flag (currently only ZFS). With this patch, mount and unmount works both with enforce_statfs = 0 and enforce_statfs = 1. I suggest disabling mount/unmount for jails with enforce_statfs = 2, as this is contradictory and does not play well with or without this patch. I have successfully tested this patch with ZFS, nullfs and tmpfs. To enable nullfs for a jail, you have to modify tmpfs/tmpfs_vfsops.c and recompile the tmpfs module: -VFS_SET(tmpfs_vfsops, tmpfs, 0); +VFS_SET(tmpfs_vfsops, tmpfs, VFCF_JAIL); To enable tmpfs for a jail, you have to modify nullfs/null_vfsops.c and recompile the nullfs module: -VFS_SET(null_vfsops, nullfs, VFCF_LOOPBACK); +VFS_SET(null_vfsops, nullfs, VFCF_LOOPBACK | VFCF_JAIL); The filesystems can be successfully mounted/unmounted inside a jail and also unmounted from the parent host without problems. The mount inside jail, a jail needs allow.mount=1 and enforce.statfs=0 or enforce.statfs=1, for more information see jail(8) I assume other filesystem not dealing with devices may work correctly with this patch, too (e.g. nfs). With jailed nullfs we can run tinderbox in a jail ;) Please review, comment and/or test my attached patch. Cheers, mm ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [PATCH] jail mount/unmount patch
For vfs_mount, the easiest way to look at this is to follow the path of the realfspath argument. It goes to vfs_domount_first() and ends up in vfs_mount_alloc() where is the only place it is consumed: strlcpy(mp-mnt_stat.f_mntonname, fspath, MNAMELEN); The original fspath (not realfspath) is correctly consumed in vnode initialization - vfs_domount(): NDINIT(...) that ends in namei() of vfs_lookup.c - here we are processing jail paths again so it gets translated properly and the correct vnode is picked. We cannot write the supplied fspath to f_mntonname as this won't reflect real path and we cannot unmount by name anymore from host and from jail. If reading, f_mntonname gets translated by prison_enforce_statfs so we should translate it correctly if writing so that it matches the vnode. We cannot add the check to vfs_mount_alloc() because we may blow MNAMELEN. The check could be theoretically moved to vfs_domount_first() before mp = vfs_mount_alloc(...) at the latest. For the enforce_statfs privilege check: with enforce_statfs=2 you are unable to unmount filesystems in a jail as they are simply not found (mount works). Cheers, mm Dňa 28. 7. 2011 18:12, Jamie Gritton wrote / napísal(a): There's a curious asymmetry here: enforce_statfs==1 is checked for munging the name on unmounting, but not on mounting. I see the point on the unmount side, as statfs would give the full un-jailed pathname and an admin would naturally want to unmount what he sees mounted, but without the same logic on the mount side, it would mean the unmount path is different from the mount path which would make fstab not use for mounting inside a jail. But then, enforce_statfs==0 is a strange world to be in anyway. I'm not sure about enforce_statfs!=2 in the privilege check. It seems a reasonable response to a contradictory set of permissions, but then so does the strange case if being able to mount a filesystem and then not being able to see it in statfs. - Jamie On 07/28/11 08:59, Martin Matuska wrote: Please review my attached patch. The patch fixes f_mntonname with mount/unmount inside a jail with allow.mount enabled. Filesystems mountable in a jail require the VFCF_JAIL flag (currently only ZFS). With this patch, mount and unmount works both with enforce_statfs = 0 and enforce_statfs = 1. I suggest disabling mount/unmount for jails with enforce_statfs = 2, as this is contradictory and does not play well with or without this patch. I have successfully tested this patch with ZFS, nullfs and tmpfs. To enable nullfs for a jail, you have to modify tmpfs/tmpfs_vfsops.c and recompile the tmpfs module: -VFS_SET(tmpfs_vfsops, tmpfs, 0); +VFS_SET(tmpfs_vfsops, tmpfs, VFCF_JAIL); To enable tmpfs for a jail, you have to modify nullfs/null_vfsops.c and recompile the nullfs module: -VFS_SET(null_vfsops, nullfs, VFCF_LOOPBACK); +VFS_SET(null_vfsops, nullfs, VFCF_LOOPBACK | VFCF_JAIL); The filesystems can be successfully mounted/unmounted inside a jail and also unmounted from the parent host without problems. The mount inside jail, a jail needs allow.mount=1 and enforce.statfs=0 or enforce.statfs=1, for more information see jail(8) I assume other filesystem not dealing with devices may work correctly with this patch, too (e.g. nfs). With jailed nullfs we can run tinderbox in a jail ;) Please review, comment and/or test my attached patch. Cheers, mm -- Martin Matuska FreeBSD committer http://blog.vx.sk ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
