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

[Topi Miettinen]

On 01/01/15 18:08, Dave Reisner wrote:
> On Thu, Jan 01, 2015 at 04:49:04PM +0200, Topi Miettinen wrote:
>> Copy parent directory mount flags when setting up a namespace and
>> don't accidentally clear mount flags later.
>>
>> Signed-off-by: Topi Miettinen <toiwoton at gmail.com>
>> ---
>>  src/core/namespace.c |  4 ++--
>>  src/shared/util.c    | 20 ++++++++++++++++++--
>>  src/shared/util.h    |  2 ++
>>  3 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/core/namespace.c b/src/core/namespace.c
>> index 5b408e0..400bc50 100644
>> --- a/src/core/namespace.c
>> +++ b/src/core/namespace.c
>> @@ -159,7 +159,7 @@ 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) {
>> +        if (mount("tmpfs", dev, "tmpfs", get_mount_flags("/dev")|MS_NOSUID|MS_STRICTATIME, "mode=755") < 0) {
>>                  r = -errno;
>>                  goto fail;
>>          }
>> @@ -282,7 +282,7 @@ 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) {
>> +        if (mount("tmpfs", root, "tmpfs", get_mount_flags("/kdbus")|MS_NOSUID|MS_STRICTATIME, "mode=777") < 0) {
> 
> Shouldn't this be /sys/fs/bus/kdbus? We certainly don't mount kdbusfs in
> the root...

Probably. I don't have kdbus here (sorry) and I don't quite get what the
function is supposed to do.

> 
>>                  r = -errno;
>>                  goto fail;
>>          }
>> diff --git a/src/shared/util.c b/src/shared/util.c
>> index dfaf7f7..31fbb68 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;
>>  }
>>  
>> +unsigned long get_mount_flags(const char *path)
>> +{
> 
> Reigning style says to put the opening { at the end of the first line.

OK.

Thanks for the review.

> 
>> +        struct statvfs buf;
>> +
>> +        if (statvfs(path, &buf) < 0)
>> +                return 0;
>> +
>> +        return buf.f_flag;
>> +}
>> +
>>  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,9 @@ 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)
>> +                        orig_flags = get_mount_flags(prefix);
>> +                        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 +7003,9 @@ 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) {
>> +                        orig_flags = get_mount_flags(x);
>> +                        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..4b3070a 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);
>>  
>> +unsigned long get_mount_flags(const char *path);
>> +
>>  int umount_recursive(const char *target, int flags);
>>  
>>  int bind_remount_recursive(const char *prefix, bool ro);
>> -- 
>> 2.1.4
>>
>> _______________________________________________
>> systemd-devel mailing list
>> systemd-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel