[systemd-devel] [PATCH v3] Do not clear parent mount flags when setting up namespaces
Topi Miettinen
toiwoton at gmail.com
Sun Jan 4 10:49:12 PST 2015
On 01/04/15 12:00, Djalal Harouni wrote:
> Hi Topi,
>
> On Sun, Jan 04, 2015 at 11:26:12AM +0000, Topi Miettinen wrote:
>> On 01/03/15 12:58, Topi Miettinen wrote:
>>> When setting up a namespace, flags like noexec, nosuid and nodev are
>>> cleared, so the mounts always have exec, suid, dev flags enabled.
>>>
>>> Copy parent directory mount flags when setting up a namespace and
>>> don't accidentally clear mount flags later.
>>
>> Sorry, this version with statvfs() error checks breaks boot. I'm trying
>> to find out why.
> Just to let you know in case you are running with kdbus, currently there
> are reports that kdbus+systemd git HEAD is broken, we plan to investigate
> it this week.
Thanks, but I don't have kdbus set up. The problem was with changes to
mount_dev() (by similarity probably also mount_kdbus() could be buggy).
I made a simpler patch which just avoids clobbering the mount flags when
remounting.
-Topi
>
> Thanks!
>
>>> ---
>>> src/core/namespace.c | 12 ++++++++++--
>>> src/shared/util.c | 24 ++++++++++++++++++++++--
>>> src/shared/util.h | 2 ++
>>> 3 files changed, 34 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/core/namespace.c b/src/core/namespace.c
>>> index 4b8dbdd..6807e0c 100644
>>> --- a/src/core/namespace.c
>>> +++ b/src/core/namespace.c
>>> @@ -149,6 +149,7 @@ static int mount_dev(BindMount *m) {
>>> const char *d, *dev = NULL, *devpts = NULL, *devshm = NULL, *devhugepages = NULL, *devmqueue = NULL, *devlog = NULL, *devptmx = NULL;
>>> _cleanup_umask_ mode_t u;
>>> int r;
>>> + unsigned long parent_flags;
>>>
>>> assert(m);
>>>
>>> @@ -159,7 +160,10 @@ static int mount_dev(BindMount *m) {
>>>
>>> dev = strappenda(temporary_mount, "/dev");
>>> (void)mkdir(dev, 0755);
>>> - if (mount("tmpfs", dev, "tmpfs", MS_NOSUID|MS_STRICTATIME, "mode=755") < 0) {
>>> + r = get_mount_flags("/dev", &parent_flags);
>>> + if (r < 0)
>>> + goto fail;
>>> + if (mount("tmpfs", dev, "tmpfs", parent_flags|MS_NOSUID|MS_STRICTATIME, "mode=755") < 0) {
>>> r = -errno;
>>> goto fail;
>>> }
>>> @@ -272,6 +276,7 @@ static int mount_kdbus(BindMount *m) {
>>> char *busnode = NULL, *root;
>>> struct stat st;
>>> int r;
>>> + unsigned long parent_flags;
>>>
>>> assert(m);
>>>
>>> @@ -282,7 +287,10 @@ static int mount_kdbus(BindMount *m) {
>>>
>>> root = strappenda(temporary_mount, "/kdbus");
>>> (void)mkdir(root, 0755);
>>> - if (mount("tmpfs", root, "tmpfs", MS_NOSUID|MS_STRICTATIME, "mode=777") < 0) {
>>> + r = get_mount_flags("/sys/fs/kdbus", &parent_flags);
>>> + if (r < 0)
>>> + goto fail;
>>> + if (mount("tmpfs", root, "tmpfs", parent_flags|MS_NOSUID|MS_STRICTATIME, "mode=777") < 0) {
>>> r = -errno;
>>> goto fail;
>>> }
>>> diff --git a/src/shared/util.c b/src/shared/util.c
>>> index dfaf7f7..b28213f 100644
>>> --- a/src/shared/util.c
>>> +++ b/src/shared/util.c
>>> @@ -61,6 +61,7 @@
>>> #include <sys/personality.h>
>>> #include <sys/xattr.h>
>>> #include <libgen.h>
>>> +#include <sys/statvfs.h>
>>> #undef basename
>>>
>>> #ifdef HAVE_SYS_AUXV_H
>>> @@ -6858,6 +6859,16 @@ int umount_recursive(const char *prefix, int flags) {
>>> return r ? r : n;
>>> }
>>>
>>> +int get_mount_flags(const char *path, unsigned long *flags) {
>>> + struct statvfs buf;
>>> +
>>> + if (statvfs(path, &buf) < 0)
>>> + return -errno;
>>> +
>>> + *flags = buf.f_flag;
>>> + return 0;
>>> +}
>>> +
>>> int bind_remount_recursive(const char *prefix, bool ro) {
>>> _cleanup_set_free_free_ Set *done = NULL;
>>> _cleanup_free_ char *cleaned = NULL;
>>> @@ -6892,6 +6903,7 @@ int bind_remount_recursive(const char *prefix, bool ro) {
>>> _cleanup_set_free_free_ Set *todo = NULL;
>>> bool top_autofs = false;
>>> char *x;
>>> + unsigned long orig_flags;
>>>
>>> todo = set_new(&string_hash_ops);
>>> if (!todo)
>>> @@ -6969,7 +6981,11 @@ int bind_remount_recursive(const char *prefix, bool ro) {
>>> if (mount(cleaned, cleaned, NULL, MS_BIND|MS_REC, NULL) < 0)
>>> return -errno;
>>>
>>> - if (mount(NULL, prefix, NULL, MS_BIND|MS_REMOUNT|(ro ? MS_RDONLY : 0), NULL) < 0)
>>> + r = get_mount_flags(prefix, &orig_flags);
>>> + if (r < 0)
>>> + return r;
>>> + orig_flags &= ~MS_RDONLY;
>>> + if (mount(NULL, prefix, NULL, orig_flags|MS_BIND|MS_REMOUNT|(ro ? MS_RDONLY : 0), NULL) < 0)
>>> return -errno;
>>>
>>> x = strdup(cleaned);
>>> @@ -6989,7 +7005,11 @@ int bind_remount_recursive(const char *prefix, bool ro) {
>>> if (r < 0)
>>> return r;
>>>
>>> - if (mount(NULL, x, NULL, MS_BIND|MS_REMOUNT|(ro ? MS_RDONLY : 0), NULL) < 0) {
>>> + r = get_mount_flags(x, &orig_flags);
>>> + if (r < 0)
>>> + return r;
>>> + orig_flags &= ~MS_RDONLY;
>>> + if (mount(NULL, x, NULL, orig_flags|MS_BIND|MS_REMOUNT|(ro ? MS_RDONLY : 0), NULL) < 0) {
>>>
>>> /* Deal with mount points that are
>>> * obstructed by a later mount */
>>> diff --git a/src/shared/util.h b/src/shared/util.h
>>> index a131a3c..d5aa988 100644
>>> --- a/src/shared/util.h
>>> +++ b/src/shared/util.h
>>> @@ -1021,6 +1021,8 @@ union file_handle_union {
>>>
>>> int update_reboot_param_file(const char *param);
>>>
>>> +int get_mount_flags(const char *path, unsigned long *flags);
>>> +
>>> int umount_recursive(const char *target, int flags);
>>>
>>> int bind_remount_recursive(const char *prefix, bool ro);
>>>
>>
>> _______________________________________________
>> systemd-devel mailing list
>> systemd-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
More information about the systemd-devel
mailing list