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