Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1613175 > unrolled thread

[PATCH RFC 0/4] proc: support multiple separate proc instances per pidnamespace

Back to article view | Back to linux.kernel

Contents

  [PATCH RFC 0/4] proc: support multiple separate proc instances per pidnamespace Djalal Harouni <tixxdz@gmail.com> - 2017-03-30 17:30 +0200
    [PATCH RFC 3/4] proc: support mounting new procfs instances inside same pid namespace Djalal Harouni <tixxdz@gmail.com> - 2017-03-30 17:30 +0200
      Re: [PATCH RFC 3/4] proc: support mounting new procfs instances  inside same pid namespace Andy Lutomirski <luto@amacapital.net> - 2017-03-30 21:20 +0200
    Re: [PATCH RFC 0/4] proc: support multiple separate proc instances  per pidnamespace Andy Lutomirski <luto@amacapital.net> - 2017-03-30 21:20 +0200
      Re: [PATCH RFC 0/4] proc: support multiple separate proc instances  per pidnamespace Djalal Harouni <tixxdz@gmail.com> - 2017-03-31 13:50 +0200
    Re: [PATCH RFC 0/4] proc: support multiple separate proc instances  per pidnamespace Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-03-31 00:10 +0200
      Re: [PATCH RFC 0/4] proc: support multiple separate proc instances  per pidnamespace Djalal Harouni <tixxdz@gmail.com> - 2017-03-31 13:30 +0200

#1613175 — [PATCH RFC 0/4] proc: support multiple separate proc instances per pidnamespace

Hi,

This RFC can be applied on top of Linus' tree 89970a04d7

This RFC implements support for multiple separate proc instances inside
the same pid namespace. This allows to solve lot of problems that
today's use case face.

Historically procfs was tied to pid namespaces, and mount options were
propagated to all other procfs instances in the same pid namespace. This
solved several use cases in that time. However today we face new
problems, there are mutliple container implementations there, some of
them want to hide pid entries, others want to hide non-pid entries,
others want to have sysctlfs, others want to share pid namespace with
private procfs mounts. All these with current implementation won't work
since all options will be propagated to all procfs mounts.

This series allow to have new instances of procfs per pid namespace where
each instance can have its own mount option inside the same pid namespace.
This was also suggested by Andy Lutomirski.


Now:
$ sudo mount -t proc -o unshare,hidepid=2 none /test

The option 'unshare' will allow to mount a new instance of procfs inside
the same pid namespace.

Before:
$ stat /proc/slabinfo

  File: ‘/proc/slabinfo’
  Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
Device: 4h/4d	Inode: 4026532046  Links: 1

$ stat /test3/slabinfo

  File: ‘/test3/slabinfo’
  Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
Device: 4h/4d	Inode: 4026532046  Links: 1


After:
$ stat /proc/slabinfo

  File: ‘/proc/slabinfo’
  Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
Device: 4h/4d	Inode: 4026532046  Links: 1

$ stat /test3/slabinfo

  File: ‘/test3/slabinfo’
  Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
Device: 31h/49d	Inode: 4026532046  Links: 1


Any better name for the option 'unshare' ? suggestions ?

I was going to use 'version=2' but then this may sound more like a
proc2 fs which currently impossible to implement since it will share
locks with the old proc.


Al, Eric any comments please ?

[Patch RFC 4/4] proc: support flushing dcache entries of a task on multiple procfs mounts
Is maybe not needed, and I have to test it further.

Thanks!

[toc] | [next] | [standalone]

#1613178 — [PATCH RFC 3/4] proc: support mounting new procfs instances inside same pid namespace

This patch adds support for 'unshare' mount option to have multiple
separated procfs inside the same pid namespace. This allows to solve lot
of problem for containers and their specific use cases.

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
 fs/proc/generic.c       | 10 +++++++++
 fs/proc/inode.c         |  3 +++
 fs/proc/root.c          | 59 +++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/proc_fs.h | 12 ++++++++++
 4 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 7e5e419..7ae5377 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -52,6 +52,11 @@ void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid)
 		fs_info->pid_gid = gid;
 }
 
+void proc_fs_set_unshare(struct proc_fs_info *fs_info, int version)
+{
+	fs_info->version = version;
+}
+
 int proc_fs_get_hide_pid(struct proc_fs_info *fs_info)
 {
 	/* For backward compatibility */
@@ -70,6 +75,11 @@ kgid_t proc_fs_get_pid_gid(struct proc_fs_info *fs_info)
 	return fs_info->pid_gid;
 }
 
+int proc_fs_get_unshare(struct proc_fs_info *fs_info)
+{
+	return fs_info->version;
+}
+
 static int proc_match(unsigned int len, const char *name, struct proc_dir_entry *de)
 {
 	if (len < de->namelen)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index ca47a0a..5f7557d 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -111,6 +111,9 @@ static int proc_show_options(struct seq_file *seq, struct dentry *root)
 	if (pid->hide_pid != HIDEPID_OFF)
 		seq_printf(seq, ",hidepid=%u", pid->hide_pid);
 
+	if (proc_fs_get_unshare(fs_info) == PROC_FS_V2)
+		seq_printf(seq, ",unshare");
+
 	return 0;
 }
 
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 6a96c02..7a8f425 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -27,15 +27,52 @@
 #include "internal.h"
 
 enum {
-	Opt_gid, Opt_hidepid, Opt_err,
+	Opt_gid, Opt_hidepid, Opt_unshare, Opt_err,
 };
 
 static const match_table_t tokens = {
 	{Opt_hidepid, "hidepid=%u"},
 	{Opt_gid, "gid=%u"},
+	{Opt_unshare, "unshare"},
 	{Opt_err, NULL},
 };
 
+/* We only parse 'unshare' option here */
+int proc_parse_early_options(char *options, struct proc_fs_info *fs_info)
+{
+	char *p, *opts, *orig;
+	substring_t args[MAX_OPT_ARGS];
+
+	if (!options)
+		return 0;
+
+	opts = kstrdup(options, GFP_KERNEL);
+	if (!opts)
+		return -ENOMEM;
+
+	orig = opts;
+
+	while ((p = strsep(&opts, ",")) != NULL) {
+		int token;
+
+		if (!*p)
+			continue;
+
+		token = match_token(p, tokens, args);
+		switch (token) {
+		case Opt_unshare:
+			pr_info("proc: mounting a new procfs instance ");
+			proc_fs_set_unshare(fs_info, PROC_FS_V2);
+			break;
+		default:
+			break;
+		}
+	}
+
+	kfree(orig);
+	return 0;
+}
+
 int proc_parse_options(char *options, struct proc_fs_info *fs_info)
 {
 	char *p;
@@ -70,6 +107,8 @@ int proc_parse_options(char *options, struct proc_fs_info *fs_info)
 			}
 			proc_fs_set_hide_pid(fs_info, option);
 			break;
+		case Opt_unshare:
+			break;
 		default:
 			pr_err("proc: unrecognized mount option \"%s\" "
 			       "or missing value\n", p);
@@ -82,9 +121,19 @@ int proc_parse_options(char *options, struct proc_fs_info *fs_info)
 
 int proc_remount(struct super_block *sb, int *flags, char *data)
 {
+	int error, version;
 	struct proc_fs_info *fs_info = proc_sb(sb);
 
+	version = proc_fs_get_unshare(fs_info);
+
 	sync_filesystem(sb);
+
+	if (version == PROC_FS_V2) {
+		error = proc_parse_early_options(data, fs_info);
+		if (error < 0)
+			return error;
+	}
+
 	return !proc_parse_options(data, fs_info);
 }
 
@@ -122,15 +171,21 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
 	if (!fs_info)
 		return ERR_PTR(-ENOMEM);
 
+	/* Set it as early as possible */
+	proc_fs_set_unshare(fs_info, PROC_FS_V1);
+
 	if (flags & MS_KERNMOUNT) {
 		ns = data;
 		data = NULL;
 	} else {
+		error = proc_parse_early_options(data, fs_info);
+		if (error < 0)
+			goto error_fs_info;
+
 		ns = task_active_pid_ns(current);
 	}
 
 	fs_info->pid_ns = ns;
