systemd: correct the mount arguments when mounting with squashfuse #2112

Merged
merged 1 commit into from Nov 16, 2016

Conversation

Projects
None yet
5 participants
Contributor

tyhicks commented Oct 6, 2016

The "allow_other" mount option must be specified when mounting snaps
with the squashfuse filesystem in order for non-root users to make use
of the mount point. Without the "allow_other" mount option, non-root
users will not be able to run snap commands due to EACESS being returned
for all filesystem accesses:

ubuntu@yakkety:~$ ls -al /snap/hello-world
ls: cannot access '/snap/hello-world/27': Permission denied
total 8
drwxr-xr-x 3 root root 4096 Oct 5 21:09 .
drwxr-xr-x 5 root root 4096 Oct 5 21:09 ..
d????????? ? ? ? ? ? 27
lrwxrwxrwx 1 root root 2 Oct 5 21:09 current -> 27

This patch also explicitly adds the "ro" mount option when squashfuse is
in use. This is done because, without the "ro" mount option, "rw"
incorrectly shows up in the mount table even though the squashfuse mount
point is read-only. Explicitly specifying the "ro" mount option prevents
any possible confusion.

Fixes: https://bugs.launchpad.net/snappy/+bug/1630789
Signed-off-by: Tyler Hicks tyhicks@canonical.com

systemd: Correct the mount arguments when mounting with squashfuse
The "allow_other" mount option must be specified when mounting snaps
with the squashfuse filesystem in order for non-root users to make use
of the mount point. Without the "allow_other" mount option, non-root
users will not be able to run snap commands due to EACESS being returned
for all filesystem accesses:

  ubuntu@yakkety:~$ ls -al /snap/hello-world
  ls: cannot access '/snap/hello-world/27': Permission denied
  total 8
  drwxr-xr-x 3 root root 4096 Oct 5 21:09 .
  drwxr-xr-x 5 root root 4096 Oct 5 21:09 ..
  d????????? ? ? ? ? ? 27
  lrwxrwxrwx 1 root root 2 Oct 5 21:09 current -> 27

This patch also explicitly adds the "ro" mount option when squashfuse is
in use. This is done because, without the "ro" mount option, "rw"
incorrectly shows up in the mount table even though the squashfuse mount
point is read-only. Explicitly specifying the "ro" mount option prevents
any possible confusion.

Fixes: https://bugs.launchpad.net/snappy/+bug/1630789
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Contributor

tyhicks commented Oct 6, 2016

One thing that's not included in this PR is how to force snapd to regenerate the existing mount units. Until the mount units are regenerated, snaps that are already installed will still have the wrong mount parameters for squashfuse mounts. Is this something that should be handled in the distro packaging?

zyga approved these changes Oct 6, 2016

LGTM

This looks good. Can we have a unit test for this, and preferably also a spread test that exercises the problem that this is fixing in practice, so we don't regress?

No need to regenerate the mount units for now. This is going to be quite tricky, and for that exact reason we're already rewriting some of our infrastructure (@chipaca is, actually) to avoid that sort of hardcoding.

Contributor

tyhicks commented Oct 6, 2016

There is an existing unit test, which is updated as part of this PR. Would you like me to add some negative tests?

I don't see where any spread tests were added as part of the original feature PR (#1380). I have some work to do in order to come up to speed on creating such tests and won't be able to add spread tests today but will get to it as soon as possible.

Collaborator

mvo5 commented Oct 10, 2016

Test failure looks unrelated: error: cannot refresh "test-snapd-tools": cannot get refresh information for snap "test-snapd-tools": Post https://search.apps.ubuntu.com/api/v1/snaps/metadata: EOF to this PR.

@niemeyer niemeyer changed the title from systemd: Correct the mount arguments when mounting with squashfuse to systemd: correct the mount arguments when mounting with squashfuse Oct 14, 2016

@chipaca chipaca merged commit deaf0bc into snapcore:master Nov 16, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tyhicks tyhicks deleted the tyhicks:lp1630789 branch May 24, 2017

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