diff mbox

[v5,3/7] proc: add helpers to set and get proc hidepid and gid mount options

Message ID 20180511093528.GA1250@comp-core-i7-2640m-0182e6 (mailing list archive)
State New, archived
Headers show

Commit Message

Alexey Gladkov May 11, 2018, 9:35 a.m. UTC 
From: Djalal Harouni <tixxdz@gmail.com>

This is a cleaning patch to add helpers to set and get proc mount
options instead of directly using them. This make it easy to track
what's happening and easy to update in future.

Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
 fs/proc/base.c          | 16 +++++++++-------
 fs/proc/inode.c         |  5 +++--
 fs/proc/internal.h      |  2 +-
 fs/proc/root.c          | 15 ++++++++++-----
 include/linux/proc_fs.h | 44 ++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 65 insertions(+), 17 deletions(-)

Comments

kernel test robot May 20, 2018, 7:53 a.m. UTC | #1 
Hi Djalal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5]
[cannot apply to next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alexey-Gladkov/proc-add-proc_fs_info-struct-to-store-proc-information/20180512-002936
config: xtensa-allnoconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
        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
        make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   init/do_mounts.o: In function `proc_fs_pid_gid':
>> do_mounts.c:(.text+0x3c): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   arch/xtensa/kernel/setup.o: In function `proc_fs_pid_gid':
   setup.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/fork.o: In function `proc_fs_pid_gid':
   fork.c:(.text+0x2a8): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/exec_domain.o: In function `proc_fs_pid_gid':
   exec_domain.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/cpu.o: In function `proc_fs_pid_gid':
   cpu.c:(.text+0xf0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/exit.o: In function `proc_fs_pid_gid':
   exit.c:(.text+0x188): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/resource.o: In function `proc_fs_pid_gid':
   resource.c:(.text+0x344): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sysctl.o: In function `proc_fs_pid_gid':
   sysctl.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/pid.o: In function `proc_fs_pid_gid':
   pid.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/core.o: In function `proc_fs_pid_gid':
   core.c:(.text+0x6c8): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/loadavg.o: In function `proc_fs_pid_gid':
   loadavg.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/clock.o: In function `proc_fs_pid_gid':
   clock.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/cputime.o: In function `proc_fs_pid_gid':
   cputime.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/idle.o: In function `proc_fs_pid_gid':
   idle.c:(.text+0x98): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/fair.o: In function `proc_fs_pid_gid':
   fair.c:(.text+0xac0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/rt.o: In function `proc_fs_pid_gid':
   rt.c:(.text+0x734): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/deadline.o: In function `proc_fs_pid_gid':
   deadline.c:(.text+0x10b0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/wait.o: In function `proc_fs_pid_gid':
   wait.c:(.text+0x120): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/wait_bit.o: In function `proc_fs_pid_gid':
   wait_bit.c:(.text+0x78): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/swait.o: In function `proc_fs_pid_gid':
   swait.c:(.text+0x24): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/sched/completion.o: In function `proc_fs_pid_gid':
   completion.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   kernel/time/timer_list.o: In function `proc_fs_pid_gid':
   timer_list.c:(.text+0x1c): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   mm/vmstat.o: In function `proc_fs_pid_gid':
   vmstat.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   mm/slab_common.o: In function `proc_fs_pid_gid':
   slab_common.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   mm/vmalloc.o: In function `proc_fs_pid_gid':
   vmalloc.c:(.text+0x514): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   mm/slub.o: In function `proc_fs_pid_gid':
   slub.c:(.text+0x8a4): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   fs/filesystems.o: In function `proc_fs_pid_gid':
   filesystems.c:(.text+0x44): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here
   drivers/char/misc.o: In function `proc_fs_pid_gid':
   misc.c:(.text+0xe0): multiple definition of `proc_fs_pid_gid'
   init/main.o:main.c:(.text+0x30): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/fs/proc/base.c b/fs/proc/base.c
index e0c2afc..57fc895 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -684,13 +684,16 @@  int proc_setattr(struct dentry *dentry, struct iattr *attr)
  * May current process learn task's sched/cmdline info (for hide_pid_min=1)
  * or euid/egid (for hide_pid_min=2)?
  */
-static bool has_pid_permissions(struct pid_namespace *pid,
+static bool has_pid_permissions(struct proc_fs_info *fs_info,
 				 struct task_struct *task,
 				 int hide_pid_min)
 {
-	if (pid->hide_pid < hide_pid_min)
+	int hide_pid = proc_fs_hide_pid(fs_info);
+	kgid_t gid = proc_fs_pid_gid(fs_info);
+
+	if (hide_pid < hide_pid_min)
 		return true;
-	if (in_group_p(pid->pid_gid))
+	if (in_group_p(gid))
 		return true;
 	return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
 }
@@ -706,7 +709,7 @@  static int proc_pid_permission(struct inode *inode, int mask)
 	task = get_proc_task(inode);
 	if (!task)
 		return -ESRCH;
-	has_perms = has_pid_permissions(pid, task, HIDEPID_NO_ACCESS);
+	has_perms = has_pid_permissions(fs_info, task, HIDEPID_NO_ACCESS);
 	put_task_struct(task);
 
 	if (!has_perms) {
@@ -1786,7 +1789,6 @@  int pid_getattr(const struct path *path, struct kstat *stat,
 	struct task_struct *task;
 	struct inode *inode = d_inode(path->dentry);
 	struct proc_fs_info *fs_info = proc_sb(inode->i_sb);
-	struct pid_namespace *pid = fs_info->pid_ns;
 
 	generic_fillattr(inode, stat);
 
@@ -1795,7 +1797,7 @@  int pid_getattr(const struct path *path, struct kstat *stat,
 	stat->gid = GLOBAL_ROOT_GID;
 	task = pid_task(proc_pid(inode), PIDTYPE_PID);
 	if (task) {
-		if (!has_pid_permissions(pid, task, HIDEPID_INVISIBLE)) {
+		if (!has_pid_permissions(fs_info, task, HIDEPID_INVISIBLE)) {
 			rcu_read_unlock();
 			/*
 			 * This doesn't prevent learning whether PID exists,
@@ -3272,7 +3274,7 @@  int proc_pid_readdir(struct file *file, struct dir_context *ctx)
 		int len;
 
 		cond_resched();
-		if (!has_pid_permissions(ns, iter.task, HIDEPID_INVISIBLE))
+		if (!has_pid_permissions(fs_info, iter.task, HIDEPID_INVISIBLE))
 			continue;
 
 		len = snprintf(name, sizeof(name), "%u", iter.tgid);
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index e34b89a..9585727 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -494,11 +494,12 @@  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 proc_fs_info *fs_info = proc_sb(s);
-	struct pid_namespace *ns = get_pid_ns(fs_info->pid_ns);
 	struct inode *root_inode;
 	int ret;
 
-	if (!proc_parse_options(data, ns))
+	get_pid_ns(fs_info->pid_ns);
+
+	if (!proc_parse_options(data, fs_info))
 		return -EINVAL;
 
 	/* User space would break if executables or devices appear on proc */
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 0f1692e..facc2d9 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -253,7 +253,7 @@  static inline void proc_tty_init(void) {}
  * root.c
  */
 extern struct proc_dir_entry proc_root;
-extern int proc_parse_options(char *options, struct pid_namespace *pid);
+extern int proc_parse_options(char *options, struct proc_fs_info *fs_info);
 
 extern void proc_self_init(void);
 extern int proc_remount(struct super_block *, int *, char *);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 184c42b..bdea220 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -37,11 +37,12 @@  static const match_table_t tokens = {
 	{Opt_err, NULL},
 };
 
-int proc_parse_options(char *options, struct pid_namespace *pid)
+int proc_parse_options(char *options, struct proc_fs_info *fs_info)
 {
 	char *p;
 	substring_t args[MAX_OPT_ARGS];
 	int option;
+	kgid_t gid;
 
 	if (!options)
 		return 1;
@@ -57,7 +58,12 @@  int proc_parse_options(char *options, struct pid_namespace *pid)
 		case Opt_gid:
 			if (match_int(&args[0], &option))
 				return 0;
-			pid->pid_gid = make_kgid(current_user_ns(), option);
+			gid = make_kgid(current_user_ns(), option);
+			if (!gid_valid(gid)) {
+				pr_err("proc: invalid gid mount option.\n");
+				return 0;
+			}
+			proc_fs_set_pid_gid(fs_info, gid);
 			break;
 		case Opt_hidepid:
 			if (match_int(&args[0], &option))
@@ -67,7 +73,7 @@  int proc_parse_options(char *options, struct pid_namespace *pid)
 				pr_err("proc: hidepid value must be between 0 and 2.\n");
 				return 0;
 			}
-			pid->hide_pid = option;
+			proc_fs_set_hide_pid(fs_info, option);
 			break;
 		default:
 			pr_err("proc: unrecognized mount option \"%s\" "
@@ -82,10 +88,9 @@  int proc_parse_options(char *options, struct pid_namespace *pid)
 int proc_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct proc_fs_info *fs_info = proc_sb(sb);
-	struct pid_namespace *pid = fs_info->pid_ns;
 
 	sync_filesystem(sb);
-	return !proc_parse_options(data, pid);
+	return !proc_parse_options(data, fs_info);
 }
 
 static int proc_test_super(struct super_block *s, void *data)
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 5e461f6..a7fc6a1 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -8,6 +8,10 @@ 
 #include <linux/types.h>
 #include <linux/fs.h>
 #include <linux/refcount.h>
+#include <linux/pid_namespace.h>
+
+struct proc_dir_entry;
+struct pid_namespace;
 
 struct proc_fs_info {
 	struct pid_namespace *pid_ns;
@@ -15,8 +19,6 @@  struct proc_fs_info {
 	struct dentry *proc_thread_self; /* For /proc/thread-self/ */
 };
 
-struct proc_dir_entry;
-
 #ifdef CONFIG_PROC_FS
 
 static inline struct proc_fs_info *proc_sb(struct super_block *sb)
@@ -24,6 +26,26 @@  static inline struct proc_fs_info *proc_sb(struct super_block *sb)
 	return sb->s_fs_info;
 }
 
+static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_pid)
+{
+	fs_info->pid_ns->hide_pid = hide_pid;
+}
+
+static inline void proc_fs_set_pid_gid(struct proc_fs_info *fs_info, kgid_t gid)
+{
+	fs_info->pid_ns->pid_gid = gid;
+}
+
+static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
+{
+	return fs_info->pid_ns->hide_pid;
+}
+
+static inline kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
+{
+	return fs_info->pid_ns->pid_gid;
+}
+
 extern void proc_root_init(void);
 extern void proc_flush_task(struct task_struct *);
 
@@ -60,6 +82,24 @@  static inline void proc_flush_task(struct task_struct *task)
 {
 }
 
+static inline void proc_fs_set_hide_pid(struct proc_fs_info *fs_info, int hide_pid)
+{
+}
+
+static inline void proc_fs_set_pid_gid(struct proc_info_fs *fs_info, kgid_t gid)
+{
+}
+
+static inline int proc_fs_hide_pid(struct proc_fs_info *fs_info)
+{
+	return 0;
+}
+
+extern kgid_t proc_fs_pid_gid(struct proc_fs_info *fs_info)
+{
+	return GLOBAL_ROOT_GID;
+}
+
 extern inline struct proc_fs_info *proc_sb(struct super_block *sb) { return NULL;}
 static inline struct proc_dir_entry *proc_symlink(const char *name,
 		struct proc_dir_entry *parent,const char *dest) { return NULL;}