-	fs_info->version = PROC_FS_V1;
 	fs_info->hide_pid = HIDEPID_OFF;
 	fs_info->pid_gid = GLOBAL_ROOT_GID;
 	refcount_set(&fs_info->users, 1);
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index c23299d..e3a78a5 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -31,8 +31,11 @@ extern void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_pid);
 
 extern void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid);
 
+extern void proc_fs_set_unshare(struct proc_fs_info *fs_info, int version);
+
 extern int proc_fs_get_hide_pid(struct proc_fs_info *fs_info);
 extern kgid_t proc_fs_get_pid_gid(struct proc_fs_info *fs_info);
+extern int proc_fs_get_unshare(struct proc_fs_info *fs_info);
 
 extern void proc_root_init(void);
 extern void proc_flush_task(struct task_struct *);
@@ -84,6 +87,10 @@ static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_p
 {
 }
 
+static inline void proc_fs_set_unshare(struct proc_fs_info *fs_info, int version)
+{
+}
+
 static inline void proc_fs_set_pid_gid(struct proc_info_fs *fs_info, kgid_t gid)
 {
 }
@@ -98,6 +105,11 @@ extern kgid_t proc_fs_get_pid_gid(struct proc_fs_info *fs_info)
 	return GLOBAL_ROOT_GID;
 }
 
+static inline int proc_fs_get_unshare(struct proc_fs_info *fs_info)
+{
+	return PROC_FS_V1;
+}
+
 extern inline struct proc_fs_info *proc_sb(struct super_block *sb) { return NULL;}
 static inline struct proc_dir_entry *proc_symlink(const char *name,
 		struct proc_dir_entry *parent,const char *dest) { return NULL;}
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1613386 — Re: [PATCH RFC 3/4] proc: support mounting new procfs instances inside same pid namespace

On Thu, Mar 30, 2017 at 8:22 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
> This patch adds support for 'unshare' mount option to have multiple
> separated procfs inside the same pid namespace. This allows to solve lot
> of problem for containers and their specific use cases.

It would be nice if we could make this work without 'unshare'.  How about:

hidepid still sets pid_ns->hidepid.  "this_mount_hidepid" (or whatever
you want to call it), if set, overrides pid_ns->hidepid.

--Andy

