From fa10fed30f2550313a8284365b3e2398526eb42c Mon Sep 17 00:00:00 2001 From: Alexey Gladkov Date: Sun, 19 Apr 2020 16:10:52 +0200 Subject: proc: allow to mount many instances of proc in one pid namespace This patch allows to have multiple procfs instances inside the same pid namespace. The aim here is lightweight sandboxes, and to allow that we have to modernize procfs internals. 1) The main aim of this work is to have on embedded systems one supervisor for apps. Right now we have some lightweight sandbox support, however if we create pid namespacess we have to manages all the processes inside too, where our goal is to be able to run a bunch of apps each one inside its own mount namespace without being able to notice each other. We only want to use mount namespaces, and we want procfs to behave more like a real mount point. 2) Linux Security Modules have multiple ptrace paths inside some subsystems, however inside procfs, the implementation does not guarantee that the ptrace() check which triggers the security_ptrace_check() hook will always run. We have the 'hidepid' mount option that can be used to force the ptrace_may_access() check inside has_pid_permissions() to run. The problem is that 'hidepid' is per pid namespace and not attached to the mount point, any remount or modification of 'hidepid' will propagate to all other procfs mounts. This also does not allow to support Yama LSM easily in desktop and user sessions. Yama ptrace scope which restricts ptrace and some other syscalls to be allowed only on inferiors, can be updated to have a per-task context, where the context will be inherited during fork(), clone() and preserved across execve(). If we support multiple private procfs instances, then we may force the ptrace_may_access() on /proc// to always run inside that new procfs instances. This will allow to specifiy on user sessions if we should populate procfs with pids that the user can ptrace or not. By using Yama ptrace scope, some restricted users will only be able to see inferiors inside /proc, they won't even be able to see their other processes. Some software like Chromium, Firefox's crash handler, Wine and others are already using Yama to restrict which processes can be ptracable. With this change this will give the possibility to restrict /proc// but more importantly this will give desktop users a generic and usuable way to specifiy which users should see all processes and which users can not. Side notes: * This covers the lack of seccomp where it is not able to parse arguments, it is easy to install a seccomp filter on direct syscalls that operate on pids, however /proc// is a Linux ABI using filesystem syscalls. With this change LSMs should be able to analyze open/read/write/close... In the new patch set version I removed the 'newinstance' option as suggested by Eric W. Biederman. Selftest has been added to verify new behavior. Signed-off-by: Alexey Gladkov Reviewed-by: Alexey Dobriyan Reviewed-by: Kees Cook Signed-off-by: Eric W. Biederman --- include/linux/pid_namespace.h | 12 ------------ include/linux/proc_fs.h | 22 +++++++++++++++++++++- 2 files changed, 21 insertions(+), 13 deletions(-) (limited to 'include') diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h index 4956e362e55e..5a5cb45ac57e 100644 --- a/include/linux/pid_namespace.h +++ b/include/linux/pid_namespace.h @@ -17,12 +17,6 @@ struct fs_pin; -enum { /* definitions for pid_namespace's hide_pid field */ - HIDEPID_OFF = 0, - HIDEPID_NO_ACCESS = 1, - HIDEPID_INVISIBLE = 2, -}; - struct pid_namespace { struct kref kref; struct idr idr; @@ -32,17 +26,11 @@ struct pid_namespace { struct kmem_cache *pid_cachep; unsigned int level; struct pid_namespace *parent; -#ifdef CONFIG_PROC_FS - struct dentry *proc_self; - struct dentry *proc_thread_self; -#endif #ifdef CONFIG_BSD_PROCESS_ACCT struct fs_pin *bacct; #endif struct user_namespace *user_ns; struct ucounts *ucounts; - kgid_t pid_gid; - int hide_pid; int reboot; /* group exit code if this pidns was rebooted */ struct ns_common ns; } __randomize_layout; diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 45c05fd9c99d..1b98a41fdd8a 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -42,6 +42,26 @@ struct proc_ops { unsigned long (*proc_get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long); } __randomize_layout; +/* definitions for hide_pid field */ +enum { + HIDEPID_OFF = 0, + HIDEPID_NO_ACCESS = 1, + HIDEPID_INVISIBLE = 2, +}; + +struct proc_fs_info { + struct pid_namespace *pid_ns; + struct dentry *proc_self; /* For /proc/self */ + struct dentry *proc_thread_self; /* For /proc/thread-self */ + kgid_t pid_gid; + int hide_pid; +}; + +static inline struct proc_fs_info *proc_sb_info(struct super_block *sb) +{ + return sb->s_fs_info; +} + #ifdef CONFIG_PROC_FS typedef int (*proc_write_t)(struct file *, char *, size_t); @@ -176,7 +196,7 @@ int open_related_ns(struct ns_common *ns, /* get the associated pid namespace for a file in procfs */ static inline struct pid_namespace *proc_pid_ns(const struct inode *inode) { - return inode->i_sb->s_fs_info; + return proc_sb_info(inode->i_sb)->pid_ns; } #endif /* _LINUX_PROC_FS_H */ -- cgit v1.2.3 From 24a71ce5c47f6b1b3cdacf544cb24220f5c3b7ef Mon Sep 17 00:00:00 2001 From: Alexey Gladkov Date: Sun, 19 Apr 2020 16:10:53 +0200 Subject: proc: instantiate only pids that we can ptrace on 'hidepid=4' mount option If "hidepid=4" mount option is set then do not instantiate pids that we can not ptrace. "hidepid=4" means that procfs should only contain pids that the caller can ptrace. Signed-off-by: Djalal Harouni Signed-off-by: Alexey Gladkov Reviewed-by: Alexey Dobriyan Reviewed-by: Kees Cook Signed-off-by: Eric W. Biederman --- include/linux/proc_fs.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 1b98a41fdd8a..5bdc117ae947 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -47,6 +47,7 @@ enum { HIDEPID_OFF = 0, HIDEPID_NO_ACCESS = 1, HIDEPID_INVISIBLE = 2, + HIDEPID_NOT_PTRACEABLE = 4, /* Limit pids to only ptraceable pids */ }; struct proc_fs_info { -- cgit v1.2.3 From 6814ef2d992af09451bbeda4770daa204461329e Mon Sep 17 00:00:00 2001 From: Alexey Gladkov Date: Sun, 19 Apr 2020 16:10:54 +0200 Subject: proc: add option to mount only a pids subset This allows to hide all files and directories in the procfs that are not related to tasks. Signed-off-by: Alexey Gladkov Reviewed-by: Alexey Dobriyan Reviewed-by: Kees Cook Signed-off-by: Eric W. Biederman --- include/linux/proc_fs.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'include') diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 5bdc117ae947..8bc31ba5cd9c 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -50,12 +50,19 @@ enum { HIDEPID_NOT_PTRACEABLE = 4, /* Limit pids to only ptraceable pids */ }; +/* definitions for proc mount option pidonly */ +enum { + PROC_PIDONLY_OFF = 0, + PROC_PIDONLY_ON = 1, +}; + struct proc_fs_info { struct pid_namespace *pid_ns; struct dentry *proc_self; /* For /proc/self */ struct dentry *proc_thread_self; /* For /proc/thread-self */ kgid_t pid_gid; int hide_pid; + int pidonly; }; static inline struct proc_fs_info *proc_sb_info(struct super_block *sb) -- cgit v1.2.3 From e61bb8b36a287dddc71bdf30be775e7abcaa595c Mon Sep 17 00:00:00 2001 From: Alexey Gladkov Date: Sun, 19 Apr 2020 16:10:57 +0200 Subject: proc: use named enums for better readability Signed-off-by: Alexey Gladkov Reviewed-by: Alexey Dobriyan Reviewed-by: Kees Cook Signed-off-by: Eric W. Biederman --- include/linux/proc_fs.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 8bc31ba5cd9c..2cb424e6f36a 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -43,7 +43,7 @@ struct proc_ops { } __randomize_layout; /* definitions for hide_pid field */ -enum { +enum proc_hidepid { HIDEPID_OFF = 0, HIDEPID_NO_ACCESS = 1, HIDEPID_INVISIBLE = 2, @@ -51,7 +51,7 @@ enum { }; /* definitions for proc mount option pidonly */ -enum { +enum proc_pidonly { PROC_PIDONLY_OFF = 0, PROC_PIDONLY_ON = 1, }; @@ -61,8 +61,8 @@ struct proc_fs_info { struct dentry *proc_self; /* For /proc/self */ struct dentry *proc_thread_self; /* For /proc/thread-self */ kgid_t pid_gid; - int hide_pid; - int pidonly; + enum proc_hidepid hide_pid; + enum proc_pidonly pidonly; }; static inline struct proc_fs_info *proc_sb_info(struct super_block *sb) -- cgit v1.2.3 From 35fc0e3b0bd5be3b059e53ae90c4536ee4922330 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 24 Apr 2020 11:19:10 -0500 Subject: rculist: Add hlists_swap_heads_rcu Using the struct pid to refer to two tasks in de_thread was a clever idea and ultimately too clever, as it has lead to proc_flush_task being called inconsistently. To support rectifying this add hlists_swap_heads_rcu. An hlist primitive that just swaps the hlist heads of two lists. This is exactly what is needed for exchanging the pids of two tasks. Only consideration of correctness of the code has been given, as the caller is expected to be a slowpath. Link: https://lore.kernel.org/lkml/87mu6vajnq.fsf_-_@x220.int.ebiederm.org/ Acked-by: Linus Torvalds Acked-by: Oleg Nesterov Signed-off-by: "Eric W. Biederman" --- include/linux/rculist.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'include') diff --git a/include/linux/rculist.h b/include/linux/rculist.h index 8214cdc715f2..67867e0d4cec 100644 --- a/include/linux/rculist.h +++ b/include/linux/rculist.h @@ -506,6 +506,27 @@ static inline void hlist_replace_rcu(struct hlist_node *old, WRITE_ONCE(old->pprev, LIST_POISON2); } +/** + * hlists_swap_heads_rcu - swap the lists the hlist heads point to + * @left: The hlist head on the left + * @right: The hlist head on the right + * + * The lists start out as [@left ][node1 ... ] and + [@right ][node2 ... ] + * The lists end up as [@left ][node2 ... ] + * [@right ][node1 ... ] + */ +static inline void hlists_swap_heads_rcu(struct hlist_head *left, struct hlist_head *right) +{ + struct hlist_node *node1 = left->first; + struct hlist_node *node2 = right->first; + + rcu_assign_pointer(left->first, node2); + rcu_assign_pointer(right->first, node1); + WRITE_ONCE(node2->pprev, &left->first); + WRITE_ONCE(node1->pprev, &right->first); +} + /* * return the first or the next element in an RCU protected hlist */ -- cgit v1.2.3 From 6b03d1304a32dc8450c7516000a0fe07bef7c446 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 19 Apr 2020 06:35:02 -0500 Subject: proc: Ensure we see the exit of each process tid exactly once When the thread group leader changes during exec and the old leaders thread is reaped proc_flush_pid will flush the dentries for the entire process because the leader still has it's original pid. Fix this by exchanging the pids in an rcu safe manner, and wrapping the code to do that up in a helper exchange_tids. When I removed switch_exec_pids and introduced this behavior in d73d65293e3e ("[PATCH] pidhash: kill switch_exec_pids") there really was nothing that cared as flushing happened with the cached dentry and de_thread flushed both of them on exec. This lack of fully exchanging pids became a problem a few months later when I introduced 48e6484d4902 ("[PATCH] proc: Rewrite the proc dentry flush on exit optimization"). Which overlooked the de_thread case was no longer swapping pids, and I was looking up proc dentries by task->pid. The current behavior isn't properly a bug as everything in proc will continue to work correctly just a little bit less efficiently. Fix this just so there are no little surprise corner cases waiting to bite people. -- Oleg points out this could be an issue in next_tgid in proc where has_group_leader_pid is called, and reording some of the assignments should fix that. -- Oleg points out this will break the 10 year old hack in __exit_signal.c > /* > * This can only happen if the caller is de_thread(). > * FIXME: this is the temporary hack, we should teach > * posix-cpu-timers to handle this case correctly. > */ > if (unlikely(has_group_leader_pid(tsk))) > posix_cpu_timers_exit_group(tsk); The code in next_tgid has been changed to use PIDTYPE_TGID, and the posix cpu timers code has been fixed so it does not need the 10 year old hack, so this should be safe to merge now. Link: https://lore.kernel.org/lkml/87h7x3ajll.fsf_-_@x220.int.ebiederm.org/ Acked-by: Linus Torvalds Acked-by: Oleg Nesterov Fixes: 48e6484d4902 ("[PATCH] proc: Rewrite the proc dentry flush on exit optimization"). Signed-off-by: Eric W. Biederman --- include/linux/pid.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/linux/pid.h b/include/linux/pid.h index cc896f0fc4e3..2159ffca63fc 100644 --- a/include/linux/pid.h +++ b/include/linux/pid.h @@ -102,6 +102,7 @@ extern void attach_pid(struct task_struct *task, enum pid_type); extern void detach_pid(struct task_struct *task, enum pid_type); extern void change_pid(struct task_struct *task, enum pid_type, struct pid *pid); +extern void exchange_tids(struct task_struct *task, struct task_struct *old); extern void transfer_pid(struct task_struct *old, struct task_struct *new, enum pid_type); -- cgit v1.2.3 From bbd40fc4816d5329a89397206ece9f1763787a88 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 26 Apr 2020 07:59:55 -0500 Subject: signal: Remove has_group_leader_pid After the introduction of exchange_tids has_group_leader_pid is equivalent to thread_group_leader. After the last couple of cleanups has_group_leader_pid has no more callers. So remove the now unused and redundant has_group_leader_pid. Signed-off-by: "Eric W. Biederman" --- include/linux/sched/signal.h | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'include') diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h index 3e5b090c16d4..0ee5e696c5d8 100644 --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -654,17 +654,6 @@ static inline bool thread_group_leader(struct task_struct *p) return p->exit_signal >= 0; } -/* Do to the insanities of de_thread it is possible for a process - * to have the pid of the thread group leader without actually being - * the thread group leader. For iteration through the pids in proc - * all we care about is that we have a task with the appropriate - * pid, we don't actually care if we have the right task. - */ -static inline bool has_group_leader_pid(struct task_struct *p) -{ - return task_pid(p) == task_tgid(p); -} - static inline bool same_thread_group(struct task_struct *p1, struct task_struct *p2) { -- cgit v1.2.3 From 9d78edeaec759f997c303f286ecd39daee166f2a Mon Sep 17 00:00:00 2001 From: Alexey Gladkov Date: Mon, 18 May 2020 20:07:38 +0200 Subject: proc: proc_pid_ns takes super_block as an argument syzbot found that touch /proc/testfile causes NULL pointer dereference at tomoyo_get_local_path() because inode of the dentry is NULL. Before c59f415a7cb6, Tomoyo received pid_ns from proc's s_fs_info directly. Since proc_pid_ns() can only work with inode, using it in the tomoyo_get_local_path() was wrong. To avoid creating more functions for getting proc_ns, change the argument type of the proc_pid_ns() function. Then, Tomoyo can use the existing super_block to get pid_ns. Link: https://lkml.kernel.org/r/0000000000002f0c7505a5b0e04c@google.com Link: https://lkml.kernel.org/r/20200518180738.2939611-1-gladkov.alexey@gmail.com Reported-by: syzbot+c1af344512918c61362c@syzkaller.appspotmail.com Fixes: c59f415a7cb6 ("Use proc_pid_ns() to get pid_namespace from the proc superblock") Signed-off-by: Alexey Gladkov Signed-off-by: Eric W. Biederman --- include/linux/proc_fs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 2cb424e6f36a..6ec524d8842c 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -202,9 +202,9 @@ int open_related_ns(struct ns_common *ns, struct ns_common *(*get_ns)(struct ns_common *ns)); /* get the associated pid namespace for a file in procfs */ -static inline struct pid_namespace *proc_pid_ns(const struct inode *inode) +static inline struct pid_namespace *proc_pid_ns(struct super_block *sb) { - return proc_sb_info(inode->i_sb)->pid_ns; + return proc_sb_info(sb)->pid_ns; } #endif /* _LINUX_PROC_FS_H */ -- cgit v1.2.3