Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v1 0/2] proc: Relax check of mount visibility
@ 2020-07-27 14:14 Alexey Gladkov
  2020-07-27 14:14 ` [PATCH v1 1/2] " Alexey Gladkov
  2020-07-27 14:14 ` [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
  0 siblings, 2 replies; 10+ messages in thread
From: Alexey Gladkov @ 2020-07-27 14:14 UTC (permalink / raw)
  To: LKML
  Cc: Linux FS Devel, Alexander Viro, Alexey Gladkov,
	Eric W . Biederman, Kees Cook

If only the dynamic part of procfs is mounted (subset=pid), then there is no
need to check if procfs is fully visible to the user in the new user namespace.

Alexey Gladkov (2):
  proc: Relax check of mount visibility
  Show /proc/self/net only for CAP_NET_ADMIN

 fs/namespace.c     | 27 ++++++++++++++++-----------
 fs/proc/proc_net.c |  6 ++++++
 fs/proc/root.c     | 16 +++++++++-------
 include/linux/fs.h |  1 +
 4 files changed, 32 insertions(+), 18 deletions(-)

-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/2] proc: Relax check of mount visibility
  2020-07-27 14:14 [PATCH v1 0/2] proc: Relax check of mount visibility Alexey Gladkov
@ 2020-07-27 14:14 ` Alexey Gladkov
  2020-07-27 14:14 ` [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
  1 sibling, 0 replies; 10+ messages in thread
From: Alexey Gladkov @ 2020-07-27 14:14 UTC (permalink / raw)
  To: LKML
  Cc: Linux FS Devel, Alexander Viro, Alexey Gladkov,
	Eric W . Biederman, Kees Cook

Allow to mount of procfs with subset=pid option even if the entire
procfs is not fully accessible to the user.

Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
 fs/namespace.c     | 27 ++++++++++++++++-----------
 fs/proc/root.c     | 16 +++++++++-------
 include/linux/fs.h |  1 +
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4a0f600a3328..ab9d607921da 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3949,18 +3949,23 @@ static bool mnt_already_visible(struct mnt_namespace *ns,
 		    ((mnt_flags & MNT_ATIME_MASK) != (new_flags & MNT_ATIME_MASK)))
 			continue;
 
-		/* This mount is not fully visible if there are any
-		 * locked child mounts that cover anything except for
-		 * empty directories.
+		/* If this filesystem is completely dynamic, then it
+		 * makes no sense to check for any child mounts.
 		 */
-		list_for_each_entry(child, &mnt->mnt_mounts, mnt_child) {
-			struct inode *inode = child->mnt_mountpoint->d_inode;
-			/* Only worry about locked mounts */
-			if (!(child->mnt.mnt_flags & MNT_LOCKED))
-				continue;
-			/* Is the directory permanetly empty? */
-			if (!is_empty_dir_inode(inode))
-				goto next;
+		if (!(sb->s_iflags & SB_I_DYNAMIC)) {
+			/* This mount is not fully visible if there are any
+			 * locked child mounts that cover anything except for
+			 * empty directories.
+			 */
+			list_for_each_entry(child, &mnt->mnt_mounts, mnt_child) {
+				struct inode *inode = child->mnt_mountpoint->d_inode;
+				/* Only worry about locked mounts */
+				if (!(child->mnt.mnt_flags & MNT_LOCKED))
+					continue;
+				/* Is the directory permanetly empty? */
+				if (!is_empty_dir_inode(inode))
+					goto next;
+			}
 		}
 		/* Preserve the locked attributes */
 		*new_mnt_flags |= mnt_flags & (MNT_LOCK_READONLY | \
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 5e444d4f9717..c6bf74de1906 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -145,18 +145,21 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	return 0;
 }
 
-static void proc_apply_options(struct proc_fs_info *fs_info,
+static void proc_apply_options(struct super_block *s,
 			       struct fs_context *fc,
 			       struct user_namespace *user_ns)
 {
 	struct proc_fs_context *ctx = fc->fs_private;
+	struct proc_fs_info *fs_info = proc_sb_info(s);
 
 	if (ctx->mask & (1 << Opt_gid))
 		fs_info->pid_gid = make_kgid(user_ns, ctx->gid);
 	if (ctx->mask & (1 << Opt_hidepid))
 		fs_info->hide_pid = ctx->hidepid;
-	if (ctx->mask & (1 << Opt_subset))
+	if (ctx->mask & (1 << Opt_subset)) {
 		fs_info->pidonly = ctx->pidonly;
+		s->s_iflags |= SB_I_DYNAMIC;
+	}
 }
 
 static int proc_fill_super(struct super_block *s, struct fs_context *fc)
@@ -170,9 +173,6 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 	if (!fs_info)
 		return -ENOMEM;
 
-	fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
-	proc_apply_options(fs_info, fc, current_user_ns());
-
 	/* 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 |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
@@ -183,6 +183,9 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_gran = 1;
 	s->s_fs_info = fs_info;
 
+	fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
+	proc_apply_options(s, fc, current_user_ns());
+
 	/*
 	 * procfs isn't actually a stacking filesystem; however, there is
 	 * too much magic going on inside it to permit stacking things on
@@ -216,11 +219,10 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 static int proc_reconfigure(struct fs_context *fc)
 {
 	struct super_block *sb = fc->root->d_sb;
-	struct proc_fs_info *fs_info = proc_sb_info(sb);
 
 	sync_filesystem(sb);
 
-	proc_apply_options(fs_info, fc, current_user_ns());
+	proc_apply_options(sb, fc, current_user_ns());
 	return 0;
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f5abba86107d..aff5ed9e8f82 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1413,6 +1413,7 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
 #define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
 #define SB_I_UNTRUSTED_MOUNTER		0x00000040
+#define SB_I_DYNAMIC			0x00000080
 
 #define SB_I_SKIP_SYNC	0x00000100	/* Skip superblock at global sync */
 
-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-07-27 14:14 [PATCH v1 0/2] proc: Relax check of mount visibility Alexey Gladkov
  2020-07-27 14:14 ` [PATCH v1 1/2] " Alexey Gladkov
@ 2020-07-27 14:14 ` Alexey Gladkov
  2020-07-27 16:29   ` Eric W. Biederman
  1 sibling, 1 reply; 10+ messages in thread
From: Alexey Gladkov @ 2020-07-27 14:14 UTC (permalink / raw)
  To: LKML
  Cc: Linux FS Devel, Alexander Viro, Alexey Gladkov,
	Eric W . Biederman, Kees Cook

Show /proc/self/net only for CAP_NET_ADMIN if procfs is mounted with
subset=pid option in user namespace. This is done to avoid possible
information leakage.

Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
 fs/proc/proc_net.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index dba63b2429f0..11fa2c4b3529 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -275,6 +275,12 @@ static struct net *get_proc_task_net(struct inode *dir)
 	struct task_struct *task;
 	struct nsproxy *ns;
 	struct net *net = NULL;
+	struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb);
+
+	if ((fs_info->pidonly == PROC_PIDONLY_ON) &&
+	    (current_user_ns() != &init_user_ns) &&
+	    !capable(CAP_NET_ADMIN))
+		return net;
 
 	rcu_read_lock();
 	task = pid_task(proc_pid(dir), PIDTYPE_PID);
-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-07-27 14:14 ` [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
@ 2020-07-27 16:29   ` Eric W. Biederman
  2020-07-28 13:54     ` Alexey Gladkov
  2020-07-31 16:10     ` [PATCH v2 " Alexey Gladkov
  0 siblings, 2 replies; 10+ messages in thread
From: Eric W. Biederman @ 2020-07-27 16:29 UTC (permalink / raw)
  To: Alexey Gladkov
  Cc: LKML, Linux FS Devel, Alexander Viro, Alexey Gladkov, Kees Cook

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

> Show /proc/self/net only for CAP_NET_ADMIN if procfs is mounted with
> subset=pid option in user namespace. This is done to avoid possible
> information leakage.
>
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
>  fs/proc/proc_net.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
> index dba63b2429f0..11fa2c4b3529 100644
> --- a/fs/proc/proc_net.c
> +++ b/fs/proc/proc_net.c
> @@ -275,6 +275,12 @@ static struct net *get_proc_task_net(struct inode *dir)
>  	struct task_struct *task;
>  	struct nsproxy *ns;
>  	struct net *net = NULL;
> +	struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb);
> +
> +	if ((fs_info->pidonly == PROC_PIDONLY_ON) &&
> +	    (current_user_ns() != &init_user_ns) &&
> +	    !capable(CAP_NET_ADMIN))
> +		return net;
>
>  	rcu_read_lock();
>  	task = pid_task(proc_pid(dir), PIDTYPE_PID);

Hmm.

I see 3 options going forward.

1) We just make PROC_PIDONLY_ON mean the net directory does not exist.
   No permission checks just always fail.

2) Move the permission checks into opendir/readdir and whichever
   is the appropriate method there and always allow the dentries
   to be cached.

3) Simply cache the mounters credentials and make access to the
   net directories contingent of the permisions of the mounter of
   proc.  Something like the code below.

static struct net *get_proc_task_net(struct inode *dir)
{
	struct task_struct *task;
	struct nsproxy *ns;
	struct net *net = NULL;

	rcu_read_lock();
	task = pid_task(proc_pid(dir), PIDTYPE_PID);
	if (task != NULL) {
		task_lock(task);
		ns = task->nsproxy;
		if (ns != NULL)
			net = get_net(ns->net_ns);
		task_unlock(task);
	}
	rcu_read_unlock();
	if ((fs_info->pidonly == PROC_PIDONLY_ON) &&
            !security_capable(fs_info->mounter_cred,
			      net->user_ns, CAP_SYS_ADMIN,
			      CAP_OPT_NONE)) {
		put_net(net);
		net = NULL;
	}
	return net;
}

Eric

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-07-27 16:29   ` Eric W. Biederman
@ 2020-07-28 13:54     ` Alexey Gladkov
  2020-07-31 16:10     ` [PATCH v2 " Alexey Gladkov
  1 sibling, 0 replies; 10+ messages in thread
From: Alexey Gladkov @ 2020-07-28 13:54 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: LKML, Linux FS Devel, Alexander Viro, Kees Cook

On Mon, Jul 27, 2020 at 11:29:36AM -0500, Eric W. Biederman wrote:
> Alexey Gladkov <gladkov.alexey@gmail.com> writes:
> 
> > Show /proc/self/net only for CAP_NET_ADMIN if procfs is mounted with
> > subset=pid option in user namespace. This is done to avoid possible
> > information leakage.
> >
> > Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> > ---
> >  fs/proc/proc_net.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
> > index dba63b2429f0..11fa2c4b3529 100644
> > --- a/fs/proc/proc_net.c
> > +++ b/fs/proc/proc_net.c
> > @@ -275,6 +275,12 @@ static struct net *get_proc_task_net(struct inode *dir)
> >  	struct task_struct *task;
> >  	struct nsproxy *ns;
> >  	struct net *net = NULL;
> > +	struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb);
> > +
> > +	if ((fs_info->pidonly == PROC_PIDONLY_ON) &&
> > +	    (current_user_ns() != &init_user_ns) &&
> > +	    !capable(CAP_NET_ADMIN))
> > +		return net;
> >
> >  	rcu_read_lock();
> >  	task = pid_task(proc_pid(dir), PIDTYPE_PID);
> 
> Hmm.
> 
> I see 3 options going forward.
> 
> 1) We just make PROC_PIDONLY_ON mean the net directory does not exist.
>    No permission checks just always fail.

I think it's wrong. Now if someone mounts a fully visible procfs then they
can see this directory. Hiding this directory completely will change the
current behavior.

> 2) Move the permission checks into opendir/readdir and whichever
>    is the appropriate method there and always allow the dentries
>    to be cached.

At first I did so, but then I transferred this check to get_proc_task_net
because if this function does not return anything, then 'net' directory
will exist but will simply be empty.

This allowed us to get rid of unnecessary wrappers for opendir/lookup.

> 3) Simply cache the mounters credentials and make access to the
>    net directories contingent of the permisions of the mounter of
>    proc.  Something like the code below.

Interesting idea. I like that :)

> static struct net *get_proc_task_net(struct inode *dir)
> {
> 	struct task_struct *task;
> 	struct nsproxy *ns;
> 	struct net *net = NULL;
> 
> 	rcu_read_lock();
> 	task = pid_task(proc_pid(dir), PIDTYPE_PID);
> 	if (task != NULL) {
> 		task_lock(task);
> 		ns = task->nsproxy;
> 		if (ns != NULL)
> 			net = get_net(ns->net_ns);
> 		task_unlock(task);
> 	}
> 	rcu_read_unlock();
> 	if ((fs_info->pidonly == PROC_PIDONLY_ON) &&

Is this check necessary? I mean, isn't it worth extending this check to
other cases?

>             !security_capable(fs_info->mounter_cred,
> 			      net->user_ns, CAP_SYS_ADMIN,
> 			      CAP_OPT_NONE)) {
> 		put_net(net);
> 		net = NULL;
> 	}
> 	return net;
> }
> 
> Eric
> 

-- 
Rgrds, legion


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-07-27 16:29   ` Eric W. Biederman
  2020-07-28 13:54     ` Alexey Gladkov
@ 2020-07-31 16:10     ` Alexey Gladkov
  1 sibling, 0 replies; 10+ messages in thread
From: Alexey Gladkov @ 2020-07-31 16:10 UTC (permalink / raw)
  To: LKML
  Cc: Linux FS Devel, Alexander Viro, Alexey Gladkov,
	Eric W . Biederman, Kees Cook

Cache the mounters credentials and make access to the net directories
contingent of the permissions of the mounter of proc.

Show /proc/self/net only if mounter has CAP_NET_ADMIN and if proc is
mounted with subset=pid option.

Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
 fs/proc/proc_net.c      | 8 ++++++++
 fs/proc/root.c          | 7 +++++++
 include/linux/proc_fs.h | 1 +
 3 files changed, 16 insertions(+)

diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index dba63b2429f0..c43fc5c907db 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -26,6 +26,7 @@
 #include <linux/uidgid.h>
 #include <net/net_namespace.h>
 #include <linux/seq_file.h>
+#include <linux/security.h>
 
 #include "internal.h"
 
@@ -275,6 +276,7 @@ static struct net *get_proc_task_net(struct inode *dir)
 	struct task_struct *task;
 	struct nsproxy *ns;
 	struct net *net = NULL;
+	struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb);
 
 	rcu_read_lock();
 	task = pid_task(proc_pid(dir), PIDTYPE_PID);
@@ -287,6 +289,12 @@ static struct net *get_proc_task_net(struct inode *dir)
 	}
 	rcu_read_unlock();
 
+	if (net && (fs_info->pidonly == PROC_PIDONLY_ON) &&
+	    security_capable(fs_info->mounter_cred, net->user_ns, CAP_NET_ADMIN, CAP_OPT_NONE) < 0) {
+		put_net(net);
+		net = NULL;
+	}
+
 	return net;
 }
 
diff --git a/fs/proc/root.c b/fs/proc/root.c
index c6bf74de1906..eeeda375cf85 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -184,6 +184,8 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_fs_info = fs_info;
 
 	fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
+	fs_info->mounter_cred = get_cred(fc->cred);
+
 	proc_apply_options(s, fc, current_user_ns());
 
 	/*
@@ -219,9 +221,13 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 static int proc_reconfigure(struct fs_context *fc)
 {
 	struct super_block *sb = fc->root->d_sb;
+	struct proc_fs_info *fs_info = proc_sb_info(sb);
 
 	sync_filesystem(sb);
 
+	put_cred(fs_info->mounter_cred);
+	fs_info->mounter_cred = get_cred(fc->cred);
+
 	proc_apply_options(sb, fc, current_user_ns());
 	return 0;
 }
@@ -276,6 +282,7 @@ static void proc_kill_sb(struct super_block *sb)
 
 	kill_anon_super(sb);
 	put_pid_ns(fs_info->pid_ns);
+	put_cred(fs_info->mounter_cred);
 	kfree(fs_info);
 }
 
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index d1eed1b43651..671c6dafc4ee 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -63,6 +63,7 @@ struct proc_fs_info {
 	kgid_t pid_gid;
 	enum proc_hidepid hide_pid;
 	enum proc_pidonly pidonly;
+	struct cred *mounter_cred;
 };
 
 static inline struct proc_fs_info *proc_sb_info(struct super_block *sb)
-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-08-19 19:14 ` [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
  2020-08-19 21:27   ` kernel test robot
  2020-08-19 21:59   ` kernel test robot
@ 2020-08-19 23:27   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-08-19 23:27 UTC (permalink / raw)
  To: Alexey Gladkov, LKML, Linux FS Devel, Eric W . Biederman
  Cc: kbuild-all, clang-built-linux, Alexey Gladkov, Alexander Viro, Kees Cook

[-- Attachment #1: Type: text/plain, Size: 4201 bytes --]

Hi Alexey,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on kees/for-next/pstore linus/master v5.9-rc1 next-20200819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexey-Gladkov/proc-Relax-check-of-mount-visibility/20200820-031542
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git bcf876870b95592b52519ed4aafcf9d95999bc9c
config: s390-randconfig-r034-20200818 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project b34b1e38381fa4d1b1d9751a6b5233b68e734cfe)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs/proc/root.c:187:24: error: assigning to 'struct cred *' from 'const struct cred *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
           fs_info->mounter_cred = get_cred(fc->cred);
                                 ^ ~~~~~~~~~~~~~~~~~~
   fs/proc/root.c:229:24: error: assigning to 'struct cred *' from 'const struct cred *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
           fs_info->mounter_cred = get_cred(fc->cred);
                                 ^ ~~~~~~~~~~~~~~~~~~
   2 errors generated.

# https://github.com/0day-ci/linux/commit/9c2a0eea7f38b1a4e201b8f2da0c5fd7b423daf9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alexey-Gladkov/proc-Relax-check-of-mount-visibility/20200820-031542
git checkout 9c2a0eea7f38b1a4e201b8f2da0c5fd7b423daf9
vim +187 fs/proc/root.c

   164	
   165	static int proc_fill_super(struct super_block *s, struct fs_context *fc)
   166	{
   167		struct proc_fs_context *ctx = fc->fs_private;
   168		struct inode *root_inode;
   169		struct proc_fs_info *fs_info;
   170		int ret;
   171	
   172		fs_info = kzalloc(sizeof(*fs_info), GFP_KERNEL);
   173		if (!fs_info)
   174			return -ENOMEM;
   175	
   176		/* User space would break if executables or devices appear on proc */
   177		s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
   178		s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
   179		s->s_blocksize = 1024;
   180		s->s_blocksize_bits = 10;
   181		s->s_magic = PROC_SUPER_MAGIC;
   182		s->s_op = &proc_sops;
   183		s->s_time_gran = 1;
   184		s->s_fs_info = fs_info;
   185	
   186		fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
 > 187		fs_info->mounter_cred = get_cred(fc->cred);
   188	
   189		proc_apply_options(s, fc, current_user_ns());
   190	
   191		/*
   192		 * procfs isn't actually a stacking filesystem; however, there is
   193		 * too much magic going on inside it to permit stacking things on
   194		 * top of it
   195		 */
   196		s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
   197	
   198		/* procfs dentries and inodes don't require IO to create */
   199		s->s_shrink.seeks = 0;
   200	
   201		pde_get(&proc_root);
   202		root_inode = proc_get_inode(s, &proc_root);
   203		if (!root_inode) {
   204			pr_err("proc_fill_super: get root inode failed\n");
   205			return -ENOMEM;
   206		}
   207	
   208		s->s_root = d_make_root(root_inode);
   209		if (!s->s_root) {
   210			pr_err("proc_fill_super: allocate dentry failed\n");
   211			return -ENOMEM;
   212		}
   213	
   214		ret = proc_setup_self(s);
   215		if (ret) {
   216			return ret;
   217		}
   218		return proc_setup_thread_self(s);
   219	}
   220	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29945 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-08-19 19:14 ` [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
  2020-08-19 21:27   ` kernel test robot
@ 2020-08-19 21:59   ` kernel test robot
  2020-08-19 23:27   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-08-19 21:59 UTC (permalink / raw)
  To: Alexey Gladkov, LKML, Linux FS Devel, Eric W . Biederman
  Cc: kbuild-all, Alexey Gladkov, Alexander Viro, Kees Cook

[-- Attachment #1: Type: text/plain, Size: 4177 bytes --]

Hi Alexey,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on kees/for-next/pstore linus/master v5.9-rc1 next-20200819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexey-Gladkov/proc-Relax-check-of-mount-visibility/20200820-031542
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git bcf876870b95592b52519ed4aafcf9d95999bc9c
config: m68k-randconfig-s032-20200819 (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-183-gaa6ede3b-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> fs/proc/root.c:187:31: sparse: sparse: incorrect type in assignment (different modifiers) @@     expected struct cred *mounter_cred @@     got struct cred const * @@
>> fs/proc/root.c:187:31: sparse:     expected struct cred *mounter_cred
>> fs/proc/root.c:187:31: sparse:     got struct cred const *
   fs/proc/root.c:229:31: sparse: sparse: incorrect type in assignment (different modifiers) @@     expected struct cred *mounter_cred @@     got struct cred const * @@
   fs/proc/root.c:229:31: sparse:     expected struct cred *mounter_cred
   fs/proc/root.c:229:31: sparse:     got struct cred const *

# https://github.com/0day-ci/linux/commit/9c2a0eea7f38b1a4e201b8f2da0c5fd7b423daf9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alexey-Gladkov/proc-Relax-check-of-mount-visibility/20200820-031542
git checkout 9c2a0eea7f38b1a4e201b8f2da0c5fd7b423daf9
vim +187 fs/proc/root.c

   164	
   165	static int proc_fill_super(struct super_block *s, struct fs_context *fc)
   166	{
   167		struct proc_fs_context *ctx = fc->fs_private;
   168		struct inode *root_inode;
   169		struct proc_fs_info *fs_info;
   170		int ret;
   171	
   172		fs_info = kzalloc(sizeof(*fs_info), GFP_KERNEL);
   173		if (!fs_info)
   174			return -ENOMEM;
   175	
   176		/* User space would break if executables or devices appear on proc */
   177		s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
   178		s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
   179		s->s_blocksize = 1024;
   180		s->s_blocksize_bits = 10;
   181		s->s_magic = PROC_SUPER_MAGIC;
   182		s->s_op = &proc_sops;
   183		s->s_time_gran = 1;
   184		s->s_fs_info = fs_info;
   185	
   186		fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
 > 187		fs_info->mounter_cred = get_cred(fc->cred);
   188	
   189		proc_apply_options(s, fc, current_user_ns());
   190	
   191		/*
   192		 * procfs isn't actually a stacking filesystem; however, there is
   193		 * too much magic going on inside it to permit stacking things on
   194		 * top of it
   195		 */
   196		s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
   197	
   198		/* procfs dentries and inodes don't require IO to create */
   199		s->s_shrink.seeks = 0;
   200	
   201		pde_get(&proc_root);
   202		root_inode = proc_get_inode(s, &proc_root);
   203		if (!root_inode) {
   204			pr_err("proc_fill_super: get root inode failed\n");
   205			return -ENOMEM;
   206		}
   207	
   208		s->s_root = d_make_root(root_inode);
   209		if (!s->s_root) {
   210			pr_err("proc_fill_super: allocate dentry failed\n");
   211			return -ENOMEM;
   212		}
   213	
   214		ret = proc_setup_self(s);
   215		if (ret) {
   216			return ret;
   217		}
   218		return proc_setup_thread_self(s);
   219	}
   220	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23980 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-08-19 19:14 ` [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
@ 2020-08-19 21:27   ` kernel test robot
  2020-08-19 21:59   ` kernel test robot
  2020-08-19 23:27   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-08-19 21:27 UTC (permalink / raw)
  To: Alexey Gladkov, LKML, Linux FS Devel, Eric W . Biederman
  Cc: kbuild-all, Alexey Gladkov, Alexander Viro, Kees Cook

[-- Attachment #1: Type: text/plain, Size: 3986 bytes --]

Hi Alexey,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on kees/for-next/pstore linus/master v5.9-rc1 next-20200819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexey-Gladkov/proc-Relax-check-of-mount-visibility/20200820-031542
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git bcf876870b95592b52519ed4aafcf9d95999bc9c
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/proc/root.c: In function 'proc_fill_super':
>> fs/proc/root.c:187:24: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     187 |  fs_info->mounter_cred = get_cred(fc->cred);
         |                        ^
   fs/proc/root.c: In function 'proc_reconfigure':
   fs/proc/root.c:229:24: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     229 |  fs_info->mounter_cred = get_cred(fc->cred);
         |                        ^

# https://github.com/0day-ci/linux/commit/9c2a0eea7f38b1a4e201b8f2da0c5fd7b423daf9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alexey-Gladkov/proc-Relax-check-of-mount-visibility/20200820-031542
git checkout 9c2a0eea7f38b1a4e201b8f2da0c5fd7b423daf9
vim +/const +187 fs/proc/root.c

   164	
   165	static int proc_fill_super(struct super_block *s, struct fs_context *fc)
   166	{
   167		struct proc_fs_context *ctx = fc->fs_private;
   168		struct inode *root_inode;
   169		struct proc_fs_info *fs_info;
   170		int ret;
   171	
   172		fs_info = kzalloc(sizeof(*fs_info), GFP_KERNEL);
   173		if (!fs_info)
   174			return -ENOMEM;
   175	
   176		/* User space would break if executables or devices appear on proc */
   177		s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
   178		s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
   179		s->s_blocksize = 1024;
   180		s->s_blocksize_bits = 10;
   181		s->s_magic = PROC_SUPER_MAGIC;
   182		s->s_op = &proc_sops;
   183		s->s_time_gran = 1;
   184		s->s_fs_info = fs_info;
   185	
   186		fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
 > 187		fs_info->mounter_cred = get_cred(fc->cred);
   188	
   189		proc_apply_options(s, fc, current_user_ns());
   190	
   191		/*
   192		 * procfs isn't actually a stacking filesystem; however, there is
   193		 * too much magic going on inside it to permit stacking things on
   194		 * top of it
   195		 */
   196		s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
   197	
   198		/* procfs dentries and inodes don't require IO to create */
   199		s->s_shrink.seeks = 0;
   200	
   201		pde_get(&proc_root);
   202		root_inode = proc_get_inode(s, &proc_root);
   203		if (!root_inode) {
   204			pr_err("proc_fill_super: get root inode failed\n");
   205			return -ENOMEM;
   206		}
   207	
   208		s->s_root = d_make_root(root_inode);
   209		if (!s->s_root) {
   210			pr_err("proc_fill_super: allocate dentry failed\n");
   211			return -ENOMEM;
   212		}
   213	
   214		ret = proc_setup_self(s);
   215		if (ret) {
   216			return ret;
   217		}
   218		return proc_setup_thread_self(s);
   219	}
   220	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64408 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN
  2020-08-19 19:14 [PATCH v2 0/2] proc: Relax check of mount visibility Alexey Gladkov
@ 2020-08-19 19:14 ` Alexey Gladkov
  2020-08-19 21:27   ` kernel test robot
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Alexey Gladkov @ 2020-08-19 19:14 UTC (permalink / raw)
  To: LKML, Linux FS Devel, Eric W . Biederman
  Cc: Alexey Gladkov, Alexander Viro, Kees Cook

Cache the mounters credentials and make access to the net directories
contingent of the permissions of the mounter of proc.

Show /proc/self/net only if mounter has CAP_NET_ADMIN and if proc is
mounted with subset=pid option.

Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
 fs/proc/proc_net.c      | 8 ++++++++
 fs/proc/root.c          | 7 +++++++
 include/linux/proc_fs.h | 1 +
 3 files changed, 16 insertions(+)

diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index dba63b2429f0..c43fc5c907db 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -26,6 +26,7 @@
 #include <linux/uidgid.h>
 #include <net/net_namespace.h>
 #include <linux/seq_file.h>
+#include <linux/security.h>
 
 #include "internal.h"
 
@@ -275,6 +276,7 @@ static struct net *get_proc_task_net(struct inode *dir)
 	struct task_struct *task;
 	struct nsproxy *ns;
 	struct net *net = NULL;
+	struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb);
 
 	rcu_read_lock();
 	task = pid_task(proc_pid(dir), PIDTYPE_PID);
@@ -287,6 +289,12 @@ static struct net *get_proc_task_net(struct inode *dir)
 	}
 	rcu_read_unlock();
 
+	if (net && (fs_info->pidonly == PROC_PIDONLY_ON) &&
+	    security_capable(fs_info->mounter_cred, net->user_ns, CAP_NET_ADMIN, CAP_OPT_NONE) < 0) {
+		put_net(net);
+		net = NULL;
+	}
+
 	return net;
 }
 
diff --git a/fs/proc/root.c b/fs/proc/root.c
index c6bf74de1906..eeeda375cf85 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -184,6 +184,8 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_fs_info = fs_info;
 
 	fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
+	fs_info->mounter_cred = get_cred(fc->cred);
+
 	proc_apply_options(s, fc, current_user_ns());
 
 	/*
@@ -219,9 +221,13 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 static int proc_reconfigure(struct fs_context *fc)
 {
 	struct super_block *sb = fc->root->d_sb;
+	struct proc_fs_info *fs_info = proc_sb_info(sb);
 
 	sync_filesystem(sb);
 
+	put_cred(fs_info->mounter_cred);
+	fs_info->mounter_cred = get_cred(fc->cred);
+
 	proc_apply_options(sb, fc, current_user_ns());
 	return 0;
 }
@@ -276,6 +282,7 @@ static void proc_kill_sb(struct super_block *sb)
 
 	kill_anon_super(sb);
 	put_pid_ns(fs_info->pid_ns);
+	put_cred(fs_info->mounter_cred);
 	kfree(fs_info);
 }
 
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index d1eed1b43651..671c6dafc4ee 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -63,6 +63,7 @@ struct proc_fs_info {
 	kgid_t pid_gid;
 	enum proc_hidepid hide_pid;
 	enum proc_pidonly pidonly;
+	struct cred *mounter_cred;
 };
 
 static inline struct proc_fs_info *proc_sb_info(struct super_block *sb)
-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-08-19 23:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 14:14 [PATCH v1 0/2] proc: Relax check of mount visibility Alexey Gladkov
2020-07-27 14:14 ` [PATCH v1 1/2] " Alexey Gladkov
2020-07-27 14:14 ` [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
2020-07-27 16:29   ` Eric W. Biederman
2020-07-28 13:54     ` Alexey Gladkov
2020-07-31 16:10     ` [PATCH v2 " Alexey Gladkov
2020-08-19 19:14 [PATCH v2 0/2] proc: Relax check of mount visibility Alexey Gladkov
2020-08-19 19:14 ` [PATCH v2 2/2] Show /proc/self/net only for CAP_NET_ADMIN Alexey Gladkov
2020-08-19 21:27   ` kernel test robot
2020-08-19 21:59   ` kernel test robot
2020-08-19 23:27   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).