>
> Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
> ---
>  fs/proc/generic.c       | 10 +++++++++
>  fs/proc/inode.c         |  3 +++
>  fs/proc/root.c          | 59 +++++++++++++++++++++++++++++++++++++++++++++++--
>  include/linux/proc_fs.h | 12 ++++++++++
>  4 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index 7e5e419..7ae5377 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -52,6 +52,11 @@ void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid)
>                 fs_info->pid_gid = gid;
>  }
>
> +void proc_fs_set_unshare(struct proc_fs_info *fs_info, int version)
> +{
> +       fs_info->version = version;
> +}
> +
>  int proc_fs_get_hide_pid(struct proc_fs_info *fs_info)
>  {
>         /* For backward compatibility */
> @@ -70,6 +75,11 @@ kgid_t proc_fs_get_pid_gid(struct proc_fs_info *fs_info)
>         return fs_info->pid_gid;
>  }
>
> +int proc_fs_get_unshare(struct proc_fs_info *fs_info)
> +{
> +       return fs_info->version;
> +}
> +
>  static int proc_match(unsigned int len, const char *name, struct proc_dir_entry *de)
>  {
>         if (len < de->namelen)
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index ca47a0a..5f7557d 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -111,6 +111,9 @@ static int proc_show_options(struct seq_file *seq, struct dentry *root)
>         if (pid->hide_pid != HIDEPID_OFF)
>                 seq_printf(seq, ",hidepid=%u", pid->hide_pid);
>
> +       if (proc_fs_get_unshare(fs_info) == PROC_FS_V2)
> +               seq_printf(seq, ",unshare");
> +
>         return 0;
>  }
>
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index 6a96c02..7a8f425 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -27,15 +27,52 @@
>  #include "internal.h"
>
>  enum {
> -       Opt_gid, Opt_hidepid, Opt_err,
> +       Opt_gid, Opt_hidepid, Opt_unshare, Opt_err,
>  };
>
>  static const match_table_t tokens = {
>         {Opt_hidepid, "hidepid=%u"},
>         {Opt_gid, "gid=%u"},
> +       {Opt_unshare, "unshare"},
>         {Opt_err, NULL},
>  };
>
> +/* We only parse 'unshare' option here */
> +int proc_parse_early_options(char *options, struct proc_fs_info *fs_info)
> +{
> +       char *p, *opts, *orig;
> +       substring_t args[MAX_OPT_ARGS];
> +
> +       if (!options)
> +               return 0;
> +
> +       opts = kstrdup(options, GFP_KERNEL);
> +       if (!opts)
> +               return -ENOMEM;
> +
> +       orig = opts;
> +
> +       while ((p = strsep(&opts, ",")) != NULL) {
> +               int token;
> +
> +               if (!*p)
> +                       continue;
> +
> +               token = match_token(p, tokens, args);
> +               switch (token) {
> +               case Opt_unshare:
> +                       pr_info("proc: mounting a new procfs instance ");
> +                       proc_fs_set_unshare(fs_info, PROC_FS_V2);
> +                       break;
> +               default:
> +                       break;
> +               }
> +       }
> +
> +       kfree(orig);
> +       return 0;
> +}
> +
>  int proc_parse_options(char *options, struct proc_fs_info *fs_info)
>  {
>         char *p;
> @@ -70,6 +107,8 @@ int proc_parse_options(char *options, struct proc_fs_info *fs_info)
>                         }
>                         proc_fs_set_hide_pid(fs_info, option);
>                         break;
> +               case Opt_unshare:
> +                       break;
>                 default:
>                         pr_err("proc: unrecognized mount option \"%s\" "
>                                "or missing value\n", p);
> @@ -82,9 +121,19 @@ int proc_parse_options(char *options, struct proc_fs_info *fs_info)
>
>  int proc_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       int error, version;
>         struct proc_fs_info *fs_info = proc_sb(sb);
>
> +       version = proc_fs_get_unshare(fs_info);
> +
>         sync_filesystem(sb);
> +
> +       if (version == PROC_FS_V2) {
> +               error = proc_parse_early_options(data, fs_info);
> +               if (error < 0)
> +                       return error;
> +       }
> +
>         return !proc_parse_options(data, fs_info);
>  }
>
> @@ -122,15 +171,21 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
>         if (!fs_info)
>                 return ERR_PTR(-ENOMEM);
>
> +       /* Set it as early as possible */
> +       proc_fs_set_unshare(fs_info, PROC_FS_V1);
> +
>         if (flags & MS_KERNMOUNT) {
>                 ns = data;
>                 data = NULL;
>         } else {
> +               error = proc_parse_early_options(data, fs_info);
> +               if (error < 0)
> +                       goto error_fs_info;
> +
>                 ns = task_active_pid_ns(current);
>         }
>
>         fs_info->pid_ns = ns;
> -       fs_info->version = PROC_FS_V1;
>         fs_info->hide_pid = HIDEPID_OFF;
>         fs_info->pid_gid = GLOBAL_ROOT_GID;
>         refcount_set(&fs_info->users, 1);
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index c23299d..e3a78a5 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -31,8 +31,11 @@ extern void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_pid);
>
>  extern void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid);
>
> +extern void proc_fs_set_unshare(struct proc_fs_info *fs_info, int version);
> +
>  extern int proc_fs_get_hide_pid(struct proc_fs_info *fs_info);
>  extern kgid_t proc_fs_get_pid_gid(struct proc_fs_info *fs_info);
> +extern int proc_fs_get_unshare(struct proc_fs_info *fs_info);
>
>  extern void proc_root_init(void);
>  extern void proc_flush_task(struct task_struct *);
> @@ -84,6 +87,10 @@ static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_p
>  {
>  }
>
> +static inline void proc_fs_set_unshare(struct proc_fs_info *fs_info, int version)
> +{
> +}
> +
>  static inline void proc_fs_set_pid_gid(struct proc_info_fs *fs_info, kgid_t gid)
>  {
>  }
> @@ -98,6 +105,11 @@ extern kgid_t proc_fs_get_pid_gid(struct proc_fs_info *fs_info)
>         return GLOBAL_ROOT_GID;
>  }
>
> +static inline int proc_fs_get_unshare(struct proc_fs_info *fs_info)
> +{
> +       return PROC_FS_V1;
> +}
> +
>  extern inline struct proc_fs_info *proc_sb(struct super_block *sb) { return NULL;}
>  static inline struct proc_dir_entry *proc_symlink(const char *name,
>                 struct proc_dir_entry *parent,const char *dest) { return NULL;}
> --
> 2.10.2
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1613385 — Re: [PATCH RFC 0/4] proc: support multiple separate proc instances per pidnamespace

