[rh7,v2] proc/pid: Don't show kernel threads inside Containers

Submitted by Konstantin Khorenko on Feb. 26, 2020, 12:08 p.m.

Details

Message ID 20200226120839.21035-1-khorenko@virtuozzo.com
State New
Series "enable running Kubernetes inside a Container"
Headers show

Commit Message

Konstantin Khorenko Feb. 26, 2020, 12:08 p.m.
We have our home-brew security defence feature from ptrace-ing
processes entered a Container outside.
It's based on "vps_dumpable" field of task->mm and is checked in
__ptrace_may_access() in particular.
Same function __ptrace_may_access() is used when someone tries to
readlink /proc/$PID/ns/$SOMENS and gets -EPERM for kthreads because they
lack task->mm and thus "vps_dumpable" field and we really would like
people not to ptrace kernel threads from inside a Container.

We might enhance the security check, but decided just to make Container
kernel threads invisible (from inside a Container only of course).

https://jira.sw.ru/browse/PSBM-92107

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

v2: * check PF_KTHREAD task flag for kthread detection as not all
      kthreads lack ->mm.
    * Hide kthreads from direct checking (via, say, /proc/$PID) access
      as well.
    * proc entries for kthreads are hidden in procfs mounted in non-init
      pidns despite the namespace of current process (i.e. if you enter
      CT's mount ns only, you still won't be able to check entries of
      CT's kthreads. Check host's procfs for that.)
    * introduce helpers
---
 fs/proc/base.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Patch hide | download patch | download mbox

diff --git a/fs/proc/base.c b/fs/proc/base.c
index dbf5a84b604f3..965a7467c2b4d 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3300,6 +3300,15 @@  static struct dentry *proc_pid_instantiate(struct inode *dir,
 	return error;
 }
 
+static bool is_visible_task_ve(struct pid_namespace *ns, struct task_struct *task)
+{
+	/* Don't show kthreads inside Containers. */
+	if ((task->flags & PF_KTHREAD) && (ns != &init_pid_ns))
+		return false;
+
+	return true;
+}
+
 struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsigned int flags)
 {
 	struct dentry *result = NULL;
@@ -3314,6 +3323,8 @@  struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsign
 	ns = dentry->d_sb->s_fs_info;
 	rcu_read_lock();
 	task = find_task_by_pid_ns(tgid, ns);
+	if (task && !is_visible_task_ve(ns, task))
+		task = NULL;
 	if (task)
 		get_task_struct(task);
 	rcu_read_unlock();
@@ -3410,6 +3421,9 @@  int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
 	for (iter = next_tgid(ns, iter);
 	     iter.task;
 	     iter.tgid += 1, iter = next_tgid(ns, iter)) {
+		if (!is_visible_task_ve(ns, iter.task))
+			continue;
+
 		if (is_visible_task(ns, iter.task))
 			__filldir = filldir;
 		else

Comments

Kirill Tkhai Feb. 26, 2020, 1:46 p.m.
On 26.02.2020 15:08, Konstantin Khorenko wrote:
> We have our home-brew security defence feature from ptrace-ing
> processes entered a Container outside.
> It's based on "vps_dumpable" field of task->mm and is checked in
> __ptrace_may_access() in particular.
> Same function __ptrace_may_access() is used when someone tries to
> readlink /proc/$PID/ns/$SOMENS and gets -EPERM for kthreads because they
> lack task->mm and thus "vps_dumpable" field and we really would like
> people not to ptrace kernel threads from inside a Container.
> 
> We might enhance the security check, but decided just to make Container
> kernel threads invisible (from inside a Container only of course).
> 
> https://jira.sw.ru/browse/PSBM-92107
> 
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
 
> v2: * check PF_KTHREAD task flag for kthread detection as not all
>       kthreads lack ->mm.
>     * Hide kthreads from direct checking (via, say, /proc/$PID) access
>       as well.
>     * proc entries for kthreads are hidden in procfs mounted in non-init
>       pidns despite the namespace of current process (i.e. if you enter
>       CT's mount ns only, you still won't be able to check entries of
>       CT's kthreads. Check host's procfs for that.)
>     * introduce helpers
> ---
>  fs/proc/base.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index dbf5a84b604f3..965a7467c2b4d 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -3300,6 +3300,15 @@ static struct dentry *proc_pid_instantiate(struct inode *dir,
>  	return error;
>  }
>  
> +static bool is_visible_task_ve(struct pid_namespace *ns, struct task_struct *task)
> +{
> +	/* Don't show kthreads inside Containers. */
> +	if ((task->flags & PF_KTHREAD) && (ns != &init_pid_ns))
> +		return false;
> +
> +	return true;
> +}
> +
>  struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsigned int flags)
>  {
>  	struct dentry *result = NULL;
> @@ -3314,6 +3323,8 @@ struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsign
>  	ns = dentry->d_sb->s_fs_info;
>  	rcu_read_lock();
>  	task = find_task_by_pid_ns(tgid, ns);
> +	if (task && !is_visible_task_ve(ns, task))
> +		task = NULL;
>  	if (task)
>  		get_task_struct(task);
>  	rcu_read_unlock();
> @@ -3410,6 +3421,9 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
>  	for (iter = next_tgid(ns, iter);
>  	     iter.task;
>  	     iter.tgid += 1, iter = next_tgid(ns, iter)) {
> +		if (!is_visible_task_ve(ns, iter.task))
> +			continue;
> +
>  		if (is_visible_task(ns, iter.task))
>  			__filldir = filldir;
>  		else
>