Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Add support module for namespace sharing #126

Merged
merged 35 commits into from Sep 12, 2016
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
a369644
Add support module for namespace sharing
zyga Sep 2, 2016
f5f622c
Editorial changes to the manual page
zyga Sep 7, 2016
f990567
Trim whitespace
zyga Sep 7, 2016
c6d34b8
Consistently use < 0 instead of == -1
zyga Sep 7, 2016
983a8c7
Fix typo
zyga Sep 7, 2016
bca0fcb
Reword documentation for sc_open_ns_group
zyga Sep 7, 2016
edbff7d
Fix function name in documentation for sc_create_or_join_ns_group
zyga Sep 7, 2016
f707d6f
Fix typo
zyga Sep 8, 2016
1c171d9
Add test for sc_init_ns_groups
zyga Sep 8, 2016
d97dfec
Add tests for sc_is_ns_group_dir_private
zyga Sep 8, 2016
d7c8826
Reformat tests
zyga Sep 8, 2016
dbdb509
Make sc_set_ns_dir() subprocess-aware
zyga Sep 8, 2016
d0bb737
Use g_test_subprocess to and g_set_nonfatal_assertions
zyga Sep 8, 2016
836af23
Cleanup temporary data in tests
zyga Sep 8, 2016
1c742ed
Fix grammar
zyga Sep 8, 2016
ddea2bc
Document why we're opening the .mnt file without O_EXCL
zyga Sep 8, 2016
2d479d4
Check that we have a lock file in namespace locking fns
zyga Sep 8, 2016
221ffeb
Add missing return in test
zyga Sep 8, 2016
e6ce2f9
Be explicit about setting child pid to 0
zyga Sep 8, 2016
d20bc45
Comment that optional_fields is never NULL
zyga Sep 8, 2016
a890dbe
Tweak comment
zyga Sep 8, 2016
524ce1a
Move function for better readability
zyga Sep 8, 2016
598b31d
Move variable declaration closer to use
zyga Sep 8, 2016
0f51648
Tweak comment and unify comment style
zyga Sep 8, 2016
aa074ed
Tweak comment
zyga Sep 8, 2016
ab3c245
Remove duplicate include
zyga Sep 8, 2016
50ebef9
Document why casting pid_t to int is safe
zyga Sep 8, 2016
a4cc855
Fix function name
zyga Sep 8, 2016
b2fc050
Fix typo
zyga Sep 8, 2016
9123679
Reword comment
zyga Sep 8, 2016
6afb53b
Fix typo
zyga Sep 8, 2016
afc1cc9
Test for '< 0' instead of '== -1'
zyga Sep 8, 2016
db1a076
Add sanity check for nsfs
zyga Sep 8, 2016
40a93a2
Use fstatfs() and NSFS_MAGIC to decide about setns()
zyga Sep 8, 2016
d9adc64
Add some safeguards to rm_rf_tmp
zyga Sep 12, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 17 additions & 20 deletions src/ns-support.c
Expand Up @@ -23,6 +23,7 @@

#include <errno.h>
#include <fcntl.h>
#include <linux/magic.h>
#include <sched.h>
#include <signal.h>
#include <string.h>
Expand All @@ -32,6 +33,7 @@
#include <sys/prctl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
#include <sys/wait.h>
#include <unistd.h>
#ifdef HAVE_APPARMOR
Expand Down Expand Up @@ -250,38 +252,33 @@ void sc_create_or_join_ns_group(struct sc_ns_group *group)
// sc_discard_preserved_ns_group() it will revert to a regular file. If
// snap-confine is killed for whatever reason after the file is created but
// before the file is bind-mounted it will also be a regular file.
//
// The code below handles this by trying to join the namespace with setns()
// and handling both the successful and the unsuccessful paths.
mnt_fd =
openat(group->dir_fd, mnt_fname,
O_CREAT | O_RDONLY | O_CLOEXEC | O_NOFOLLOW, 0600);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment here on why you are not using O_EXCL? AIUI, you want to either open the nsfs mounted file or open the newly created file. It would be good to document that for auditability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right both in the deduction of the intent and the suggestion to document this better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented

if (mnt_fd < 0) {
die("cannot open mount namespace file for namespace group %s",
group->name);
}
// attempt to join an existing group
debug
("attempting to re-associate the mount namespace with the namespace group %s",
group->name);
if (setns(mnt_fd, CLONE_NEWNS) == 0) {
// Check if we got an nsfs-based file or a regular file. This can be
// reliably tested because nsfs has an unique filesystem type NSFS_MAGIC.
// We can just ensure that this is the case thanks to fstatfs.
struct statfs buf;
if (fstatfs(mnt_fd, &buf) < 0) {
die("cannot perform fstatfs() on an mount namespace file descriptor");
}
if (buf.f_type == NSFS_MAGIC) {
debug
("attempting to re-associate the mount namespace with the namespace group %s",
group->name);
if (setns(mnt_fd, CLONE_NEWNS) < 0) {
die("cannot re-associate the mount namespace with namespace group %s", group->name);
}
debug
("successfully re-associated the mount namespace with the namespace group %s",
group->name);
return;
}
// Anything but EINVAL is an unexpected error.
//
// EINVAL is simply a sign that the file we've opened is not a valid
// namespace file descriptor. One potential case where this can happen is
// when another snap-confine tried to initialize the namespace but was
// killed before it managed to complete the process.
if (errno != EINVAL) {
die("cannot re-associate the mount namespace with namespace group %s", group->name);
}
debug
("cannot re-associate the mount namespace with namespace group %s, falling back to initialization",
group->name);
debug("initializing new namespace group %s", group->name);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is soo much nicer, thanks! :)

It's interesting that NSFS_MAGIC is not included in man fstatfs. Note that this was added only in http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/include/uapi/linux/magic.h?id=e149ed2b805fefdccf7ccdfc19eca22fdd4514ac so this will likely need to be added to the backports list if it isn't already. It does seem present in the 14.04 Ubuntu kernel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've informed tvoss about this. With the sanity check we'll know if tests are all green. Thanks for making me aware of this :)

// Create a new namespace and ask the caller to populate it.
// For rationale of forking see this:
// https://lists.linuxfoundation.org/pipermail/containers/2013-August/033386.html
Expand Down