All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Ingo Molnar <mingo@redhat.com>,
	Mandeep Singh Baines <msb@chromium.org>,
	Neil Horman <nhorman@redhat.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Roland McGrath <roland@hack.frob.com>, Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/3] coredump: ensure that SIGKILL always kills the dumping thread
Date: Sun, 17 Feb 2013 20:19:02 +0100	[thread overview]
Message-ID: <20130217191902.GA21817@redhat.com> (raw)
In-Reply-To: <20130217191819.GA21778@redhat.com>

prepare_signal() blesses SIGKILL sent to the dumping process but
this signal can be "lost" anyway. The problems is, complete_signal()
sees SIGNAL_GROUP_EXIT and skips the "kill them all" logic. And even
if the dumping process is single-threaded (so the target is always
"correct"), the group-wide SIGKILL is not recorded in task->pending
and thus __fatal_signal_pending() won't be true. A multi-threaded
case has even more problems.

And even ignoring all technical details, SIGNAL_GROUP_EXIT doesn't
look right to me. This coredumping process is not exiting yet, it can
do a lot of work dumping the core.

With this patch the dumping process doesn't have SIGNAL_GROUP_EXIT,
we set signal->group_exit_task instead. This makes signal_group_exit()
true and thus this should equally close the races with exit/exec/stop
but allows to kill the dumping thread reliably.

Notes:
	- It is not clear what should we do with ->group_exit_code
	  if the dumper was killed, see the next change.

	- we need more (hopefully straightforward) changes to ensure
	  that SIGKILL actually interrupts the coredump. Basically we
	  need to check __fatal_signal_pending() in dump_write() and
	  dump_seek().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/coredump.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 2c1ef6a..835d731 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -263,7 +263,6 @@ static int zap_process(struct task_struct *start, int exit_code)
 	struct task_struct *t;
 	int nr = 0;
 
-	start->signal->flags = SIGNAL_GROUP_EXIT;
 	start->signal->group_exit_code = exit_code;
 	start->signal->group_stop_count = 0;
 
@@ -291,8 +290,9 @@ static int zap_threads(struct task_struct *tsk, struct mm_struct *mm,
 	if (!signal_group_exit(tsk->signal)) {
 		mm->core_state = core_state;
 		nr = zap_process(tsk, exit_code);
+		tsk->signal->group_exit_task = tsk;
 		/* ignore all signals except SIGKILL, see prepare_signal() */
-		tsk->signal->flags |= SIGNAL_GROUP_COREDUMP;
+		tsk->signal->flags = SIGNAL_GROUP_COREDUMP;
 		clear_tsk_thread_flag(tsk, TIF_SIGPENDING);
 	}
 	spin_unlock_irq(&tsk->sighand->siglock);
@@ -343,6 +343,7 @@ static int zap_threads(struct task_struct *tsk, struct mm_struct *mm,
 				if (unlikely(p->mm == mm)) {
 					lock_task_sighand(p, &flags);
 					nr += zap_process(p, exit_code);
+					p->signal->flags = SIGNAL_GROUP_EXIT;
 					unlock_task_sighand(p, &flags);
 				}
 				break;
@@ -394,6 +395,11 @@ static void coredump_finish(struct mm_struct *mm)
 	struct core_thread *curr, *next;
 	struct task_struct *task;
 
+	spin_lock_irq(&current->sighand->siglock);
+	current->signal->group_exit_task = NULL;
+	current->signal->flags = SIGNAL_GROUP_EXIT;
+	spin_unlock_irq(&current->sighand->siglock);
+
 	next = mm->core_state->dumper.next;
 	while ((curr = next) != NULL) {
 		next = curr->next;
-- 
1.5.5.1


  parent reply	other threads:[~2013-02-17 19:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-17 19:18 [PATCH 0/3] coredump: fix the ancient signal problems Oleg Nesterov
2013-02-17 19:18 ` [PATCH 1/3] coredump: only SIGKILL should interrupt the coredumping task Oleg Nesterov
2013-02-17 19:19 ` Oleg Nesterov [this message]
2013-02-17 19:19 ` [PATCH 3/3] coredump: sanitize the setting of signal->group_exit_code Oleg Nesterov
2013-02-17 19:34 ` [PATCH 0/3] coredump: fix the ancient signal problems Linus Torvalds
2013-02-17 19:50   ` Oleg Nesterov
2013-02-17 20:01     ` Oleg Nesterov
2013-02-20  1:29 ` Mandeep Singh Baines
2013-02-20 22:32   ` Oleg Nesterov
2013-02-20 23:14     ` Andrew Morton
2013-02-23 20:21       ` Oleg Nesterov

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=20130217191902.GA21817@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=msb@chromium.org \
    --cc=nhorman@redhat.com \
    --cc=rjw@sisk.pl \
    --cc=roland@hack.frob.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.