cmd/snap-confine: look for PROCFS_SUPER_MAGIC #2843

Merged
merged 1 commit into from Feb 14, 2017

Conversation

Projects
None yet
3 participants
Contributor

zyga commented Feb 14, 2017

This patch makes it possible for snap-confine to run on kernel 3.10 where nsfs
is not a thing yet. Everything seems to work fine but we need to look for this
filesystem type in addition to NSFS_MAGIC as we usually do.

Signed-off-by: Zygmunt Krynicki me@zygoon.pl

cmd/snap-confine: look for PROCFS_SUPER_MAGIC
This patch makes it possible for snap-confine to run on kernel 3.10 where nsfs
is not a thing yet. Everything seems to work fine but we need to look for this
filesystem type in addition to NSFS_MAGIC as we usually do.

Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>

@zyga zyga added this to the 2.23 milestone Feb 14, 2017

Contributor

jdstrand commented Feb 14, 2017

The code changes are fine assuming that we can do everything exactly the same when buf.f_type == PROC_SUPER_MAGIC, but I don't know that to be true. You said 'everything works' fine, but I don't see tests demonstrating it does. Can you add a test that is skipped when NSFS_MAGIC is available and used when it is not?

+1 conditional on test demonstrating this works properly. Since I suspect this will need to be a manual test, please paste the output of the manual test. Thanks!

Contributor

zyga commented Feb 14, 2017

@jdstrand I ran half a dozen snaps with this patch in. I don't have a reliable test at this stage as the effort is just unfolding but landing this will pave the way to packaging and testing this on an ongoing basis, with spread. Until then (until we have spread run on 3.10) I can only give you my word that it works.

The manual page for namespaces talks about /proc/$PID/ns/mnt being a thing since 3.8 but the new nsfs filesystem was added later (I didn't check when exactly). The important property is that the behaviour of bind mounting those files was already supported back then, it was just the case that they all belonged to the proc filesystem.

Contributor

jdstrand commented Feb 14, 2017

Isn't the test simply on a 3.10 kernel, install snapd/snap-confine configured for devmode, install hello-world, start hello-world.sh, touch /tmp/foo, exit, then start hello-world.sh and see if /tmp/foo exists? If this is what you meant by 'ran half a dozen snaps with this patch in', then +1.

Contributor

zyga commented Feb 14, 2017

@jdstrand yes, I did exactly this (on CentOS 7 to be precise). Thank you for the +1

vosst approved these changes Feb 14, 2017

LGTM

@zyga zyga merged commit 4e6420a into snapcore:master Feb 14, 2017

6 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details

@zyga zyga deleted the zyga:kernel-3.10 branch Feb 15, 2017

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