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.
Changelog
---------
v4:
* Set SB_I_DYNAMIC only if pidonly is set.
* Add an error message if subset=pid is canceled during remount.
v3:
* Add 'const' to struct cred *mounter_cred (fix kernel test robot warning).
v2:
* cache the mounters credentials and make access to the net directories
contingent of the permissions of the mounter of procfs.
--
Alexey Gladkov (5):
docs: proc: add documentation about mount restrictions
proc: Show /proc/self/net only for CAP_NET_ADMIN
proc: Disable cancellation of subset=pid option
proc: Relax check of mount visibility
docs: proc: add documentation about relaxing visibility restrictions
Documentation/filesystems/proc.rst | 18 ++++++++++++++++++
fs/namespace.c | 27 ++++++++++++++++-----------
fs/proc/proc_net.c | 8 ++++++++
fs/proc/root.c | 25 +++++++++++++++++++------
include/linux/fs.h | 1 +
include/linux/proc_fs.h | 1 +
6 files changed, 63 insertions(+), 17 deletions(-)
--
2.29.2
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 <[email protected]>
---
fs/proc/proc_net.c | 8 ++++++++
fs/proc/root.c | 5 +++++
include/linux/proc_fs.h | 1 +
3 files changed, 14 insertions(+)
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index 18601042af99..a198f74cdb3b 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"
@@ -259,6 +260,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);
@@ -271,6 +273,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 5e444d4f9717..6a75ac717455 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -171,6 +171,7 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
return -ENOMEM;
fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
+ fs_info->mounter_cred = get_cred(fc->cred);
proc_apply_options(fs_info, fc, current_user_ns());
/* User space would break if executables or devices appear on proc */
@@ -220,6 +221,9 @@ static int proc_reconfigure(struct fs_context *fc)
sync_filesystem(sb);
+ put_cred(fs_info->mounter_cred);
+ fs_info->mounter_cred = get_cred(fc->cred);
+
proc_apply_options(fs_info, fc, current_user_ns());
return 0;
}
@@ -274,6 +278,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 000cc0533c33..ffa871941bd0 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -64,6 +64,7 @@ struct proc_fs_info {
kgid_t pid_gid;
enum proc_hidepid hide_pid;
enum proc_pidonly pidonly;
+ const struct cred *mounter_cred;
};
static inline struct proc_fs_info *proc_sb_info(struct super_block *sb)
--
2.29.2
Signed-off-by: Alexey Gladkov <[email protected]>
---
Documentation/filesystems/proc.rst | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 2fa69f710e2a..3daf0e7d1071 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -50,6 +50,7 @@ fixes/update part 1.1 Stefani Seibold <[email protected]> June 9 2009
4 Configuring procfs
4.1 Mount options
+ 4.2 Mount restrictions
5 Filesystem behavior
@@ -2175,6 +2176,21 @@ information about processes information, just add identd to this group.
subset=pid hides all top level files and directories in the procfs that
are not related to tasks.
+4.2 Mount restrictions
+--------------------------
+
+The procfs can be mounted without any special restrictions if user namespace is
+not used. You only need to have permission to mount (CAP_SYS_ADMIN).
+
+If you are inside the user namespace, the kernel checks the instances of procfs
+available to you and will not allow procfs to be mounted if:
+
+ 1. There is a bind mount of part of procfs visible. Whoever mounts should be
+ able to see the entire filesystem.
+ 2. Mount is prohibited if a new mount overrides the readonly option or family
+ of atime options.
+ 3. If any file or non-empty procfs directory is hidden by another filesystem.
+
Chapter 5: Filesystem behavior
==============================
--
2.29.2
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 <[email protected]>
---
fs/namespace.c | 27 ++++++++++++++++-----------
fs/proc/root.c | 17 ++++++++++-------
include/linux/fs.h | 1 +
3 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 9d33909d0f9e..f9a38584f865 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3971,18 +3971,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 0d20bb67e79a..049d5c125f8f 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 int proc_apply_options(struct proc_fs_info *fs_info,
+static int 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->pidonly != PROC_PIDONLY_ON && fs_info->pidonly == PROC_PIDONLY_ON)
+ if (ctx->pidonly == PROC_PIDONLY_ON)
+ s->s_iflags |= SB_I_DYNAMIC;
+ else if (fs_info->pidonly == PROC_PIDONLY_ON)
return invalf(fc, "proc: subset=pid cannot be unset\n");
fs_info->pidonly = ctx->pidonly;
}
@@ -176,9 +179,6 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
fs_info->mounter_cred = get_cred(fc->cred);
- ret = proc_apply_options(fs_info, fc, current_user_ns());
- if (ret)
- return ret;
/* User space would break if executables or devices appear on proc */
s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
@@ -190,6 +190,10 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
s->s_time_gran = 1;
s->s_fs_info = fs_info;
+ ret = proc_apply_options(s, fc, current_user_ns());
+ if (ret)
+ return ret;
+
/*
* procfs isn't actually a stacking filesystem; however, there is
* too much magic going on inside it to permit stacking things on
@@ -223,14 +227,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);
- return proc_apply_options(fs_info, fc, current_user_ns());
+ return proc_apply_options(sb, fc, current_user_ns());
}
static int proc_get_tree(struct fs_context *fc)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd47deea7c17..2c9a47bad796 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1390,6 +1390,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.29.2
There is no way to remount procfs mountpoint with subset=pid option
without it. This is done in order not to make visible what was hidden
since some checks occur during mount.
This patch makes this limitation explicit and demonstrates the error.
Signed-off-by: Alexey Gladkov <[email protected]>
---
fs/proc/root.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 6a75ac717455..0d20bb67e79a 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -145,7 +145,7 @@ 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 int proc_apply_options(struct proc_fs_info *fs_info,
struct fs_context *fc,
struct user_namespace *user_ns)
{
@@ -155,8 +155,12 @@ static void proc_apply_options(struct proc_fs_info *fs_info,
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)) {
+ if (ctx->pidonly != PROC_PIDONLY_ON && fs_info->pidonly == PROC_PIDONLY_ON)
+ return invalf(fc, "proc: subset=pid cannot be unset\n");
fs_info->pidonly = ctx->pidonly;
+ }
+ return 0;
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
@@ -172,7 +176,9 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
fs_info->mounter_cred = get_cred(fc->cred);
- proc_apply_options(fs_info, fc, current_user_ns());
+ ret = proc_apply_options(fs_info, fc, current_user_ns());
+ if (ret)
+ return ret;
/* User space would break if executables or devices appear on proc */
s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
@@ -224,8 +230,7 @@ static int proc_reconfigure(struct fs_context *fc)
put_cred(fs_info->mounter_cred);
fs_info->mounter_cred = get_cred(fc->cred);
- proc_apply_options(fs_info, fc, current_user_ns());
- return 0;
+ return proc_apply_options(fs_info, fc, current_user_ns());
}
static int proc_get_tree(struct fs_context *fc)
--
2.29.2
Signed-off-by: Alexey Gladkov <[email protected]>
---
Documentation/filesystems/proc.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 3daf0e7d1071..9d2985a7aad6 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2190,6 +2190,8 @@ available to you and will not allow procfs to be mounted if:
2. Mount is prohibited if a new mount overrides the readonly option or family
of atime options.
3. If any file or non-empty procfs directory is hidden by another filesystem.
+ You can still mount procfs even with overlapped directories if the
+ subset=pid option is used.
Chapter 5: Filesystem behavior
==============================
--
2.29.2
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.12-rc2 next-20210310]
[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/20210311-022252
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32
config: powerpc-randconfig-s032-20210309 (attached as .config)
compiler: powerpc64-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.3-262-g5e674421-dirty
# https://github.com/0day-ci/linux/commit/57a1fff647a507e103bbe22d67c6fe6b54c6a088
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/20210311-022252
git checkout 57a1fff647a507e103bbe22d67c6fe6b54c6a088
# 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=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
fs/proc/root.c: In function 'proc_reconfigure':
>> fs/proc/root.c:233:11: error: 'fs_info' undeclared (first use in this function); did you mean 'qc_info'?
233 | put_cred(fs_info->mounter_cred);
| ^~~~~~~
| qc_info
fs/proc/root.c:233:11: note: each undeclared identifier is reported only once for each function it appears in
vim +233 fs/proc/root.c
97412950b10e64 Vasiliy Kulikov 2012-01-10 226
66f592e2ece038 David Howells 2018-11-01 227 static int proc_reconfigure(struct fs_context *fc)
97412950b10e64 Vasiliy Kulikov 2012-01-10 228 {
66f592e2ece038 David Howells 2018-11-01 229 struct super_block *sb = fc->root->d_sb;
02b9984d640873 Theodore Ts'o 2014-03-13 230
02b9984d640873 Theodore Ts'o 2014-03-13 231 sync_filesystem(sb);
66f592e2ece038 David Howells 2018-11-01 232
b84f25e5938b65 Alexey Gladkov 2021-03-10 @233 put_cred(fs_info->mounter_cred);
b84f25e5938b65 Alexey Gladkov 2021-03-10 234 fs_info->mounter_cred = get_cred(fc->cred);
b84f25e5938b65 Alexey Gladkov 2021-03-10 235
57a1fff647a507 Alexey Gladkov 2021-03-10 236 return proc_apply_options(sb, fc, current_user_ns());
97412950b10e64 Vasiliy Kulikov 2012-01-10 237 }
97412950b10e64 Vasiliy Kulikov 2012-01-10 238
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Wed, Mar 10, 2021 at 07:19:55PM +0100, Alexey Gladkov wrote:
> 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.
I'm sorry about that unfinished patch set. Please ignore it.
> Changelog
> ---------
> v4:
> * Set SB_I_DYNAMIC only if pidonly is set.
> * Add an error message if subset=pid is canceled during remount.
>
> v3:
> * Add 'const' to struct cred *mounter_cred (fix kernel test robot warning).
>
> v2:
> * cache the mounters credentials and make access to the net directories
> contingent of the permissions of the mounter of procfs.
>
> --
>
> Alexey Gladkov (5):
> docs: proc: add documentation about mount restrictions
> proc: Show /proc/self/net only for CAP_NET_ADMIN
> proc: Disable cancellation of subset=pid option
> proc: Relax check of mount visibility
> docs: proc: add documentation about relaxing visibility restrictions
>
> Documentation/filesystems/proc.rst | 18 ++++++++++++++++++
> fs/namespace.c | 27 ++++++++++++++++-----------
> fs/proc/proc_net.c | 8 ++++++++
> fs/proc/root.c | 25 +++++++++++++++++++------
> include/linux/fs.h | 1 +
> include/linux/proc_fs.h | 1 +
> 6 files changed, 63 insertions(+), 17 deletions(-)
>
> --
> 2.29.2
>
--
Rgrds, legion