[apparmor] [patch] tests: work around systemd mounting / shared in pivot_root tests

[Steve Beattie]

On Tue, Mar 24, 2015 at 05:22:01PM -0500, Tyler Hicks wrote:
> On 2015-03-24 14:48:22, Steve Beattie wrote:
> > The systemd init daemon mounts the / filesystem as shared [1], which
> > breaks pivot_root(2). The following patch adjusts the pivot_root
> > test script to remount / as private if it detects that its shared,
> > allowing the tests to run successfully, and then undoes it once the
> > tests are complete.
> > 
> > [1] http://cgit.freedesktop.org/systemd/systemd/commit/?id=b3ac5f8cb98757416d8660023d6564a7c411f0a0
> > 
> > Signed-off-by: Steve Beattie <steve at nxnw.org>
> > ---
> >  tests/regression/apparmor/pivot_root.sh |   24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > Index: b/tests/regression/apparmor/pivot_root.sh
> > ===================================================================
> > --- a/tests/regression/apparmor/pivot_root.sh
> > +++ b/tests/regression/apparmor/pivot_root.sh
> > @@ -25,6 +25,7 @@ put_old=${new_root}put_old/
> >  bad=$tmpdir/BAD/
> >  proc=$new_root/proc
> >  fstype="ext2"
> > +root_was_shared="no"
> >  
> >  pivot_root_cleanup() {
> >  	mountpoint -q "$proc"
> > @@ -36,9 +37,32 @@ pivot_root_cleanup() {
> >  	if [ $? -eq 0 ] ; then
> >  		umount "$new_root"
> >  	fi
> > +
> > +	if [ "${root_was_shared}" = "yes" ] ; then
> > +		[ -n "$VERBOSE" ] && echo 'notice: re-mounting / as shared'
> > +		mount -o remount --make-shared /
> 
> I don't think the '-o remount' is needed. I've never used it when
> changing a mount's propagation status and the mount(8) man page doesn't
> use it in its examples.
> 
> I strace'ed mount with and without '-o remount' to see if there's a
> difference:
> 
>  $ sudo strace mount --make-private / 2>&1 | tail
>  fstat(3, {st_mode=S_IFREG|0644, st_size=3165552, ...}) = 0
>  mmap(NULL, 3165552, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fed06565000
>  close(3)                                = 0
>  getuid()                                = 0
>  geteuid()                               = 0
>  mount("none", "/", NULL, MS_PRIVATE, NULL) = 0
>  close(1)                                = 0
>  close(2)                                = 0
>  exit_group(0)                           = ?
>  +++ exited with 0 +++
> 
>  $ sudo strace mount -o remount --make-private / 2>&1 | tail
>  stat("/sbin/mount.ext3", 0x7ffdf0a11730) = -1 ENOENT (No such file or directory)
>  stat("/sbin/fs.d/mount.ext3", 0x7ffdf0a11730) = -1 ENOENT (No such file or directory)
>  stat("/sbin/fs/mount.ext3", 0x7ffdf0a11730) = -1 ENOENT (No such file or directory)
>  mount("none", "/", 0x18a1210, MS_MGC_VAL|MS_REMOUNT|MS_SILENT, NULL) = 0
>  mount("none", "/", NULL, MS_SILENT|MS_PRIVATE, NULL) = 0
>  access("/", W_OK)                       = 0
>  close(1)                                = 0
>  close(2)                                = 0
>  exit_group(0)                           = ?
>  +++ exited with 0 +++
> 
> The '-o remount' ends up being an entirely separate operation that
> shouldn't be needed so I think it is best to drop it.
> 
> > +	fi
> >  }
> >  do_onexit="pivot_root_cleanup"
> >  
> > +# systemd mounts / and everything under it MS_SHARED. This breaks
> > +# pivot_root entirely, so attempt to detect it, and remount /
> > +# MS_PRIVATE temporarily.
> > +FINDMNT=/bin/findmnt
> > +if [ -x "${FINDMNT}" ] && ${FINDMNT} -no PROPAGATION / > /dev/null 2>&1 ; then
> > +	if [ "$(${FINDMNT} -no PROPAGATION /)" == "shared" ] ; then
> > +		root_was_shared="yes"
> > +	fi
> > +elif [ "$(ps -hp1  -ocomm)" = "systemd" ] ; then
> > +	# no findmnt or findmnt doesn't know the PROPAGATION column,
> > +	# but init is systemd so assume rootfs is shared
> > +	root_was_shared="yes"
> > +fi
> > +if [ "${root_was_shared}" = "yes" ] ; then
> > +	[ -n "$VERBOSE" ] && echo 'notice: re-mounting / as private'
> > +	mount -o remount --make-private /
> 
> Here, too.
> 
> Everything else looks good. Feel free to put my ack on it if you agree
> with my proposed changes.

All the more evidence that this shared mount stuff is duct-taped to
the side of the kernel. But I agree with your analysis and will drop
the -o remount bits.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150324/d5378cdf/attachment-0001.pgp>