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

2015-01-04 Thread 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
 @@

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

2015-01-04 Thread Djalal Harouni 
Hi Topi,

On Sun, Jan 04, 2015 at 11:26:12AM +, 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!

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

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

2015-01-04 Thread Topi Miettinen 
On 01/04/15 12:00, Djalal Harouni wrote:
 Hi Topi,
 
 On Sun, Jan 04, 2015 at 11:26:12AM +, 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,

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

2015-01-03 Thread Topi Miettinen 
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.
---
 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