cmd/snap-confine,tests: fix unmounting on systems without rshared / #4258

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

zyga commented Nov 20, 2017

This patch adds a test that reproduces a bug in snap-confine/snapd that
affects Ubuntu 16.04 running an LXD container with snapd. Inside such
container the root directory is not implicitly rshared by systemd and
after running any snap command snaps cannot be removed/unmounted
correctly.

The error is addressed by a little trick in snap-confine. All the snap
bind mounts are stopped. The /snap directory is bind-mounted over
itself, rshare is applied and then all the snap mount units are started
again. We must rely on this trick as there is insufficient information
in the mount table for to correctly stop and then restart fuse
(squashfuse) mounts.

With-Kind-Regards-To: Kyle Fazzari
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

cmd/snap-confine,tests: fix unmounting on systems without rshared /
This patch adds a test that reproduces a bug in snap-confine/snapd that
affects Ubuntu 16.04 running an LXD container with snapd. Inside such
container the root directory is not implicitly rshared by systemd and
after running any snap command snaps cannot be removed/unmounted
correctly.

The error is addressed by a little trick in snap-confine. All the snap
bind mounts are stopped. The /snap directory is bind-mounted over
itself, rshare is applied and then all the snap mount units are started
again. We must rely on this trick as there is insufficient information
in the mount table for to correctly stop and then restart fuse
(squashfuse) mounts.

With-Kind-Regards-To: Kyle Fazzari
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

codecov-io commented Nov 20, 2017

Codecov Report

Merging #4258 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4258   +/-   ##
=======================================
  Coverage   75.96%   75.96%           
=======================================
  Files         440      440           
  Lines       38428    38428           
=======================================
  Hits        29191    29191           
  Misses       7223     7223           
  Partials     2014     2014

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4520d5...6282e07. Read the comment docs.

Contributor

zyga commented Nov 20, 2017

I need to figure out why this works in qemu but fails on linode.

@zyga zyga closed this Nov 20, 2017

I don't care for snap-confine calling systemd to stop/start mount units. It just doesn't seem like the right place to be doing it. I can't help but think that there are other alternatives, such as snapd detecting it is running under confinement and doing something with the mounts, adding additional options to .mount units, introducing additional another unit that calls a non-setuid helper, etc.

+ dent->d_name);
+ if (system(buf) < 0) {
+ die("cannot stop mount unit %s", dent->d_name);
+ }
@@ -120,6 +120,8 @@
# LP: #1668659
mount options=(rw rbind) /snap/ -> /snap/,
mount options=(rw rshared) -> /snap/,
+ /etc/systemd/system/ r,
+ /bin/systemctl Ux,
@jdstrand

jdstrand Nov 20, 2017

Contributor

With this you we're pretty much to the point that snap-confine may as well run unconfined.

@zyga

zyga Nov 20, 2017

Contributor

I realize that but apart from running a helper that is unconfined we cannot do the mounting ourselves. This is, unfortunately, caused by FUSE being opaque and not showing what is being mounted. As an alternative we could unmount the mount units ourselves (I assume systemd is smart enough here though I haven't checked that) and build some logic to mount them back using either squashfuse or directly. Then it should be very well defined what is going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment