Messages in this thread | | | From | Roland McGrath <> | Subject | Re: [PATCH] fix de_thread() vs do_coredump() deadlock | Date | Tue, 11 Apr 2006 01:01:19 -0700 (PDT) |
| |
> 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.
I am not talking about the case where it's still pending on either queue. Those are fine as they are. What matters is when it's been dequeued, in the race window afer releasing the siglock in get_signal_to_deliver. There are two windows of race here.
The first one is only when ptrace'd, and doesn't even require a race that takes good timing to create. The window is in ptrace_stop when the siglock is released, including all the time stopped in TASK_TRACED. Say another thread does an exec (and de_thread) while we're in TASK_TRACED after reporting a death signal to the debugger. The SIGKILL wakes us out of ptrace_stop. Assuming the debugger wasn't racing with a PTRACE_CONT too, then the signal remains in ->exit_code and (assuming the SIGNAL_GROUP_EXIT check there reverted, as I mentioned before), we just come out of the ptrace path with the siglock held as if we'd dequeued the signal without ptrace.
Then comes the second window. With no ptrace, or after ptrace, we've dequeued the signal and if it's a SIG_DFL fatal signal, we release the siglock. Here a non-coredump signal just calls do_group_exit. Meanwhile, a racing exec comes along and sets SIGNAL_GROUP_EXIT (or it already did earlier while we were in ptrace_stop). In do_group_exit, we will see that SIGNAL_GROUP_EXIT is set, and just do_exit ourselves with the group_exit_code. When it's an exec rather than a real exit, we've swallowed the signal. This is no different than the coredump case. (When do_coredump bails out, then it joins this very same code path.)
> What do you think about something like this untested patch instead? > I am far from sure it is correct, I need a sleep ...
For one thing, it only handles the coredump case. Moreover, I think there are too many problems with any plan to stuff a signal back on the queue. 1. This might be moving a signal from some thread's per-thread queue to the shared_pending queue. If the signal was going to be fatal to the group, that is fine before the exec but not after it--the new program shouldn't get a fatal signal generated for some thread it never had. 2. siginfo_t is lost. These are just what I could think of off hand. Pushing a signal back on the queue is a whole can of worms that I really want to avoid.
Thanks, Roland - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |