All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@tv-sign.ru>
To: Roland McGrath <roland@redhat.com>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	Michael Kerrisk <mtk-lkml@gmx.net>,
	Linus Torvalds <torvalds@osdl.org>, Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH] fix de_thread() vs do_coredump() deadlock
Date: Tue, 11 Apr 2006 01:40:18 +0400	[thread overview]
Message-ID: <20060410214018.GA635@oleg> (raw)
In-Reply-To: <20060410013651.4D1791809D1@magilla.sf.frob.com>

On 04/09, Roland McGrath wrote:
>
>                                                                  Now, the
> coredump case matches the non-coredump fatal signal: the signal is dropped
> on the floor.

A fatal signal is placed to ->shared_pending in any (non tkill) case, so I
think it is not lost (but may be unnoticed for a while).

sig_kernel_coredump() is different. It could be stealed by one of sub-threads
while another one does de_thread(), that is why it could be lost.

What do you think about something like this untested patch instead?
I am far from sure it is correct, I need a sleep ...

Oleg.

--- fs/exec.c~	2006-04-10 22:15:06.000000000 +0400
+++ fs/exec.c	2006-04-11 00:56:52.000000000 +0400
@@ -657,6 +657,7 @@ static int de_thread(struct task_struct 
 	}
 	sig->group_exit_task = NULL;
 	sig->notify_count = 0;
+	recalc_sigpending();
 	spin_unlock_irq(lock);
 
 	/*
@@ -1478,9 +1479,15 @@ int do_coredump(long signr, int exit_cod
 	}
 	mm->dumpable = 0;
 
-	retval = -EAGAIN;
 	spin_lock_irq(&current->sighand->siglock);
-	if (!(current->signal->flags & SIGNAL_GROUP_EXIT)) {
+	if (current->signal->flags & SIGNAL_GROUP_EXIT) {
+		// Re-add the signal. This does not matter
+		// if we are doing do_group_exit().
+		// If it was de_thread(), this signal will be
+		// received again after sys_exec() succeeds.
+		sigaddset(&current->signal->shared_pending.signal, signr);
+		retval = -EAGAIN;
+	} else {
 		current->signal->flags = SIGNAL_GROUP_EXIT;
 		current->signal->group_exit_code = exit_code;
 		current->signal->group_stop_count = 0;


  parent reply	other threads:[~2006-04-10 17:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-10-13 16:50 [PATCH] fix de_thread() vs do_coredump() deadlock Oleg Nesterov
2006-04-10  1:36 ` Roland McGrath
2006-04-10 17:43   ` Oleg Nesterov
2006-04-11  7:27     ` Roland McGrath
2006-04-11 11:47       ` Oleg Nesterov
2006-04-11  9:10         ` Roland McGrath
2006-04-11 12:03       ` Oleg Nesterov
2006-04-10 21:40   ` Oleg Nesterov [this message]
2006-04-11  8:01     ` Roland McGrath
2006-04-11 13:13       ` Oleg Nesterov
2006-04-11  9:49         ` Roland McGrath

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=20060410214018.GA635@oleg \
    --to=oleg@tv-sign.ru \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mtk-lkml@gmx.net \
    --cc=roland@redhat.com \
    --cc=torvalds@osdl.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.