On Thu, Mar 30, 2017 at 8:22 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
> Hi,
>
> This RFC can be applied on top of Linus' tree 89970a04d7
>
> This RFC implements support for multiple separate proc instances inside
> the same pid namespace. This allows to solve lot of problems that
> today's use case face.
>
> Historically procfs was tied to pid namespaces, and mount options were
> propagated to all other procfs instances in the same pid namespace. This
> solved several use cases in that time. However today we face new
> problems, there are mutliple container implementations there, some of
> them want to hide pid entries, others want to hide non-pid entries,
> others want to have sysctlfs, others want to share pid namespace with
> private procfs mounts. All these with current implementation won't work
> since all options will be propagated to all procfs mounts.
>
> This series allow to have new instances of procfs per pid namespace where
> each instance can have its own mount option inside the same pid namespace.
> This was also suggested by Andy Lutomirski.
>
>
> Now:
> $ sudo mount -t proc -o unshare,hidepid=2 none /test
>
> The option 'unshare' will allow to mount a new instance of procfs inside
> the same pid namespace.
>
> Before:
> $ stat /proc/slabinfo
>
>   File: ‘/proc/slabinfo’
>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
> Device: 4h/4d   Inode: 4026532046  Links: 1
>
> $ stat /test3/slabinfo
>
>   File: ‘/test3/slabinfo’
>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
> Device: 4h/4d   Inode: 4026532046  Links: 1
>
>
> After:
> $ stat /proc/slabinfo
>
>   File: ‘/proc/slabinfo’
>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
> Device: 4h/4d   Inode: 4026532046  Links: 1
>
> $ stat /test3/slabinfo
>
>   File: ‘/test3/slabinfo’
>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
> Device: 31h/49d Inode: 4026532046  Links: 1
>
>
> Any better name for the option 'unshare' ? suggestions ?
>
> I was going to use 'version=2' but then this may sound more like a
> proc2 fs which currently impossible to implement since it will share
> locks with the old proc.
>
>
> Al, Eric any comments please ?

I like the concept, except that I think it would be nice to avoid
needing 'unshare', perhaps by making unsharing the default and making
hidepid work backwards compatibly if needed.

[toc] | [prev] | [next] | [standalone]

#1613923 — Re: [PATCH RFC 0/4] proc: support multiple separate proc instances per pidnamespace

On Thu, Mar 30, 2017 at 9:12 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Mar 30, 2017 at 8:22 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
>> Hi,
>>
>> This RFC can be applied on top of Linus' tree 89970a04d7
>>
>> This RFC implements support for multiple separate proc instances inside
>> the same pid namespace. This allows to solve lot of problems that
>> today's use case face.
>>
>> Historically procfs was tied to pid namespaces, and mount options were
>> propagated to all other procfs instances in the same pid namespace. This
>> solved several use cases in that time. However today we face new
>> problems, there are mutliple container implementations there, some of
>> them want to hide pid entries, others want to hide non-pid entries,
>> others want to have sysctlfs, others want to share pid namespace with
>> private procfs mounts. All these with current implementation won't work
>> since all options will be propagated to all procfs mounts.
>>
>> This series allow to have new instances of procfs per pid namespace where
>> each instance can have its own mount option inside the same pid namespace.
>> This was also suggested by Andy Lutomirski.
>>
>>
>> Now:
>> $ sudo mount -t proc -o unshare,hidepid=2 none /test
>>
>> The option 'unshare' will allow to mount a new instance of procfs inside
>> the same pid namespace.
>>
>> Before:
>> $ stat /proc/slabinfo
>>
>>   File: ‘/proc/slabinfo’
>>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
>> Device: 4h/4d   Inode: 4026532046  Links: 1
>>
>> $ stat /test3/slabinfo
>>
>>   File: ‘/test3/slabinfo’
>>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
>> Device: 4h/4d   Inode: 4026532046  Links: 1
>>
>>
>> After:
>> $ stat /proc/slabinfo
>>
>>   File: ‘/proc/slabinfo’
>>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
>> Device: 4h/4d   Inode: 4026532046  Links: 1
>>
>> $ stat /test3/slabinfo
>>
>>   File: ‘/test3/slabinfo’
>>   Size: 0               Blocks: 0          IO Block: 1024   regular empty file
>> Device: 31h/49d Inode: 4026532046  Links: 1
>>
>>
>> Any better name for the option 'unshare' ? suggestions ?
>>
>> I was going to use 'version=2' but then this may sound more like a
>> proc2 fs which currently impossible to implement since it will share
>> locks with the old proc.
>>
>>
>> Al, Eric any comments please ?
>
> I like the concept, except that I think it would be nice to avoid
> needing 'unshare', perhaps by making unsharing the default and making
> hidepid work backwards compatibly if needed.

Of course I can update, but as said lot of stuff may already depend on
the behaviour of procfs mount options, and the device ID inside same
pid namespace where is it is always the same ID. With this change we
will have different IDs inside same pid namespace, and I don't have
the knowledge to predict if something will break. I remember you said
that we may update stat() to show same ID but I really don't know if
that's a good idea, what if userspace now tries to check if these are
two separate proc mounts ? also maybe there is third party kernel code
that does something with these device IDs ?

For the namespace introspection, Eric suggested that we may stat /proc
before stating /proc/pid/ns/x , however he also concluded that some
security or other obscure stuff may break! I basically don't have the
resources nor the knowledge to predict what errors may come due to
changing the default behaviour of procfs. From my position it is
better to introduce a new option for it so users are warned. Is this
reasonable ? or do we have to make it default ?

Thanks!

-- 
tixxdz

[toc] | [prev] | [next] | [standalone]

#1613511 — Re: [PATCH RFC 0/4] proc: support multiple separate proc instances per pidnamespace

On Thu, Mar 30, 2017 at 05:22:55PM +0200, Djalal Harouni wrote:
> Hi,
> 
> This RFC can be applied on top of Linus' tree 89970a04d7
> 
> This RFC implements support for multiple separate proc instances inside
> the same pid namespace. This allows to solve lot of problems that
> today's use case face.
> 
> Historically procfs was tied to pid namespaces, and mount options were
> propagated to all other procfs instances in the same pid namespace. This
> solved several use cases in that time. However today we face new
> problems, there are mutliple container implementations there, some of
> them want to hide pid entries, others want to hide non-pid entries,
> others want to have sysctlfs, others want to share pid namespace with
> private procfs mounts. All these with current implementation won't work
> since all options will be propagated to all procfs mounts.
> 
> This series allow to have new instances of procfs per pid namespace where
> each instance can have its own mount option inside the same pid namespace.
> This was also suggested by Andy Lutomirski.
> 
> 
> Now:
> $ sudo mount -t proc -o unshare,hidepid=2 none /test
> 
> The option 'unshare' will allow to mount a new instance of procfs inside
> the same pid namespace.
> 
> Before:
> $ stat /proc/slabinfo
> 
>   File: ‘/proc/slabinfo’
>   Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
> Device: 4h/4d	Inode: 4026532046  Links: 1
> 
> $ stat /test3/slabinfo
> 
>   File: ‘/test3/slabinfo’
>   Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
> Device: 4h/4d	Inode: 4026532046  Links: 1
> 
> 
> After:
> $ stat /proc/slabinfo
> 
>   File: ‘/proc/slabinfo’
>   Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
> Device: 4h/4d	Inode: 4026532046  Links: 1
> 
> $ stat /test3/slabinfo
> 
>   File: ‘/test3/slabinfo’
>   Size: 0         	Blocks: 0          IO Block: 1024   regular empty file
> Device: 31h/49d	Inode: 4026532046  Links: 1
> 
> 
> Any better name for the option 'unshare' ? suggestions ?
> 
> I was going to use 'version=2' but then this may sound more like a
> proc2 fs which currently impossible to implement since it will share
> locks with the old proc.
> 
> 
> Al, Eric any comments please ?

Multiple mnt_root's lead us to significant memory costs for storing dentry
of tasks. I mean what we will get as many copies of the tasks dentry as many
times we have mounted the procfs with 'unshare' flag. No?

-- 
Rgrds, legion

[toc] | [prev] | [next] | [standalone]

#1613887 — Re: [PATCH RFC 0/4] proc: support multiple separate proc instances per pidnamespace

On Fri, Mar 31, 2017 at 12:16 AM, Alexey Gladkov
<gladkov.alexey@gmail.com> wrote:
> On Thu, Mar 30, 2017 at 05:22:55PM +0200, Djalal Harouni wrote:
>> Hi,
>>
>> This RFC can be applied on top of Linus' tree 89970a04d7
>>
>> This RFC implements support for multiple separate proc instances inside
>> the same pid namespace. This allows to solve lot of problems that
>> today's use case face.
>>
>> Historically procfs was tied to pid namespaces, and mount options were
>> propagated to all other procfs instances in the same pid namespace. This
>> solved several use cases in that time. However today we face new
>> problems, there are mutliple container implementations there, some of
>> them want to hide pid entries, others want to hide non-pid entries,
>> others want to have sysctlfs, others want to share pid namespace with
>> private procfs mounts. All these with current implementation won't work
>> since all options will be propagated to all procfs mounts.
>>
>> This series allow to have new instances of procfs per pid namespace where
>> each instance can have its own mount option inside the same pid namespace.
>> This was also suggested by Andy Lutomirski.
>>
>>
>> Now:
>> $ sudo mount -t proc -o unshare,hidepid=2 none /test
>>
>> The option 'unshare' will allow to mount a new instance of procfs inside
>> the same pid namespace.
>>
>> Before:
>> $ stat /proc/slabinfo
>>
>>   File: ‘/proc/slabinfo’
>>   Size: 0             Blocks: 0          IO Block: 1024   regular empty file
>> Device: 4h/4d Inode: 4026532046  Links: 1
>>
>> $ stat /test3/slabinfo
>>
>>   File: ‘/test3/slabinfo’
>>   Size: 0             Blocks: 0          IO Block: 1024   regular empty file
>> Device: 4h/4d Inode: 4026532046  Links: 1
>>
>>
>> After:
>> $ stat /proc/slabinfo
>>
>>   File: ‘/proc/slabinfo’
>>   Size: 0             Blocks: 0          IO Block: 1024   regular empty file
>> Device: 4h/4d Inode: 4026532046  Links: 1
>>
>> $ stat /test3/slabinfo
>>
>>   File: ‘/test3/slabinfo’
>>   Size: 0             Blocks: 0          IO Block: 1024   regular empty file
>> Device: 31h/49d       Inode: 4026532046  Links: 1
>>
>>
>> Any better name for the option 'unshare' ? suggestions ?
>>
>> I was going to use 'version=2' but then this may sound more like a
>> proc2 fs which currently impossible to implement since it will share
>> locks with the old proc.
>>
>>
>> Al, Eric any comments please ?
>
> Multiple mnt_root's lead us to significant memory costs for storing dentry
> of tasks. I mean what we will get as many copies of the tasks dentry as many
> times we have mounted the procfs with 'unshare' flag. No?

With current implementation, that's true. However I think that we
should not sacrifice usage for optimization, currently it is
practically impossible to improve procfs, support new options or make
use of the current ones without affecting other procfs mounts. Andy
also suggested to have a mini-proc without non-pid stuff inside, and
without a new disconnected instance, new mounts or bind mounts may
expose the non-pid stuff.

Also we can improve this, right now it is not implemented but we may
can change how we do lookups, instead of doing a ptrace task after
instantiating a pid dentry we may do a ptrace permission check on task
there then create its related proc inode. With this all new procfs
instances with hidepid option set, will only have dentries of tasks
that the caller can ptrace. Also there is already the code to flush
the related task when it dies, tough, it needs further testing.

Also as with tmpfs where inodes are accounted by the memory
controller, I'm not sure if it's possible to account the same in
procfs during the first access ?

I don't see a better way to solve the current procfs problems that we
face or how to modernize it and add new options... in the end users
can always chose to use it or not.

-- 
tixxdz

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web