Zombie stuck in zap_pid_ns_processes()

Zombie stuck in zap_pid_ns_processes()

[Caj Larsson]

Hello,

We are setting up a container using a CLONE_NEWNS linux namespace.
Previously we used the 3.4.4 kernel, which worked fine. After I
upgraded i also experienced races in netlink, which has been resolved
by placing a monitor around the namespace setup. When we upgraded to
Linux 3.8.0 however our init processes does not get reaped when the
namespace is killed and lingers as zombie process under the global
init.

The init has multiple threads when running and two remain in the
zombie. One of them is hung in zap_pid_ns_processes() and has been set
uninterruptible. The other one, which has Tgid=PID, is still in
do_exit().

-- 
Caj Larsson, Omnicloud

Re: Zombie stuck in zap_pid_ns_processes()

[Eric W. Biederman]

Caj Larsson <caj@omnicloud.com> writes:

> Hello,
>
> We are setting up a container using a CLONE_NEWNS linux namespace.
> Previously we used the 3.4.4 kernel, which worked fine. After I
> upgraded i also experienced races in netlink, which has been resolved
> by placing a monitor around the namespace setup. When we upgraded to
> Linux 3.8.0 however our init processes does not get reaped when the
> namespace is killed and lingers as zombie process under the global
> init.
>
> The init has multiple threads when running and two remain in the
> zombie. One of them is hung in zap_pid_ns_processes() and has been set
> uninterruptible. The other one, which has Tgid=PID, is still in
> do_exit().

Doh.

Thank you for the detailed bug report it appears I goofed, and failed
to account for a multi-threaded init.

Will you please verify that the following patch fixes it for you.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index c1c3dc1..72b7722 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -181,6 +181,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	int nr;
 	int rc;
 	struct task_struct *task, *me = current;
+	int init_pids = task_pid_vnr(me) == 1 ? 1 : 2;
 
 	/* Don't allow any more processes into the pid namespace */
 	disable_pid_allocation(pid_ns);
@@ -230,7 +231,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	 */
 	for (;;) {
 		set_current_state(TASK_UNINTERRUPTIBLE);
-		if (pid_ns->nr_hashed == 1)
+		if (pid_ns->nr_hashed == init_pids)
 			break;
 		schedule();
 	}

Re: Zombie stuck in zap_pid_ns_processes()

[Oleg Nesterov]

On 03/24, Eric W. Biederman wrote:
>
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -181,6 +181,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
>  	int nr;
>  	int rc;
>  	struct task_struct *task, *me = current;
> +	int init_pids = task_pid_vnr(me) == 1 ? 1 : 2;
                        ^^^^^^^^^^^^^^^^^^^^^

Ah, mt-init.

perhaps thread_group_leader(me) would be more clean/simple,
but I won't insist of course.

Oleg.

Re: Zombie stuck in zap_pid_ns_processes()

[Eric W. Biederman]

Oleg Nesterov <oleg@redhat.com> writes:

> On 03/24, Eric W. Biederman wrote:
>>
>> --- a/kernel/pid_namespace.c
>> +++ b/kernel/pid_namespace.c
>> @@ -181,6 +181,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
>>  	int nr;
>>  	int rc;
>>  	struct task_struct *task, *me = current;
>> +	int init_pids = task_pid_vnr(me) == 1 ? 1 : 2;
>                         ^^^^^^^^^^^^^^^^^^^^^
>
> Ah, mt-init.
>
> perhaps thread_group_leader(me) would be more clean/simple,
> but I won't insist of course.

It is.  Thanks, I was going to follow the reaper but that changes when the
initial thread exits...

Here is my final version of the patch, that I will push shortly.

Eric

From: "Eric W. Biederman" <ebiederm@xmission.com>
Date: Tue, 26 Mar 2013 02:27:11 -0700
Subject: [PATCH] pid: Handle the exit of a multi-threaded init.

When a multi-threaded init exits and the initial thread is not the
last thread to exit the initial thread hangs around as a zombie
until the last thread exits.  In that case zap_pid_ns_processes
needs to wait until there are only 2 hashed pids in the pid
namespace not one.

v2. Replace thread_pid_vnr(me) == 1 with the test thread_group_leader(me)
    as suggested by Oleg.

Cc: stable@vger.kernel.org
Cc: Oleg Nesterov <oleg@redhat.com>
Reported-by: Caj Larsson <caj@omnicloud.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/pid_namespace.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index c1c3dc1..bea15bd 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -181,6 +181,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;
 
 	/* Don't allow any more processes into the pid namespace */
 	disable_pid_allocation(pid_ns);
@@ -230,7 +231,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	 */
 	for (;;) {
 		set_current_state(TASK_UNINTERRUPTIBLE);
-		if (pid_ns->nr_hashed == 1)
+		if (pid_ns->nr_hashed == init_pids)
 			break;
 		schedule();
 	}
-- 
1.7.5.4