linux-kernel - [PATCH 26/26] pidns: Ensure zap_pid_ns

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue,  6 Jun 2017 14:03:38 -0500
From:   "Eric W. Biederman" <ebiederm@...ssion.com>
To:     linux-kernel@...r.kernel.org
Cc:     linux-api@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Kees Cook <keescook@...omium.org>,
        Roland McGrath <roland@...k.frob.com>,
        Al Viro <viro@...IV.linux.org.uk>,
        David Howells <dhowells@...hat.com>,
        "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>
Subject: [PATCH 26/26] pidns: Ensure zap_pid_ns_processes always terminates

The function zap_pid_processes terminates when the number of pids used
by processes in a pid namespace drops to just those pids used by the
last thread of the dying thread.

Don't allow an init process aka a child_reaper to call
setpgid(0, some_other_processes_pid).  That case is already broken today
as it would result in a pid namespace that will hang when the thread group
leader dies.  Thankfully I have not received that bug report so it
appears that no one cares and uses that case.

Limiting setpgid ensures that the only two pids in the pid namespace
on the init process that are worth worrying about are the pid and the
tgid.  The pgrp will now either match the tgid or it will be from
outside the pid namespace.  Likewise the sid will either match the
tgid or be from outside the pid namespace.

To make it clear what is being counted test if the task's tgid is the same
as the the task's pid.  In particular the code does not count the
number of processes in a pid namespace, just the number of pids those
processes use.  A subtle but important distinction for understanding
the code.

Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
---
 kernel/pid_namespace.c | 2 +-
 kernel/sys.c           | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 74a5a7255b4d..bdda73768cc0 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -208,7 +208,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	int nr;
 	int rc;
 	struct task_struct *task, *me = current;
-	int init_pids = thread_group_leader(me) ? 1 : 2;
+	int init_pids = task_pid(me) != task_tgid(me) ? 2 : 1;

 	/* Don't allow any more processes into the pid namespace */
 	disable_pid_allocation(pid_ns);
diff --git a/kernel/sys.c b/kernel/sys.c
index 705f14b28134..775dea1e2e06 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -975,7 +975,8 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)

 		pgrp = find_vpid(pgid);
 		g = pid_task(pgrp, PIDTYPE_PGID);
-		if (!g || task_session(g) != task_session(group_leader))
+		if (!g || task_session(g) != task_session(group_leader) ||
+		    is_child_reaper(task_tgid(p)))
 			goto out;
 	}

-- 
2.10.1