Linux Container Development
 help / color / mirror / Atom feed
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
To: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
Cc: Louis.Rilling-aw0BnHfMbSpBDgjK7y7TUQ@public.gmane.org,
	Containers
	<containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>
Subject: [PATCH][cr]: Fix ghost task bug
Date: Thu, 24 Feb 2011 23:55:52 -0800	[thread overview]
Message-ID: <20110225075552.GB24361@us.ibm.com> (raw)


From b32a8c440687955975180e909620eba50cb146c3 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 22 Feb 2011 10:44:55 -0800
Subject: [PATCH 1/1] Fix ghost task bug

Ghost tasks used to be marked as "detached" in do_ghost_task() but
this resulted in a crash which was fixed as discussed in:
https://lists.linux-foundation.org/pipermail/containers/2010-December/026076.html.

But that fix incorrectly attempted to mark a task as detached from user
space. It is not possible to "detach" a task from user space, which
means we must detach the thread in the kernel while still avoiding
the above crash.

The original crash occured because the container-init did not wait for
the detached children. When the detached children exited, they would
access a freed proc_mnt. Eric Biederman fixed this crash as discussed in
the above link.

While Eric's patch fixed the crash it could still leave the container
init hanging indefinitely due to the following race:

container-int:				ghost task
-----------------                      ------------
					do_ghost_task()

zap_pid_ns_processes()
- send SIGKILL
- do_wait()
   - at least one child exists
   - so wait for child to exit
   					wake up for the SIGKILL
					set ->exit_signal = -1
					exit without notifying parent

This leaves the container init waiting indefinitely.

To fix this hang, we have the children of container-init issue an extra wake
up call in exit_checkpoint(). Note that in exit_checkpoint() we do not know
if it is the ghost task that is exiting. Since this wake up applies to any
other task, we should further make sure that the parent is itself not exiting
(which could cause __wake_up_parent() to access invalid pointers in the
parent's task structure).

See this thread for more discussion:

https://lists.linux-foundation.org/pipermail/containers/2011-February/026459.html

Signed-off-by: Sukadev Bhattiprolu (sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org)
Cc: Louis Rilling <Louis.Rilling-aw0BnHfMbSpBDgjK7y7TUQ@public.gmane.org>
---
 kernel/checkpoint/restart.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/kernel/checkpoint/restart.c b/kernel/checkpoint/restart.c
index b0ea8ec..8ecc052 100644
--- a/kernel/checkpoint/restart.c
+++ b/kernel/checkpoint/restart.c
@@ -972,6 +972,7 @@ static int do_ghost_task(void)
 	if (ret < 0)
 		ckpt_err(ctx, ret, "ghost restart failed\n");
 
+	current->exit_signal = -1;
 	restore_debug_exit(ctx);
 	ckpt_ctx_put(ctx);
 	do_exit(0);
@@ -1465,7 +1466,22 @@ void exit_checkpoint(struct task_struct *tsk)
 	/* restarting zombies will activate next task in restart */
 	if (tsk->flags & PF_RESTARTING) {
 		BUG_ON(ctx->active_pid == -1);
+
+		/*
+		 * if we are a "ghost" task, that was terminated by the
+		 * container-init (from zap_pid_ns_processes()), we should
+		 * wake up the parent since we are now a detached process.
+		 */
+		read_lock_irq(&tasklist_lock);
+                if (tsk->exit_state == EXIT_DEAD && !tsk->parent->exit_state) {
+                        ckpt_debug("[%d, %s]: exit_checkpoint(): notifying "
+					"parent\n", tsk->pid, tsk->comm);
+                        __wake_up_parent(tsk, tsk->parent);
+                }
+		read_unlock_irq(&tasklist_lock);
+
 		restore_task_done(ctx);
+
 	}
 
 	ckpt_ctx_put(ctx);
-- 
1.6.6.1

             reply	other threads:[~2011-02-25  7:55 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-25  7:55 Sukadev Bhattiprolu [this message]
     [not found] ` <20110225075552.GB24361-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2011-02-25 11:55   ` [PATCH][cr]: Fix ghost task bug Louis Rilling
     [not found]     ` <20110225115507.GD3915-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2011-02-25 19:01       ` Sukadev Bhattiprolu
     [not found]         ` <20110225190132.GA31300-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2011-02-26 13:54           ` Louis Rilling
     [not found]             ` <20110226135431.GA3251-aw0BnHfMbSpBDgjK7y7TUQ@public.gmane.org>
2011-02-28 14:43               ` Oren Laadan
     [not found]                 ` <4D6BB499.7050602-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2011-02-28 15:09                   ` Louis Rilling
     [not found]                     ` <20110228150942.GD18970-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2011-02-28 22:10                       ` Oren Laadan
     [not found]                         ` <4D6C1D58.5060400-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2011-03-01 15:31                           ` Louis Rilling
     [not found]                             ` <20110301153105.GD4410-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2011-03-03 15:38                               ` Oren Laadan
     [not found]                                 ` <4D6FB5D9.8010002-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2011-03-03 16:35                                   ` Louis Rilling
     [not found]                                     ` <20110303163531.GO21429-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2011-03-04 16:07                                       ` Oren Laadan
     [not found]                                         ` <4D710E37.8000109-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2011-03-04 17:29                                           ` Louis Rilling
     [not found]                                             ` <20110304172914.GE3543-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2011-03-26  2:14                                               ` Oren Laadan
     [not found]                                                 ` <4D8D4BE9.8010605-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2011-03-26 16:06                                                   ` Louis Rilling
     [not found]                                                     ` <20110326160607.GA18492-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2011-03-27  3:17                                                       ` Oren Laadan
     [not found]                                                         ` <4D8EAC5A.2080604-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2011-03-28 15:24                                                           ` Louis Rilling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110225075552.GB24361@us.ibm.com \
    --to=sukadev-23vcf4htsmix0ybbhkvfkdbpr1lh4cv8@public.gmane.org \
    --cc=Louis.Rilling-aw0BnHfMbSpBDgjK7y7TUQ@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox