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

Groups > linux.kernel > #1584027 > unrolled thread

[PATCH] Add pidfs filesystem

Back to article view | Back to linux.kernel

Contents

  [PATCH] Add pidfs filesystem Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-02-19 00:00 +0100
    Re: [PATCH] Add pidfs filesystem ebiederm@xmission.com (Eric W. Biederman) - 2017-02-20 05:20 +0100
      Re: [PATCH] Add pidfs filesystem Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-02-20 11:40 +0100
      Re: [PATCH] Add pidfs filesystem Richard Weinberger <richard.weinberger@gmail.com> - 2017-02-22 21:20 +0100
    Re: [PATCH] Add pidfs filesystem Oleg Nesterov <oleg@redhat.com> - 2017-02-21 16:00 +0100
      Re: [PATCH] Add pidfs filesystem Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-02-22 12:50 +0100
      Re: [PATCH] Add pidfs filesystem Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-02-22 13:00 +0100
      Re: [PATCH] Add pidfs filesystem "Dmitry V. Levin" <ldv@altlinux.org> - 2017-02-22 16:50 +0100
        Re: [PATCH] Add pidfs filesystem Oleg Nesterov <oleg@redhat.com> - 2017-02-22 18:50 +0100
          Re: [PATCH] Add pidfs filesystem Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-02-22 20:50 +0100
    Re: [PATCH] Add pidfs filesystem Michael Kerrisk <mtk.manpages@gmail.com> - 2017-02-27 20:00 +0100

#1584027 — [PATCH] Add pidfs filesystem

The pidfs filesystem contains a subset of the /proc file system which
contains only information about the processes.

Some of the container virtualization systems are mounted /proc inside
the container. This is done in most cases to operate with information
about the processes. Knowing that /proc filesystem is not fully
virtualized they are mounted on top of dangerous places empty files or
directories (for exmaple /proc/kcore, /sys/firmware, etc.).

The structure of this filesystem is dynamic and any module can create a
new object which will not necessarily be virtualized. There are
proprietary modules that aren't in the mainline whose work we can not
verify.

This opens up a potential threat to the system. The developers of the
virtualization system can't predict all dangerous places in /proc by
definition.

A more effective solution would be to mount into the container only what
is necessary and ignore the rest.

Right now there is the opportunity to pass in the container any port of
the /proc filesystem using mount --bind expect the pids.

This patch allows to mount only the part of /proc related to pids
without rest objects. Since this is an addon to /proc, flags applied to
/proc have an effect on this pidfs filesystem.

Why not implement it as another flag to /proc ?

The /proc flags is stored in the pid_namespace and are global for
namespace. It means that if you add a flag to hide all except the pids,
then it will act on all mounted instances of /proc.

Originally the idea was that the container will be mounted only pidfs
and additional required files will be mounted on top using the
overlayfs. But I found out that /proc does not support overlayfs and
does not allow to mount anything on top or under it.

My question is whether it's possible to add overlayfs support for /proc?

Cc: Kirill A. Shutemov <kirill@shutemov.name>
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
 Documentation/filesystems/pidfs.txt | 16 ++++++++
 fs/proc/Kconfig                     |  8 ++++
 fs/proc/inode.c                     |  8 +++-
 fs/proc/internal.h                  |  2 +
 fs/proc/root.c                      | 76 ++++++++++++++++++++++++++++++++++---
 fs/proc/self.c                      |  6 +++
 fs/proc/thread_self.c               |  6 +++
 include/linux/pid_namespace.h       |  5 +++
 8 files changed, 119 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/filesystems/pidfs.txt

diff --git a/Documentation/filesystems/pidfs.txt b/Documentation/filesystems/pidfs.txt
new file mode 100644
index 0000000..ce958a5
--- /dev/null
+++ b/Documentation/filesystems/pidfs.txt
@@ -0,0 +1,16 @@
+The PIDFS Filesystem
+====================
+
+The pidfs filesystem contains a subset of the /proc file system which contains
+only information about the processes. The link self points to the process
+reading the file system. All other special files and directories in /proc are
+not available in this filesystem.
+
+The pidfs is not an independent filesystem, its implementation shares code
+with /proc.
+
+All mount options applicable to /proc filesystem are also applicable
+to pidfs filesystem. For example, access to the information in /proc/[pid]
+directories can be restricted using hidepid option.
+
+To get more information about the processes read the proc.txt
diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
index 1ade120..fa568f6 100644
--- a/fs/proc/Kconfig
+++ b/fs/proc/Kconfig
@@ -43,6 +43,14 @@ config PROC_VMCORE
         help
         Exports the dump image of crashed kernel in ELF format.
 
+config PROC_PIDFS
+	bool "pidfs file system support"
+	depends on PROC_FS
+	default n
+	help
+	  The pidfs filesystem contains a subset of the /proc file system
+	  which contains only information only about the processes.
+
 config PROC_SYSCTL
 	bool "Sysctl support (/proc/sys)" if EXPERT
 	depends on PROC_FS
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 783bc19..1be65b4 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -474,12 +474,16 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
 int proc_fill_super(struct super_block *s, void *data, int silent)
 {
 	struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
+	struct proc_dir_entry *fs_root = &proc_root;
 	struct inode *root_inode;
 	int ret;
 
 	if (!proc_parse_options(data, ns))
 		return -EINVAL;
 
+	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type)
+		fs_root = &pidfs_root;
+
 	/* User space would break if executables or devices appear on proc */
 	s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
 	s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC;
@@ -496,8 +500,8 @@ int proc_fill_super(struct super_block *s, void *data, int silent)
 	 */
 	s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
 	
-	pde_get(&proc_root);
-	root_inode = proc_get_inode(s, &proc_root);
+	pde_get(fs_root);
+	root_inode = proc_get_inode(s, fs_root);
 	if (!root_inode) {
 		pr_err("proc_fill_super: get root inode failed\n");
 		return -ENOMEM;
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 2de5194..a7c068c 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -267,6 +267,8 @@ static inline void proc_tty_init(void) {}
 /*
  * root.c
  */
+extern struct file_system_type pidfs_fs_type;
+extern struct proc_dir_entry pidfs_root;
 extern struct proc_dir_entry proc_root;
 extern int proc_parse_options(char *options, struct pid_namespace *pid);
 
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 4bd0373..de16ac1 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -102,10 +102,21 @@ static void proc_kill_sb(struct super_block *sb)
 	struct pid_namespace *ns;
 
 	ns = (struct pid_namespace *)sb->s_fs_info;
-	if (ns->proc_self)
-		dput(ns->proc_self);
-	if (ns->proc_thread_self)
-		dput(ns->proc_thread_self);
+
+	if (IS_ENABLED(CONFIG_PROC_PIDFS) && sb->s_type == &pidfs_fs_type) {
+		if (ns->pidfs_self)
+			dput(ns->pidfs_self);
+
+		if (ns->pidfs_thread_self)
+			dput(ns->pidfs_thread_self);
+	} else {
+		if (ns->proc_self)
+			dput(ns->proc_self);
+
+		if (ns->proc_thread_self)
+			dput(ns->proc_thread_self);
+	}
+
 	kill_anon_super(sb);
 	put_pid_ns(ns);
 }
@@ -117,6 +128,13 @@ static struct file_system_type proc_fs_type = {
 	.fs_flags	= FS_USERNS_MOUNT,
 };
 
+struct file_system_type pidfs_fs_type = {
+	.name		= "pidfs",
+	.mount		= proc_mount,
+	.kill_sb	= proc_kill_sb,
+	.fs_flags	= FS_USERNS_MOUNT,
+};
+
 void __init proc_root_init(void)
 {
 	int err;
@@ -127,6 +145,10 @@ void __init proc_root_init(void)
 	if (err)
 		return;
 
+	err = register_filesystem(&pidfs_fs_type);
+	if (err)
+		return;
+
 	proc_self_init();
 	proc_thread_self_init();
 	proc_symlink("mounts", NULL, "self/mounts");
@@ -148,8 +170,7 @@ void __init proc_root_init(void)
 	proc_sys_init();
 }
 
-static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat
-)
+static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
 {
 	generic_fillattr(d_inode(dentry), stat);
 	stat->nlink = proc_root.nlink + nr_processes();
@@ -176,6 +197,14 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
 	return proc_pid_readdir(file, ctx);
 }
 
+static int pidfs_root_readdir(struct file *file, struct dir_context *ctx)
+{
+	if (ctx->pos < FIRST_PROCESS_ENTRY)
+		ctx->pos = FIRST_PROCESS_ENTRY;
+
+	return proc_pid_readdir(file, ctx);
+}
+
 /*
  * The root /proc directory is special, as it has the
  * <pid> directories. Thus we don't use the generic
@@ -187,6 +216,12 @@ static const struct file_operations proc_root_operations = {
 	.llseek		= generic_file_llseek,
 };
 
+static const struct file_operations pidfs_root_operations = {
+	.read		 = generic_read_dir,
+	.iterate_shared	 = pidfs_root_readdir,
+	.llseek		= generic_file_llseek,
+};
+
 /*
  * proc root can do almost nothing..
  */
@@ -195,6 +230,11 @@ static const struct inode_operations proc_root_inode_operations = {
 	.getattr	= proc_root_getattr,
 };
 
+static const struct inode_operations pidfs_root_inode_operations = {
+	.lookup		= proc_pid_lookup,
+	.getattr	= proc_root_getattr,
+};
+
 /*
  * This is the root "inode" in the /proc tree..
  */
@@ -211,6 +251,19 @@ struct proc_dir_entry proc_root = {
 	.name		= "/proc",
 };
 
+struct proc_dir_entry pidfs_root = {
+	.low_ino	= PROC_ROOT_INO,
+	.namelen	= 6,
+	.mode		= S_IFDIR | S_IRUGO | S_IXUGO,
+	.nlink		= 2,
+	.count		= ATOMIC_INIT(1),
+	.proc_iops	= &pidfs_root_inode_operations,
+	.proc_fops	= &pidfs_root_operations,
+	.parent		= &pidfs_root,
+	.subdir		= RB_ROOT,
+	.name		= "/pidfs",
+};
+
 int pid_ns_prepare_proc(struct pid_namespace *ns)
 {
 	struct vfsmount *mnt;
@@ -220,10 +273,21 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
 		return PTR_ERR(mnt);
 
 	ns->proc_mnt = mnt;
+
+	if (IS_ENABLED(CONFIG_PROC_PIDFS)) {
+		mnt = kern_mount_data(&pidfs_fs_type, ns);
+		if (IS_ERR(mnt))
+			return PTR_ERR(mnt);
+
+		ns->pidfs_mnt = mnt;
+	}
 	return 0;
 }
 
 void pid_ns_release_proc(struct pid_namespace *ns)
 {
 	kern_unmount(ns->proc_mnt);
+
+	if (IS_ENABLED(CONFIG_PROC_PIDFS))
+		kern_unmount(ns->pidfs_mnt);
 }
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 4024595..dea7e17 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -74,6 +74,12 @@ int proc_setup_self(struct super_block *s)
 		pr_err("proc_fill_super: can't allocate /proc/self\n");
 		return PTR_ERR(self);
 	}
+
+	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
+		ns->pidfs_self = self;
+		return 0;
+	}
+
 	ns->proc_self = self;
 	return 0;
 }
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index 595b90a97..274c618 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -76,6 +76,12 @@ int proc_setup_thread_self(struct super_block *s)
 		pr_err("proc_fill_super: can't allocate /proc/thread_self\n");
 		return PTR_ERR(thread_self);
 	}
+
+	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
+		ns->pidfs_thread_self = thread_self;
+		return 0;
+	}
+
 	ns->proc_thread_self = thread_self;
 	return 0;
 }
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 34cce96..fca3a76 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -46,6 +46,11 @@ struct pid_namespace {
 	int hide_pid;
 	int reboot;	/* group exit code if this pidns was rebooted */
 	struct ns_common ns;
+#ifdef CONFIG_PROC_PIDFS
+	struct vfsmount *pidfs_mnt;
+	struct dentry *pidfs_self;
+	struct dentry *pidfs_thread_self;
+#endif
 };
 
 extern struct pid_namespace init_pid_ns;
-- 
2.10.2


-- 
Rgrds, legion

[toc] | [next] | [standalone]

#1584337

Alexey Gladkov <gladkov.alexey@gmail.com> writes:

> The pidfs filesystem contains a subset of the /proc file system which
> contains only information about the processes.

My summary of your motivation.

It hurts when I create a container with a processes with uid 0 inside of
it.  This generates lots of hacks to attempt to limit uid 0.

  My answer:  Don't run a container with a real uid 0 inside of it.

Any reasonable permission check will on proc files
will keep you safe if your container does not have a real uid 0 in it.


That said I am not opposed in principle to a pidfs.  And the idea of
using overlay for this purpose is intriguing.  I have not looked at it
in enough detail comment on the technical merits.

Eric


> Some of the container virtualization systems are mounted /proc inside
> the container. This is done in most cases to operate with information
> about the processes. Knowing that /proc filesystem is not fully
> virtualized they are mounted on top of dangerous places empty files or
> directories (for exmaple /proc/kcore, /sys/firmware, etc.).
>
> The structure of this filesystem is dynamic and any module can create a
> new object which will not necessarily be virtualized. There are
> proprietary modules that aren't in the mainline whose work we can not
> verify.
>
> This opens up a potential threat to the system. The developers of the
> virtualization system can't predict all dangerous places in /proc by
> definition.
>
> A more effective solution would be to mount into the container only what
> is necessary and ignore the rest.
>
> Right now there is the opportunity to pass in the container any port of
> the /proc filesystem using mount --bind expect the pids.
>
> This patch allows to mount only the part of /proc related to pids
> without rest objects. Since this is an addon to /proc, flags applied to
> /proc have an effect on this pidfs filesystem.
>
> Why not implement it as another flag to /proc ?
>
> The /proc flags is stored in the pid_namespace and are global for
> namespace. It means that if you add a flag to hide all except the pids,
> then it will act on all mounted instances of /proc.
>
> Originally the idea was that the container will be mounted only pidfs
> and additional required files will be mounted on top using the
> overlayfs. But I found out that /proc does not support overlayfs and
> does not allow to mount anything on top or under it.
>
> My question is whether it's possible to add overlayfs support for /proc?
>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
>  Documentation/filesystems/pidfs.txt | 16 ++++++++
>  fs/proc/Kconfig                     |  8 ++++
>  fs/proc/inode.c                     |  8 +++-
>  fs/proc/internal.h                  |  2 +
>  fs/proc/root.c                      | 76 ++++++++++++++++++++++++++++++++++---
>  fs/proc/self.c                      |  6 +++
>  fs/proc/thread_self.c               |  6 +++
>  include/linux/pid_namespace.h       |  5 +++
>  8 files changed, 119 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/filesystems/pidfs.txt
>
> diff --git a/Documentation/filesystems/pidfs.txt b/Documentation/filesystems/pidfs.txt
> new file mode 100644
> index 0000000..ce958a5
> --- /dev/null
> +++ b/Documentation/filesystems/pidfs.txt
> @@ -0,0 +1,16 @@
> +The PIDFS Filesystem
> +====================
> +
> +The pidfs filesystem contains a subset of the /proc file system which contains
> +only information about the processes. The link self points to the process
> +reading the file system. All other special files and directories in /proc are
> +not available in this filesystem.
> +
> +The pidfs is not an independent filesystem, its implementation shares code
> +with /proc.
> +
> +All mount options applicable to /proc filesystem are also applicable
> +to pidfs filesystem. For example, access to the information in /proc/[pid]
> +directories can be restricted using hidepid option.
> +
> +To get more information about the processes read the proc.txt
> diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
> index 1ade120..fa568f6 100644
> --- a/fs/proc/Kconfig
> +++ b/fs/proc/Kconfig
> @@ -43,6 +43,14 @@ config PROC_VMCORE
>          help
>          Exports the dump image of crashed kernel in ELF format.
>  
> +config PROC_PIDFS
> +	bool "pidfs file system support"
> +	depends on PROC_FS
> +	default n
> +	help
> +	  The pidfs filesystem contains a subset of the /proc file system
> +	  which contains only information only about the processes.
> +
>  config PROC_SYSCTL
>  	bool "Sysctl support (/proc/sys)" if EXPERT
>  	depends on PROC_FS
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index 783bc19..1be65b4 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -474,12 +474,16 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
>  int proc_fill_super(struct super_block *s, void *data, int silent)
>  {
>  	struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
> +	struct proc_dir_entry *fs_root = &proc_root;
>  	struct inode *root_inode;
>  	int ret;
>  
>  	if (!proc_parse_options(data, ns))
>  		return -EINVAL;
>  
> +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type)
> +		fs_root = &pidfs_root;
> +
>  	/* User space would break if executables or devices appear on proc */
>  	s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
>  	s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC;
> @@ -496,8 +500,8 @@ int proc_fill_super(struct super_block *s, void *data, int silent)
>  	 */
>  	s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
>  	
> -	pde_get(&proc_root);
> -	root_inode = proc_get_inode(s, &proc_root);
> +	pde_get(fs_root);
> +	root_inode = proc_get_inode(s, fs_root);
>  	if (!root_inode) {
>  		pr_err("proc_fill_super: get root inode failed\n");
>  		return -ENOMEM;
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 2de5194..a7c068c 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -267,6 +267,8 @@ static inline void proc_tty_init(void) {}
>  /*
>   * root.c
>   */
> +extern struct file_system_type pidfs_fs_type;
> +extern struct proc_dir_entry pidfs_root;
>  extern struct proc_dir_entry proc_root;
>  extern int proc_parse_options(char *options, struct pid_namespace *pid);
>  
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index 4bd0373..de16ac1 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -102,10 +102,21 @@ static void proc_kill_sb(struct super_block *sb)
>  	struct pid_namespace *ns;
>  
>  	ns = (struct pid_namespace *)sb->s_fs_info;
> -	if (ns->proc_self)
> -		dput(ns->proc_self);
> -	if (ns->proc_thread_self)
> -		dput(ns->proc_thread_self);
> +
> +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && sb->s_type == &pidfs_fs_type) {
> +		if (ns->pidfs_self)
> +			dput(ns->pidfs_self);
> +
> +		if (ns->pidfs_thread_self)
> +			dput(ns->pidfs_thread_self);
> +	} else {
> +		if (ns->proc_self)
> +			dput(ns->proc_self);
> +
> +		if (ns->proc_thread_self)
> +			dput(ns->proc_thread_self);
> +	}
> +
>  	kill_anon_super(sb);
>  	put_pid_ns(ns);
>  }
> @@ -117,6 +128,13 @@ static struct file_system_type proc_fs_type = {
>  	.fs_flags	= FS_USERNS_MOUNT,
>  };
>  
> +struct file_system_type pidfs_fs_type = {
> +	.name		= "pidfs",
> +	.mount		= proc_mount,
> +	.kill_sb	= proc_kill_sb,
> +	.fs_flags	= FS_USERNS_MOUNT,
> +};
> +
>  void __init proc_root_init(void)
>  {
>  	int err;
> @@ -127,6 +145,10 @@ void __init proc_root_init(void)
>  	if (err)
>  		return;
>  
> +	err = register_filesystem(&pidfs_fs_type);
> +	if (err)
> +		return;
> +
>  	proc_self_init();
>  	proc_thread_self_init();
>  	proc_symlink("mounts", NULL, "self/mounts");
> @@ -148,8 +170,7 @@ void __init proc_root_init(void)
>  	proc_sys_init();
>  }
>  
> -static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat
> -)
> +static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
>  {
>  	generic_fillattr(d_inode(dentry), stat);
>  	stat->nlink = proc_root.nlink + nr_processes();
> @@ -176,6 +197,14 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
>  	return proc_pid_readdir(file, ctx);
>  }
>  
> +static int pidfs_root_readdir(struct file *file, struct dir_context *ctx)
> +{
> +	if (ctx->pos < FIRST_PROCESS_ENTRY)
> +		ctx->pos = FIRST_PROCESS_ENTRY;
> +
> +	return proc_pid_readdir(file, ctx);
> +}
> +
>  /*
>   * The root /proc directory is special, as it has the
>   * <pid> directories. Thus we don't use the generic
> @@ -187,6 +216,12 @@ static const struct file_operations proc_root_operations = {
>  	.llseek		= generic_file_llseek,
>  };
>  
> +static const struct file_operations pidfs_root_operations = {
> +	.read		 = generic_read_dir,
> +	.iterate_shared	 = pidfs_root_readdir,
> +	.llseek		= generic_file_llseek,
> +};
> +
>  /*
>   * proc root can do almost nothing..
>   */
> @@ -195,6 +230,11 @@ static const struct inode_operations proc_root_inode_operations = {
>  	.getattr	= proc_root_getattr,
>  };
>  
> +static const struct inode_operations pidfs_root_inode_operations = {
> +	.lookup		= proc_pid_lookup,
> +	.getattr	= proc_root_getattr,
> +};
> +
>  /*
>   * This is the root "inode" in the /proc tree..
>   */
> @@ -211,6 +251,19 @@ struct proc_dir_entry proc_root = {
>  	.name		= "/proc",
>  };
>  
> +struct proc_dir_entry pidfs_root = {
> +	.low_ino	= PROC_ROOT_INO,
> +	.namelen	= 6,
> +	.mode		= S_IFDIR | S_IRUGO | S_IXUGO,
> +	.nlink		= 2,
> +	.count		= ATOMIC_INIT(1),
> +	.proc_iops	= &pidfs_root_inode_operations,
> +	.proc_fops	= &pidfs_root_operations,
> +	.parent		= &pidfs_root,
> +	.subdir		= RB_ROOT,
> +	.name		= "/pidfs",
> +};
> +
>  int pid_ns_prepare_proc(struct pid_namespace *ns)
>  {
>  	struct vfsmount *mnt;
> @@ -220,10 +273,21 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
>  		return PTR_ERR(mnt);
>  
>  	ns->proc_mnt = mnt;
> +
> +	if (IS_ENABLED(CONFIG_PROC_PIDFS)) {
> +		mnt = kern_mount_data(&pidfs_fs_type, ns);
> +		if (IS_ERR(mnt))
> +			return PTR_ERR(mnt);
> +
> +		ns->pidfs_mnt = mnt;
> +	}
>  	return 0;
>  }
>  
>  void pid_ns_release_proc(struct pid_namespace *ns)
>  {
>  	kern_unmount(ns->proc_mnt);
> +
> +	if (IS_ENABLED(CONFIG_PROC_PIDFS))
> +		kern_unmount(ns->pidfs_mnt);
>  }
> diff --git a/fs/proc/self.c b/fs/proc/self.c
> index 4024595..dea7e17 100644
> --- a/fs/proc/self.c
> +++ b/fs/proc/self.c
> @@ -74,6 +74,12 @@ int proc_setup_self(struct super_block *s)
>  		pr_err("proc_fill_super: can't allocate /proc/self\n");
>  		return PTR_ERR(self);
>  	}
> +
> +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
> +		ns->pidfs_self = self;
> +		return 0;
> +	}
> +
>  	ns->proc_self = self;
>  	return 0;
>  }
> diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
> index 595b90a97..274c618 100644
> --- a/fs/proc/thread_self.c
> +++ b/fs/proc/thread_self.c
> @@ -76,6 +76,12 @@ int proc_setup_thread_self(struct super_block *s)
>  		pr_err("proc_fill_super: can't allocate /proc/thread_self\n");
>  		return PTR_ERR(thread_self);
>  	}
> +
> +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
> +		ns->pidfs_thread_self = thread_self;
> +		return 0;
> +	}
> +
>  	ns->proc_thread_self = thread_self;
>  	return 0;
>  }
> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> index 34cce96..fca3a76 100644
> --- a/include/linux/pid_namespace.h
> +++ b/include/linux/pid_namespace.h
> @@ -46,6 +46,11 @@ struct pid_namespace {
>  	int hide_pid;
>  	int reboot;	/* group exit code if this pidns was rebooted */
>  	struct ns_common ns;
> +#ifdef CONFIG_PROC_PIDFS
> +	struct vfsmount *pidfs_mnt;
> +	struct dentry *pidfs_self;
> +	struct dentry *pidfs_thread_self;
> +#endif
>  };
>  
>  extern struct pid_namespace init_pid_ns;
> -- 
> 2.10.2

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

#1584499

On Mon, Feb 20, 2017 at 05:05:18PM +1300, Eric W. Biederman wrote:
> Alexey Gladkov <gladkov.alexey@gmail.com> writes:
> 
> > The pidfs filesystem contains a subset of the /proc file system which
> > contains only information about the processes.
> 
> My summary of your motivation.
> 
> It hurts when I create a container with a processes with uid 0 inside of
> it.  This generates lots of hacks to attempt to limit uid 0.
> 
>   My answer:  Don't run a container with a real uid 0 inside of it.
> 
> Any reasonable permission check will on proc files
> will keep you safe if your container does not have a real uid 0 in it.

Right now this approach is all we have for not virtualized objects in /proc.
We can only hope that those who create files in /proc are intelligent enough.

We can't protect ourselves from the appearance of something unwanted in /proc.

> That said I am not opposed in principle to a pidfs.  And the idea of
> using overlay for this purpose is intriguing.  I have not looked at it
> in enough detail comment on the technical merits.

I'm currently researching the way of adding support overlayfs for /proc.

But before going any further, I would like to discuss in the right direction
I think or not.

> > Some of the container virtualization systems are mounted /proc inside
> > the container. This is done in most cases to operate with information
> > about the processes. Knowing that /proc filesystem is not fully
> > virtualized they are mounted on top of dangerous places empty files or
> > directories (for exmaple /proc/kcore, /sys/firmware, etc.).
> >
> > The structure of this filesystem is dynamic and any module can create a
> > new object which will not necessarily be virtualized. There are
> > proprietary modules that aren't in the mainline whose work we can not
> > verify.
> >
> > This opens up a potential threat to the system. The developers of the
> > virtualization system can't predict all dangerous places in /proc by
> > definition.
> >
> > A more effective solution would be to mount into the container only what
> > is necessary and ignore the rest.
> >
> > Right now there is the opportunity to pass in the container any port of
> > the /proc filesystem using mount --bind expect the pids.
> >
> > This patch allows to mount only the part of /proc related to pids
> > without rest objects. Since this is an addon to /proc, flags applied to
> > /proc have an effect on this pidfs filesystem.
> >
> > Why not implement it as another flag to /proc ?
> >
> > The /proc flags is stored in the pid_namespace and are global for
> > namespace. It means that if you add a flag to hide all except the pids,
> > then it will act on all mounted instances of /proc.
> >
> > Originally the idea was that the container will be mounted only pidfs
> > and additional required files will be mounted on top using the
> > overlayfs. But I found out that /proc does not support overlayfs and
> > does not allow to mount anything on top or under it.
> >
> > My question is whether it's possible to add overlayfs support for /proc?
> >
> > Cc: Kirill A. Shutemov <kirill@shutemov.name>
> > Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> > ---
> >  Documentation/filesystems/pidfs.txt | 16 ++++++++
> >  fs/proc/Kconfig                     |  8 ++++
> >  fs/proc/inode.c                     |  8 +++-
> >  fs/proc/internal.h                  |  2 +
> >  fs/proc/root.c                      | 76 ++++++++++++++++++++++++++++++++++---
> >  fs/proc/self.c                      |  6 +++
> >  fs/proc/thread_self.c               |  6 +++
> >  include/linux/pid_namespace.h       |  5 +++
> >  8 files changed, 119 insertions(+), 8 deletions(-)
> >  create mode 100644 Documentation/filesystems/pidfs.txt
> >
> > diff --git a/Documentation/filesystems/pidfs.txt b/Documentation/filesystems/pidfs.txt
> > new file mode 100644
> > index 0000000..ce958a5
> > --- /dev/null
> > +++ b/Documentation/filesystems/pidfs.txt
> > @@ -0,0 +1,16 @@
> > +The PIDFS Filesystem
> > +====================
> > +
> > +The pidfs filesystem contains a subset of the /proc file system which contains
> > +only information about the processes. The link self points to the process
> > +reading the file system. All other special files and directories in /proc are
> > +not available in this filesystem.
> > +
> > +The pidfs is not an independent filesystem, its implementation shares code
> > +with /proc.
> > +
> > +All mount options applicable to /proc filesystem are also applicable
> > +to pidfs filesystem. For example, access to the information in /proc/[pid]
> > +directories can be restricted using hidepid option.
> > +
> > +To get more information about the processes read the proc.txt
> > diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
> > index 1ade120..fa568f6 100644
> > --- a/fs/proc/Kconfig
> > +++ b/fs/proc/Kconfig
> > @@ -43,6 +43,14 @@ config PROC_VMCORE
> >          help
> >          Exports the dump image of crashed kernel in ELF format.
> >  
> > +config PROC_PIDFS
> > +	bool "pidfs file system support"
> > +	depends on PROC_FS
> > +	default n
> > +	help
> > +	  The pidfs filesystem contains a subset of the /proc file system
> > +	  which contains only information only about the processes.
> > +
> >  config PROC_SYSCTL
> >  	bool "Sysctl support (/proc/sys)" if EXPERT
> >  	depends on PROC_FS
> > diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> > index 783bc19..1be65b4 100644
> > --- a/fs/proc/inode.c
> > +++ b/fs/proc/inode.c
> > @@ -474,12 +474,16 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
> >  int proc_fill_super(struct super_block *s, void *data, int silent)
> >  {
> >  	struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
> > +	struct proc_dir_entry *fs_root = &proc_root;
> >  	struct inode *root_inode;
> >  	int ret;
> >  
> >  	if (!proc_parse_options(data, ns))
> >  		return -EINVAL;
> >  
> > +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type)
> > +		fs_root = &pidfs_root;
> > +
> >  	/* User space would break if executables or devices appear on proc */
> >  	s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
> >  	s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC;
> > @@ -496,8 +500,8 @@ int proc_fill_super(struct super_block *s, void *data, int silent)
> >  	 */
> >  	s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
> >  	
> > -	pde_get(&proc_root);
> > -	root_inode = proc_get_inode(s, &proc_root);
> > +	pde_get(fs_root);
> > +	root_inode = proc_get_inode(s, fs_root);
> >  	if (!root_inode) {
> >  		pr_err("proc_fill_super: get root inode failed\n");
> >  		return -ENOMEM;
> > diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> > index 2de5194..a7c068c 100644
> > --- a/fs/proc/internal.h
> > +++ b/fs/proc/internal.h
> > @@ -267,6 +267,8 @@ static inline void proc_tty_init(void) {}
> >  /*
> >   * root.c
> >   */
> > +extern struct file_system_type pidfs_fs_type;
> > +extern struct proc_dir_entry pidfs_root;
> >  extern struct proc_dir_entry proc_root;
> >  extern int proc_parse_options(char *options, struct pid_namespace *pid);
> >  
> > diff --git a/fs/proc/root.c b/fs/proc/root.c
> > index 4bd0373..de16ac1 100644
> > --- a/fs/proc/root.c
> > +++ b/fs/proc/root.c
> > @@ -102,10 +102,21 @@ static void proc_kill_sb(struct super_block *sb)
> >  	struct pid_namespace *ns;
> >  
> >  	ns = (struct pid_namespace *)sb->s_fs_info;
> > -	if (ns->proc_self)
> > -		dput(ns->proc_self);
> > -	if (ns->proc_thread_self)
> > -		dput(ns->proc_thread_self);
> > +
> > +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && sb->s_type == &pidfs_fs_type) {
> > +		if (ns->pidfs_self)
> > +			dput(ns->pidfs_self);
> > +
> > +		if (ns->pidfs_thread_self)
> > +			dput(ns->pidfs_thread_self);
> > +	} else {
> > +		if (ns->proc_self)
> > +			dput(ns->proc_self);
> > +
> > +		if (ns->proc_thread_self)
> > +			dput(ns->proc_thread_self);
> > +	}
> > +
> >  	kill_anon_super(sb);
> >  	put_pid_ns(ns);
> >  }
> > @@ -117,6 +128,13 @@ static struct file_system_type proc_fs_type = {
> >  	.fs_flags	= FS_USERNS_MOUNT,
> >  };
> >  
> > +struct file_system_type pidfs_fs_type = {
> > +	.name		= "pidfs",
> > +	.mount		= proc_mount,
> > +	.kill_sb	= proc_kill_sb,
> > +	.fs_flags	= FS_USERNS_MOUNT,
> > +};
> > +
> >  void __init proc_root_init(void)
> >  {
> >  	int err;
> > @@ -127,6 +145,10 @@ void __init proc_root_init(void)
> >  	if (err)
> >  		return;
> >  
> > +	err = register_filesystem(&pidfs_fs_type);
> > +	if (err)
> > +		return;
> > +
> >  	proc_self_init();
> >  	proc_thread_self_init();
> >  	proc_symlink("mounts", NULL, "self/mounts");
> > @@ -148,8 +170,7 @@ void __init proc_root_init(void)
> >  	proc_sys_init();
> >  }
> >  
> > -static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat
> > -)
> > +static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
> >  {
> >  	generic_fillattr(d_inode(dentry), stat);
> >  	stat->nlink = proc_root.nlink + nr_processes();
> > @@ -176,6 +197,14 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
> >  	return proc_pid_readdir(file, ctx);
> >  }
> >  
> > +static int pidfs_root_readdir(struct file *file, struct dir_context *ctx)
> > +{
> > +	if (ctx->pos < FIRST_PROCESS_ENTRY)
> > +		ctx->pos = FIRST_PROCESS_ENTRY;
> > +
> > +	return proc_pid_readdir(file, ctx);
> > +}
> > +
> >  /*
> >   * The root /proc directory is special, as it has the
> >   * <pid> directories. Thus we don't use the generic
> > @@ -187,6 +216,12 @@ static const struct file_operations proc_root_operations = {
> >  	.llseek		= generic_file_llseek,
> >  };
> >  
> > +static const struct file_operations pidfs_root_operations = {
> > +	.read		 = generic_read_dir,
> > +	.iterate_shared	 = pidfs_root_readdir,
> > +	.llseek		= generic_file_llseek,
> > +};
> > +
> >  /*
> >   * proc root can do almost nothing..
> >   */
> > @@ -195,6 +230,11 @@ static const struct inode_operations proc_root_inode_operations = {
> >  	.getattr	= proc_root_getattr,
> >  };
> >  
> > +static const struct inode_operations pidfs_root_inode_operations = {
> > +	.lookup		= proc_pid_lookup,
> > +	.getattr	= proc_root_getattr,
> > +};
> > +
> >  /*
> >   * This is the root "inode" in the /proc tree..
> >   */
> > @@ -211,6 +251,19 @@ struct proc_dir_entry proc_root = {
> >  	.name		= "/proc",
> >  };
> >  
> > +struct proc_dir_entry pidfs_root = {
> > +	.low_ino	= PROC_ROOT_INO,
> > +	.namelen	= 6,
> > +	.mode		= S_IFDIR | S_IRUGO | S_IXUGO,
> > +	.nlink		= 2,
> > +	.count		= ATOMIC_INIT(1),
> > +	.proc_iops	= &pidfs_root_inode_operations,
> > +	.proc_fops	= &pidfs_root_operations,
> > +	.parent		= &pidfs_root,
> > +	.subdir		= RB_ROOT,
> > +	.name		= "/pidfs",
> > +};
> > +
> >  int pid_ns_prepare_proc(struct pid_namespace *ns)
> >  {
> >  	struct vfsmount *mnt;
> > @@ -220,10 +273,21 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
> >  		return PTR_ERR(mnt);
> >  
> >  	ns->proc_mnt = mnt;
> > +
> > +	if (IS_ENABLED(CONFIG_PROC_PIDFS)) {
> > +		mnt = kern_mount_data(&pidfs_fs_type, ns);
> > +		if (IS_ERR(mnt))
> > +			return PTR_ERR(mnt);
> > +
> > +		ns->pidfs_mnt = mnt;
> > +	}
> >  	return 0;
> >  }
> >  
> >  void pid_ns_release_proc(struct pid_namespace *ns)
> >  {
> >  	kern_unmount(ns->proc_mnt);
> > +
> > +	if (IS_ENABLED(CONFIG_PROC_PIDFS))
> > +		kern_unmount(ns->pidfs_mnt);
> >  }
> > diff --git a/fs/proc/self.c b/fs/proc/self.c
> > index 4024595..dea7e17 100644
> > --- a/fs/proc/self.c
> > +++ b/fs/proc/self.c
> > @@ -74,6 +74,12 @@ int proc_setup_self(struct super_block *s)
> >  		pr_err("proc_fill_super: can't allocate /proc/self\n");
> >  		return PTR_ERR(self);
> >  	}
> > +
> > +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
> > +		ns->pidfs_self = self;
> > +		return 0;
> > +	}
> > +
> >  	ns->proc_self = self;
> >  	return 0;
> >  }
> > diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
> > index 595b90a97..274c618 100644
> > --- a/fs/proc/thread_self.c
> > +++ b/fs/proc/thread_self.c
> > @@ -76,6 +76,12 @@ int proc_setup_thread_self(struct super_block *s)
> >  		pr_err("proc_fill_super: can't allocate /proc/thread_self\n");
> >  		return PTR_ERR(thread_self);
> >  	}
> > +
> > +	if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
> > +		ns->pidfs_thread_self = thread_self;
> > +		return 0;
> > +	}
> > +
> >  	ns->proc_thread_self = thread_self;
> >  	return 0;
> >  }
> > diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> > index 34cce96..fca3a76 100644
> > --- a/include/linux/pid_namespace.h
> > +++ b/include/linux/pid_namespace.h
> > @@ -46,6 +46,11 @@ struct pid_namespace {
> >  	int hide_pid;
> >  	int reboot;	/* group exit code if this pidns was rebooted */
> >  	struct ns_common ns;
> > +#ifdef CONFIG_PROC_PIDFS
> > +	struct vfsmount *pidfs_mnt;
> > +	struct dentry *pidfs_self;
> > +	struct dentry *pidfs_thread_self;
> > +#endif
> >  };
> >  
> >  extern struct pid_namespace init_pid_ns;
> > -- 
> > 2.10.2

-- 
Rgrds, legion

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

#1586452

On Mon, Feb 20, 2017 at 5:05 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Alexey Gladkov <gladkov.alexey@gmail.com> writes:
>
>> The pidfs filesystem contains a subset of the /proc file system which
>> contains only information about the processes.
>
> My summary of your motivation.
>
> It hurts when I create a container with a processes with uid 0 inside of
> it.  This generates lots of hacks to attempt to limit uid 0.
>
>   My answer:  Don't run a container with a real uid 0 inside of it.

I agree. Unless I miss something I'd say use a user namespace
to get decent permission checks in /proc (and /sys).

-- 
Thanks,
//richard

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

#1585395

On 02/18, Alexey Gladkov wrote:
>
> This patch allows to mount only the part of /proc related to pids
> without rest objects. Since this is an addon to /proc, flags applied to
> /proc have an effect on this pidfs filesystem.

I leave this to you and Eric, but imo it would be nice to avoid another
filesystem.

> Why not implement it as another flag to /proc ?
>
> The /proc flags is stored in the pid_namespace and are global for
> namespace. It means that if you add a flag to hide all except the pids,
> then it will act on all mounted instances of /proc.

But perhaps we can use mnt_flags? For example, lets abuse MNT_NODEV, see
the simple patch below. Not sure it is correct/complete, just to illustrate
the idea.

With this patch you can mount proc with -onodev and it will only show
pids/self/thread_self:

	# mkdir /tmp/D
	# mount -t proc -o nodev none /tmp/D
	# ls /tmp/D
	1   11	13  15	17  19	20  22	24  28	3   31	33  4  56  7  9     thread-self
	10  12	14  16	18  2	21  23	27  29	30  32	34  5  6   8  self
	# cat /tmp/D/meminfo
	cat: /tmp/D/meminfo: No such file or directory
	# ls /tmp/D/irq
	ls: cannot open directory /tmp/D/irq: No such file or directory

No?

Oleg.


--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -305,11 +305,22 @@ int proc_readdir_de(struct proc_dir_entry *de, struct file *file,
 
 int proc_readdir(struct file *file, struct dir_context *ctx)
 {
+	int mnt_flags = file->f_path.mnt->mnt_flags;
 	struct inode *inode = file_inode(file);
 
+	if (mnt_flags & MNT_NODEV)
+		return 1;
+
 	return proc_readdir_de(PDE(inode), file, ctx);
 }
 
+static int proc_dir_open(struct inode *inode, struct file *file)
+{
+	if (file->f_path.mnt->mnt_flags & MNT_NODEV)
+		return -ENOENT;
+	return 0;
+}
+
 /*
  * These are the generic /proc directory operations. They
  * use the in-memory "struct proc_dir_entry" tree to parse
@@ -319,6 +330,7 @@ static const struct file_operations proc_dir_operations = {
 	.llseek			= generic_file_llseek,
 	.read			= generic_read_dir,
 	.iterate_shared		= proc_readdir,
+	.open			= proc_dir_open,
 };
 
 /*
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -318,12 +318,16 @@ proc_reg_get_unmapped_area(struct file *file, unsigned long orig_addr,
 
 static int proc_reg_open(struct inode *inode, struct file *file)
 {
+	int mnt_flags = file->f_path.mnt->mnt_flags;
 	struct proc_dir_entry *pde = PDE(inode);
 	int rv = 0;
 	int (*open)(struct inode *, struct file *);
 	int (*release)(struct inode *, struct file *);
 	struct pde_opener *pdeo;
 
+	if (mnt_flags & MNT_NODEV)
+		return -ENOENT;
+
 	/*
 	 * Ensure that
 	 * 1) PDE's ->release hook will be called no matter what

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

#1586095

On Tue, Feb 21, 2017 at 03:57:47PM +0100, Oleg Nesterov wrote:
> On 02/18, Alexey Gladkov wrote:
> >
> > This patch allows to mount only the part of /proc related to pids
> > without rest objects. Since this is an addon to /proc, flags applied to
> > /proc have an effect on this pidfs filesystem.
> 
> I leave this to you and Eric, but imo it would be nice to avoid another
> filesystem.
> 
> > Why not implement it as another flag to /proc ?
> >
> > The /proc flags is stored in the pid_namespace and are global for
> > namespace. It means that if you add a flag to hide all except the pids,
> > then it will act on all mounted instances of /proc.
> 
> But perhaps we can use mnt_flags? For example, lets abuse MNT_NODEV, see
> the simple patch below. Not sure it is correct/complete, just to illustrate
> the idea.
> 
> With this patch you can mount proc with -onodev and it will only show
> pids/self/thread_self:
> 
> 	# mkdir /tmp/D
> 	# mount -t proc -o nodev none /tmp/D
> 	# ls /tmp/D
> 	1   11	13  15	17  19	20  22	24  28	3   31	33  4  56  7  9     thread-self
> 	10  12	14  16	18  2	21  23	27  29	30  32	34  5  6   8  self
> 	# cat /tmp/D/meminfo
> 	cat: /tmp/D/meminfo: No such file or directory
> 	# ls /tmp/D/irq
> 	ls: cannot open directory /tmp/D/irq: No such file or directory
> 
> No?

I'm embarrassed that we start the change procfs by abuse something. It is very
difficult to explain why this option leads to such consequences on procfs.

Also with this change it won't be the same procfs. We can't to name it as procfs
because procfs have cpuinfo, meminfo, etc.

What do you think about this ?

-- 
Rgrds, legion

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

#1586107

On Wed, Feb 22, 2017 at 10:40:49AM +0300, Pavel Emelyanov wrote:
> On 02/21/2017 05:57 PM, Oleg Nesterov wrote:
> > On 02/18, Alexey Gladkov wrote:
> >>
> >> This patch allows to mount only the part of /proc related to pids
> >> without rest objects. Since this is an addon to /proc, flags applied to
> >> /proc have an effect on this pidfs filesystem.
> > 
> > I leave this to you and Eric, but imo it would be nice to avoid another
> > filesystem.
> > 
> >> Why not implement it as another flag to /proc ?
> >>
> >> The /proc flags is stored in the pid_namespace and are global for
> >> namespace. It means that if you add a flag to hide all except the pids,
> >> then it will act on all mounted instances of /proc.
> > 
> > But perhaps we can use mnt_flags? For example, lets abuse MNT_NODEV, see
> > the simple patch below. Not sure it is correct/complete, just to illustrate
> > the idea.
> > 
> > With this patch you can mount proc with -onodev and it will only show
> > pids/self/thread_self:
> > 
> > 	# mkdir /tmp/D
> > 	# mount -t proc -o nodev none /tmp/D
> > 	# ls /tmp/D
> > 	1   11	13  15	17  19	20  22	24  28	3   31	33  4  56  7  9     thread-self
> > 	10  12	14  16	18  2	21  23	27  29	30  32	34  5  6   8  self
> > 	# cat /tmp/D/meminfo
> > 	cat: /tmp/D/meminfo: No such file or directory
> > 	# ls /tmp/D/irq
> > 	ls: cannot open directory /tmp/D/irq: No such file or directory
> > 
> > No?
> 
> Yes!!! If this whole effort with pidfs and overlayfs will move forward, I would
> prefer seeing the nodev procfs version, rather than another fs.

But this is not procfs anymore. If someone will wait for procfs here it will
be disappointed :)

> As far as the overlayfs part is concerned, having an overlayfs mounted on /proc
> inside container may result in problems as applications sometimes check for /proc
> containing procfs (by checking statfs.f_type == PROC_SUPER_MAGIC or by reading
> the /proc/mounts).

It is not a replacement for procfs. It's a subset of procfs. If someone wants
the procfs in the code we should not deceive him.

No?

> -- Pavel
> 
> > Oleg.
> > 
> > 
> > --- a/fs/proc/generic.c
> > +++ b/fs/proc/generic.c
> > @@ -305,11 +305,22 @@ int proc_readdir_de(struct proc_dir_entry *de, struct file *file,
> >  
> >  int proc_readdir(struct file *file, struct dir_context *ctx)
> >  {
> > +	int mnt_flags = file->f_path.mnt->mnt_flags;
> >  	struct inode *inode = file_inode(file);
> >  
> > +	if (mnt_flags & MNT_NODEV)
> > +		return 1;
> > +
> >  	return proc_readdir_de(PDE(inode), file, ctx);
> >  }
> >  
> > +static int proc_dir_open(struct inode *inode, struct file *file)
> > +{
> > +	if (file->f_path.mnt->mnt_flags & MNT_NODEV)
> > +		return -ENOENT;
> > +	return 0;
> > +}
> > +
> >  /*
> >   * These are the generic /proc directory operations. They
> >   * use the in-memory "struct proc_dir_entry" tree to parse
> > @@ -319,6 +330,7 @@ static const struct file_operations proc_dir_operations = {
> >  	.llseek			= generic_file_llseek,
> >  	.read			= generic_read_dir,
> >  	.iterate_shared		= proc_readdir,
> > +	.open			= proc_dir_open,
> >  };
> >  
> >  /*
> > --- a/fs/proc/inode.c
> > +++ b/fs/proc/inode.c
> > @@ -318,12 +318,16 @@ proc_reg_get_unmapped_area(struct file *file, unsigned long orig_addr,
> >  
> >  static int proc_reg_open(struct inode *inode, struct file *file)
> >  {
> > +	int mnt_flags = file->f_path.mnt->mnt_flags;
> >  	struct proc_dir_entry *pde = PDE(inode);
> >  	int rv = 0;
> >  	int (*open)(struct inode *, struct file *);
> >  	int (*release)(struct inode *, struct file *);
> >  	struct pde_opener *pdeo;
> >  
> > +	if (mnt_flags & MNT_NODEV)
> > +		return -ENOENT;
> > +
> >  	/*
> >  	 * Ensure that
> >  	 * 1) PDE's ->release hook will be called no matter what
> > 
> > .
> > 

-- 
Rgrds, legion

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

#1586271

[Multipart message — attachments visible in raw view] — view raw

On Tue, Feb 21, 2017 at 03:57:47PM +0100, Oleg Nesterov wrote:
> On 02/18, Alexey Gladkov wrote:
> >
> > This patch allows to mount only the part of /proc related to pids
> > without rest objects. Since this is an addon to /proc, flags applied to
> > /proc have an effect on this pidfs filesystem.
> 
> I leave this to you and Eric, but imo it would be nice to avoid another
> filesystem.
> 
> > Why not implement it as another flag to /proc ?
> >
> > The /proc flags is stored in the pid_namespace and are global for
> > namespace. It means that if you add a flag to hide all except the pids,
> > then it will act on all mounted instances of /proc.
> 
> But perhaps we can use mnt_flags? For example, lets abuse MNT_NODEV, see
> the simple patch below. Not sure it is correct/complete, just to illustrate
> the idea.
> 
> With this patch you can mount proc with -onodev and it will only show
> pids/self/thread_self:
> 
> 	# mkdir /tmp/D
> 	# mount -t proc -o nodev none /tmp/D
> 	# ls /tmp/D
> 	1   11	13  15	17  19	20  22	24  28	3   31	33  4  56  7  9     thread-self
> 	10  12	14  16	18  2	21  23	27  29	30  32	34  5  6   8  self
> 	# cat /tmp/D/meminfo
> 	cat: /tmp/D/meminfo: No such file or directory
> 	# ls /tmp/D/irq
> 	ls: cannot open directory /tmp/D/irq: No such file or directory
> 
> No?

I like the idea of using mnt_flags to turn procfs into pidfs, thus
avoiding yet another filesystem, but MNT_NODEV has a different meaning,
namely "do not interpret character or block special devices on the file
system".  I've actually found a system nearby that already mounts /proc
with nodev:

# grep 'proc.*nodev' /proc/mounts
proc /var/lib/vz/root/1002/proc proc rw,nosuid,nodev,noexec,relatime,gid=19,hidepid=2 0 0
proc /var/lib/vz/root/1003/proc proc rw,nosuid,nodev,noexec,relatime,gid=19,hidepid=2 0 0
proc /var/lib/vz/root/1004/proc proc rw,nosuid,nodev,noexec,relatime,gid=19,hidepid=2 0 0
proc /var/lib/vz/root/1005/proc proc rw,nosuid,nodev,noexec,relatime,gid=19,hidepid=2 0 0


-- 
ldv

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

#1586349

On 02/22, Dmitry V. Levin wrote:
>
> On Tue, Feb 21, 2017 at 03:57:47PM +0100, Oleg Nesterov wrote:
> >
> > But perhaps we can use mnt_flags? For example, lets abuse MNT_NODEV, see
> > the simple patch below. Not sure it is correct/complete, just to illustrate
> > the idea.

...

> I like the idea of using mnt_flags to turn procfs into pidfs, thus
> avoiding yet another filesystem, but MNT_NODEV has a different meaning,

Yes, yes, see "abuse MNT_NODEV" above. The patch just tries to illustrate
the idea.

Oleg.

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

#1586437

On Wed, Feb 22, 2017 at 06:48:03PM +0100, Oleg Nesterov wrote:
> On 02/22, Dmitry V. Levin wrote:
> >
> > On Tue, Feb 21, 2017 at 03:57:47PM +0100, Oleg Nesterov wrote:
> > >
> > > But perhaps we can use mnt_flags? For example, lets abuse MNT_NODEV, see
> > > the simple patch below. Not sure it is correct/complete, just to illustrate
> > > the idea.
> 
> ...
> 
> > I like the idea of using mnt_flags to turn procfs into pidfs, thus
> > avoiding yet another filesystem, but MNT_NODEV has a different meaning,
> 
> Yes, yes, see "abuse MNT_NODEV" above. The patch just tries to illustrate
> the idea.

I think I can try to use subtype for this. In this case, it is still the same
procfs, but with the subtype:

mount -t proc.pidfs ...

-- 
Rgrds, legion

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

#1588874

[CC += linux-api@vger.kernel.org]

Hi Alexey

This is a change to the kernel-user-space API. Please CC
linux-api@vger.kernel.org on any future iterations of this patch.

Thanks,

Michael


On Sat, Feb 18, 2017 at 11:53 PM, Alexey Gladkov
<gladkov.alexey@gmail.com> wrote:
> The pidfs filesystem contains a subset of the /proc file system which
> contains only information about the processes.
>
> Some of the container virtualization systems are mounted /proc inside
> the container. This is done in most cases to operate with information
> about the processes. Knowing that /proc filesystem is not fully
> virtualized they are mounted on top of dangerous places empty files or
> directories (for exmaple /proc/kcore, /sys/firmware, etc.).
>
> The structure of this filesystem is dynamic and any module can create a
> new object which will not necessarily be virtualized. There are
> proprietary modules that aren't in the mainline whose work we can not
> verify.
>
> This opens up a potential threat to the system. The developers of the
> virtualization system can't predict all dangerous places in /proc by
> definition.
>
> A more effective solution would be to mount into the container only what
> is necessary and ignore the rest.
>
> Right now there is the opportunity to pass in the container any port of
> the /proc filesystem using mount --bind expect the pids.
>
> This patch allows to mount only the part of /proc related to pids
> without rest objects. Since this is an addon to /proc, flags applied to
> /proc have an effect on this pidfs filesystem.
>
> Why not implement it as another flag to /proc ?
>
> The /proc flags is stored in the pid_namespace and are global for
> namespace. It means that if you add a flag to hide all except the pids,
> then it will act on all mounted instances of /proc.
>
> Originally the idea was that the container will be mounted only pidfs
> and additional required files will be mounted on top using the
> overlayfs. But I found out that /proc does not support overlayfs and
> does not allow to mount anything on top or under it.
>
> My question is whether it's possible to add overlayfs support for /proc?
>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
>  Documentation/filesystems/pidfs.txt | 16 ++++++++
>  fs/proc/Kconfig                     |  8 ++++
>  fs/proc/inode.c                     |  8 +++-
>  fs/proc/internal.h                  |  2 +
>  fs/proc/root.c                      | 76 ++++++++++++++++++++++++++++++++++---
>  fs/proc/self.c                      |  6 +++
>  fs/proc/thread_self.c               |  6 +++
>  include/linux/pid_namespace.h       |  5 +++
>  8 files changed, 119 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/filesystems/pidfs.txt
>
> diff --git a/Documentation/filesystems/pidfs.txt b/Documentation/filesystems/pidfs.txt
> new file mode 100644
> index 0000000..ce958a5
> --- /dev/null
> +++ b/Documentation/filesystems/pidfs.txt
> @@ -0,0 +1,16 @@
> +The PIDFS Filesystem
> +====================
> +
> +The pidfs filesystem contains a subset of the /proc file system which contains
> +only information about the processes. The link self points to the process
> +reading the file system. All other special files and directories in /proc are
> +not available in this filesystem.
> +
> +The pidfs is not an independent filesystem, its implementation shares code
> +with /proc.
> +
> +All mount options applicable to /proc filesystem are also applicable
> +to pidfs filesystem. For example, access to the information in /proc/[pid]
> +directories can be restricted using hidepid option.
> +
> +To get more information about the processes read the proc.txt
> diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
> index 1ade120..fa568f6 100644
> --- a/fs/proc/Kconfig
> +++ b/fs/proc/Kconfig
> @@ -43,6 +43,14 @@ config PROC_VMCORE
>          help
>          Exports the dump image of crashed kernel in ELF format.
>
> +config PROC_PIDFS
> +       bool "pidfs file system support"
> +       depends on PROC_FS
> +       default n
> +       help
> +         The pidfs filesystem contains a subset of the /proc file system
> +         which contains only information only about the processes.
> +
>  config PROC_SYSCTL
>         bool "Sysctl support (/proc/sys)" if EXPERT
>         depends on PROC_FS
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index 783bc19..1be65b4 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -474,12 +474,16 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
>  int proc_fill_super(struct super_block *s, void *data, int silent)
>  {
>         struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
> +       struct proc_dir_entry *fs_root = &proc_root;
>         struct inode *root_inode;
>         int ret;
>
>         if (!proc_parse_options(data, ns))
>                 return -EINVAL;
>
> +       if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type)
> +               fs_root = &pidfs_root;
> +
>         /* User space would break if executables or devices appear on proc */
>         s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
>         s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC;
> @@ -496,8 +500,8 @@ int proc_fill_super(struct super_block *s, void *data, int silent)
>          */
>         s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
>
> -       pde_get(&proc_root);
> -       root_inode = proc_get_inode(s, &proc_root);
> +       pde_get(fs_root);
> +       root_inode = proc_get_inode(s, fs_root);
>         if (!root_inode) {
>                 pr_err("proc_fill_super: get root inode failed\n");
>                 return -ENOMEM;
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 2de5194..a7c068c 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -267,6 +267,8 @@ static inline void proc_tty_init(void) {}
>  /*
>   * root.c
>   */
> +extern struct file_system_type pidfs_fs_type;
> +extern struct proc_dir_entry pidfs_root;
>  extern struct proc_dir_entry proc_root;
>  extern int proc_parse_options(char *options, struct pid_namespace *pid);
>
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index 4bd0373..de16ac1 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -102,10 +102,21 @@ static void proc_kill_sb(struct super_block *sb)
>         struct pid_namespace *ns;
>
>         ns = (struct pid_namespace *)sb->s_fs_info;
> -       if (ns->proc_self)
> -               dput(ns->proc_self);
> -       if (ns->proc_thread_self)
> -               dput(ns->proc_thread_self);
> +
> +       if (IS_ENABLED(CONFIG_PROC_PIDFS) && sb->s_type == &pidfs_fs_type) {
> +               if (ns->pidfs_self)
> +                       dput(ns->pidfs_self);
> +
> +               if (ns->pidfs_thread_self)
> +                       dput(ns->pidfs_thread_self);
> +       } else {
> +               if (ns->proc_self)
> +                       dput(ns->proc_self);
> +
> +               if (ns->proc_thread_self)
> +                       dput(ns->proc_thread_self);
> +       }
> +
>         kill_anon_super(sb);
>         put_pid_ns(ns);
>  }
> @@ -117,6 +128,13 @@ static struct file_system_type proc_fs_type = {
>         .fs_flags       = FS_USERNS_MOUNT,
>  };
>
> +struct file_system_type pidfs_fs_type = {
> +       .name           = "pidfs",
> +       .mount          = proc_mount,
> +       .kill_sb        = proc_kill_sb,
> +       .fs_flags       = FS_USERNS_MOUNT,
> +};
> +
>  void __init proc_root_init(void)
>  {
>         int err;
> @@ -127,6 +145,10 @@ void __init proc_root_init(void)
>         if (err)
>                 return;
>
> +       err = register_filesystem(&pidfs_fs_type);
> +       if (err)
> +               return;
> +
>         proc_self_init();
>         proc_thread_self_init();
>         proc_symlink("mounts", NULL, "self/mounts");
> @@ -148,8 +170,7 @@ void __init proc_root_init(void)
>         proc_sys_init();
>  }
>
> -static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat
> -)
> +static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
>  {
>         generic_fillattr(d_inode(dentry), stat);
>         stat->nlink = proc_root.nlink + nr_processes();
> @@ -176,6 +197,14 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
>         return proc_pid_readdir(file, ctx);
>  }
>
> +static int pidfs_root_readdir(struct file *file, struct dir_context *ctx)
> +{
> +       if (ctx->pos < FIRST_PROCESS_ENTRY)
> +               ctx->pos = FIRST_PROCESS_ENTRY;
> +
> +       return proc_pid_readdir(file, ctx);
> +}
> +
>  /*
>   * The root /proc directory is special, as it has the
>   * <pid> directories. Thus we don't use the generic
> @@ -187,6 +216,12 @@ static const struct file_operations proc_root_operations = {
>         .llseek         = generic_file_llseek,
>  };
>
> +static const struct file_operations pidfs_root_operations = {
> +       .read            = generic_read_dir,
> +       .iterate_shared  = pidfs_root_readdir,
> +       .llseek         = generic_file_llseek,
> +};
> +
>  /*
>   * proc root can do almost nothing..
>   */
> @@ -195,6 +230,11 @@ static const struct inode_operations proc_root_inode_operations = {
>         .getattr        = proc_root_getattr,
>  };
>
> +static const struct inode_operations pidfs_root_inode_operations = {
> +       .lookup         = proc_pid_lookup,
> +       .getattr        = proc_root_getattr,
> +};
> +
>  /*
>   * This is the root "inode" in the /proc tree..
>   */
> @@ -211,6 +251,19 @@ struct proc_dir_entry proc_root = {
>         .name           = "/proc",
>  };
>
> +struct proc_dir_entry pidfs_root = {
> +       .low_ino        = PROC_ROOT_INO,
> +       .namelen        = 6,
> +       .mode           = S_IFDIR | S_IRUGO | S_IXUGO,
> +       .nlink          = 2,
> +       .count          = ATOMIC_INIT(1),
> +       .proc_iops      = &pidfs_root_inode_operations,
> +       .proc_fops      = &pidfs_root_operations,
> +       .parent         = &pidfs_root,
> +       .subdir         = RB_ROOT,
> +       .name           = "/pidfs",
> +};
> +
>  int pid_ns_prepare_proc(struct pid_namespace *ns)
>  {
>         struct vfsmount *mnt;
> @@ -220,10 +273,21 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
>                 return PTR_ERR(mnt);
>
>         ns->proc_mnt = mnt;
> +
> +       if (IS_ENABLED(CONFIG_PROC_PIDFS)) {
> +               mnt = kern_mount_data(&pidfs_fs_type, ns);
> +               if (IS_ERR(mnt))
> +                       return PTR_ERR(mnt);
> +
> +               ns->pidfs_mnt = mnt;
> +       }
>         return 0;
>  }
>
>  void pid_ns_release_proc(struct pid_namespace *ns)
>  {
>         kern_unmount(ns->proc_mnt);
> +
> +       if (IS_ENABLED(CONFIG_PROC_PIDFS))
> +               kern_unmount(ns->pidfs_mnt);
>  }
> diff --git a/fs/proc/self.c b/fs/proc/self.c
> index 4024595..dea7e17 100644
> --- a/fs/proc/self.c
> +++ b/fs/proc/self.c
> @@ -74,6 +74,12 @@ int proc_setup_self(struct super_block *s)
>                 pr_err("proc_fill_super: can't allocate /proc/self\n");
>                 return PTR_ERR(self);
>         }
> +
> +       if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
> +               ns->pidfs_self = self;
> +               return 0;
> +       }
> +
>         ns->proc_self = self;
>         return 0;
>  }
> diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
> index 595b90a97..274c618 100644
> --- a/fs/proc/thread_self.c
> +++ b/fs/proc/thread_self.c
> @@ -76,6 +76,12 @@ int proc_setup_thread_self(struct super_block *s)
>                 pr_err("proc_fill_super: can't allocate /proc/thread_self\n");
>                 return PTR_ERR(thread_self);
>         }
> +
> +       if (IS_ENABLED(CONFIG_PROC_PIDFS) && s->s_type == &pidfs_fs_type) {
> +               ns->pidfs_thread_self = thread_self;
> +               return 0;
> +       }
> +
>         ns->proc_thread_self = thread_self;
>         return 0;
>  }
> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> index 34cce96..fca3a76 100644
> --- a/include/linux/pid_namespace.h
> +++ b/include/linux/pid_namespace.h
> @@ -46,6 +46,11 @@ struct pid_namespace {
>         int hide_pid;
>         int reboot;     /* group exit code if this pidns was rebooted */
>         struct ns_common ns;
> +#ifdef CONFIG_PROC_PIDFS
> +       struct vfsmount *pidfs_mnt;
> +       struct dentry *pidfs_self;
> +       struct dentry *pidfs_thread_self;
> +#endif
>  };
>
>  extern struct pid_namespace init_pid_ns;
> --
> 2.10.2
>
>
> --
> Rgrds, legion
>



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web