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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/systemd-devel > _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel