[lxc-devel] [PATCH] Support MS_SHARED /

[Eric W. Biederman]

Serge Hallyn <serge.hallyn at canonical.com> writes:

> Quoting Eric W. Biederman (ebiederm at xmission.com):
>> Serge Hallyn <serge.hallyn at canonical.com> writes:
>> 
>> > Quoting Michael H. Warfield (mhw at WittsEnd.com):
>> >> On Wed, 2013-01-16 at 16:46 -0600, Serge Hallyn wrote:
>> >> > Quoting Michael H. Warfield (mhw at WittsEnd.com):
>> >> > > Serge,
>> >> > > 
>> >> > > Revisiting an earlier remark...
>> >> > ...
>> >> > > > Now I tested, and with a simple setup we can use a much simpler
>> >> > > > patch which just does mount("", "/", NULL, MS_SLAVE|MS_REC, 0);
>> >> > > > for the whole of chroot_into_slave() (and skips the new umount2()
>> >> > > > in start.c).  The container then starts, and its mounts table
>> >> > > > is clean.
>> >> > > 
>> >> > > Were you still looking at this?  Currently, with the MS_SHARED patch
>> >> > 
>> >> > No, I haven't been.
>> >> > 
>> >> > > work in 0.9.0, the mount table is pretty ugly and running "df" in a
>> >> > > container is really ugly...
>> >> > > 
>> >> > 
>> >> > ...
>> >> > 
>> >> > > > Where that won't work is in a livecd or any fancy raid setup,
>> >> > > > where your process's / has a parent which is MS_SHARED.
>> >> > > 
>> >> > > How bad is this breakage in regards to that then?
>> >> 
>> >> > pivot_root would simply fail.
>> >> 
>> >> > Likewise, the case where / is actually type 'rootfs', which is
>> >> > not MS_USER and therefore can't be pivot_root()d from would
>> >> > fail.
>> >> 
>> >> > There is something else we could try.  Before we chroot() into
>> >> > our custom MS_SLAVE /, we could fork a child.  That child sticks
>> >> > around, waits for a signal saying the pivot_root+umounts are
>> >> > done, then it looks through /proc/self/mounts and unmounts
>> >> > anything which is not under '/root/'.
>> >> 
>> >> > I think that might really work best.
>> >> 
>> >> That certainly sounds like it might be worth a shot.  If you could get
>> >> me a patch, I could test.  Beginning in a week I'm going to be out of
>> >> pocket for a couple of weeks with access severely limited to my test
>> >> servers, though.
>> >
>> > Let's remember this for when you get back then - I won't have time (that
>> > is, be able to justify moving other things) before then.
>> >
>> > If someone else wants to write the patch, I think it'll be a fun one to
>> > write.
>> 
>> No patch.  But the way I can report how I solved this in iproute.
>> 
>> mount("", "/", "none", MS_SLAVE | MS_REC, NULL);
>
> Yes I've mentioned that as one possibility, but note that that
> won't work in one of the pathological cases - where / is actually
> the original root_fs, which is not MS_USER - so it actually will
> still prevent pivot_root.

If you don't want to modify the original mounts I think
mount --make-rslave / is pretty much mandatory.  Even
if your root is rootfs you probably want that in case
someone has done mount --make-rshared / on the rootfs.

And yes rootfs is special.  The proper thing to do with rootfs
is to unmount everything that is mounted delete all of the files
and then to mount over the top of it because things don't quite
work right if you do anything else.  And of course for that very reason
you will only see rootfs from an initramfs.

Anyway I just wanted to point it out because I hadn't seen it in the
conversation and the mount --make-rshared / command is a single function
call which makes it an easy change to make.  Even if there are more
comprehensive changes that can be made later.

Eric