[systemd-devel] [PATCH v3] Do not clear parent mount flags when setting up namespaces

[Topi Miettinen]

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.

> ---
>  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);
>