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(¤t->sighand->siglock);
+ current->signal->group_exit_task = NULL;
+ current->signal->flags = SIGNAL_GROUP_EXIT;
+ spin_unlock_irq(¤t->sighand->siglock);
+
next = mm->core_state->dumper.next;
while ((curr = next) != NULL) {
next = curr->next;
--
1.5.5.1
next prev 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.