* unkillable process due to setup_frame() failure
@ 2005-09-06 16:42 Atsushi Nemoto
2005-09-06 18:41 ` Ralf Baechle
0 siblings, 1 reply; 11+ messages in thread
From: Atsushi Nemoto @ 2005-09-06 16:42 UTC (permalink / raw)
To: linux-mips; +Cc: ralf
For a long time, this test program is unkillable (by SIGKILL) on
Linux/MIPS.
#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
void sighandler(int sig)
{
printf("SIGNAL %d!\n", sig);
exit(2);
}
void setup_signal(int sig)
{
struct sigaction act;
memset(&act, 0, sizeof(act));
act.sa_handler = sighandler;
act.sa_flags = SA_NOMASK | SA_RESTART;
sigaction(sig, &act, 0);
}
int main(int argc, char **argv)
{
setup_signal(SIGTRAP);
__asm__ __volatile__("move $29,$0");
__asm__ __volatile__("break");
printf("done!\n");
return 0;
}
If we run this program,
1. The "break" instruction raises a exception.
2. The exception handler queues SIGTRAP(5).
3. dequeue_signal() dequeue a signal with LOWEST number (i.e. SIGTRAP).
4. setup_frame() fails due to bad stack pointer and queues SIGSEGV(11).
5. returns to user process (pc unchanged).
6. goto 1. (forever)
So, the process can not be kill by SIGKILL. In 2.6.12, 'sigkill
priority fix' was applied to __dequeue_signal(), but it does not help
while the SIGTRAP is queued to tsk->pending but SIGKILL (by kill
command) is queued to tsk->signal->shared_pending.
I have two proposal fix:
1. Now do_signal() returns 0 if setup_frame() failed. Therefore if
do_signal returned 0 (and there is still pending signal), calling
do_signal again will handle the SIGSIGV. Like this:
--- linux-mips/arch/mips/kernel/signal.c 2005-08-30 11:02:00.000000000 +0900
+++ linux/arch/mips/kernel/signal.c 2005-09-06 15:07:45.000000000 +0900
@@ -481,6 +481,10 @@
{
/* deal with pending signal delivery */
if (thread_info_flags & _TIF_SIGPENDING) {
- current->thread.abi->do_signal(oldset, regs);
+ if (unlikely(!current->thread.abi->do_signal(oldset, regs)) &&
+ unlikely(signal_pending(current))) {
+ /* SIGSEGV mighe be sent in setup_frame */
+ current->thread.abi->do_signal(oldset, regs);
+ }
}
}
2. Make 'sigkill priority fix' can handle this case. First search
SIGKILL in tsk->pending and tsk->signal->shared_pending, then
search another signals. Like this:
--- linux-mips/kernel/signal.c 2005-08-29 08:41:01.000000000 +0900
+++ linux/kernel/signal.c 2005-09-07 01:33:52.338420760 +0900
@@ -520,19 +520,14 @@
}
static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
- siginfo_t *info)
+ siginfo_t *info, int sig)
{
- int sig = 0;
-
- /* SIGKILL must have priority, otherwise it is quite easy
- * to create an unkillable process, sending sig < SIGKILL
- * to self */
- if (unlikely(sigismember(&pending->signal, SIGKILL))) {
- if (!sigismember(mask, SIGKILL))
- sig = SIGKILL;
- }
-
- if (likely(!sig))
+ if (sig) {
+ /* check signal with priority first */
+ if (likely(!sigismember(&pending->signal, sig)) ||
+ sigismember(mask, sig))
+ sig = 0;
+ } else
sig = next_signal(pending, mask);
if (sig) {
if (current->notifier) {
@@ -561,10 +556,18 @@
*/
int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
{
- int signr = __dequeue_signal(&tsk->pending, mask, info);
+ /* SIGKILL must have priority, otherwise it is quite easy
+ * to create an unkillable process, sending sig < SIGKILL
+ * to self */
+ int signr = __dequeue_signal(&tsk->pending, mask, info, SIGKILL);
+ if (likely(!signr))
+ signr = __dequeue_signal(&tsk->signal->shared_pending,
+ mask, info, SIGKILL);
+ if (likely(!signr))
+ signr = __dequeue_signal(&tsk->pending, mask, info, 0);
if (!signr)
signr = __dequeue_signal(&tsk->signal->shared_pending,
- mask, info);
+ mask, info, 0);
if (signr && unlikely(sig_kernel_stop(signr))) {
/*
* Set a marker that we have dequeued a stop signal. Our
Which are preferred? Or is there other solutions?
---
Atsushi Nemoto
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: unkillable process due to setup_frame() failure
2005-09-06 16:42 unkillable process due to setup_frame() failure Atsushi Nemoto
@ 2005-09-06 18:41 ` Ralf Baechle
2005-09-07 9:14 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: Ralf Baechle @ 2005-09-06 18:41 UTC (permalink / raw)
To: Atsushi Nemoto; +Cc: linux-mips
On Wed, Sep 07, 2005 at 01:42:34AM +0900, Atsushi Nemoto wrote:
> 1. The "break" instruction raises a exception.
> 2. The exception handler queues SIGTRAP(5).
> 3. dequeue_signal() dequeue a signal with LOWEST number (i.e. SIGTRAP).
> 4. setup_frame() fails due to bad stack pointer and queues SIGSEGV(11).
> 5. returns to user process (pc unchanged).
> 6. goto 1. (forever)
>
> So, the process can not be kill by SIGKILL. In 2.6.12, 'sigkill
> priority fix' was applied to __dequeue_signal(), but it does not help
> while the SIGTRAP is queued to tsk->pending but SIGKILL (by kill
> command) is queued to tsk->signal->shared_pending.
The behaviour of not advancing the EPC beyond the faulting instruction is
part of the problem - but I believe that was the usual behaviour for
MIPS UNIXoid operating systems.
Ralf
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: unkillable process due to setup_frame() failure
2005-09-06 18:41 ` Ralf Baechle
@ 2005-09-07 9:14 ` Maciej W. Rozycki
2005-09-07 13:47 ` Ralf Baechle
0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2005-09-07 9:14 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Atsushi Nemoto, linux-mips
On Tue, 6 Sep 2005, Ralf Baechle wrote:
> > So, the process can not be kill by SIGKILL. In 2.6.12, 'sigkill
> > priority fix' was applied to __dequeue_signal(), but it does not help
> > while the SIGTRAP is queued to tsk->pending but SIGKILL (by kill
> > command) is queued to tsk->signal->shared_pending.
>
> The behaviour of not advancing the EPC beyond the faulting instruction is
> part of the problem - but I believe that was the usual behaviour for
> MIPS UNIXoid operating systems.
Well, SIGKILL should always work and frankly I can't see a reason to
return back to user space in the affected context in the first place.
What's left in EPC shouldn't matter.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: unkillable process due to setup_frame() failure
2005-09-07 9:14 ` Maciej W. Rozycki
@ 2005-09-07 13:47 ` Ralf Baechle
2005-09-07 14:44 ` Atsushi Nemoto
0 siblings, 1 reply; 11+ messages in thread
From: Ralf Baechle @ 2005-09-07 13:47 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Atsushi Nemoto, linux-mips
On Wed, Sep 07, 2005 at 10:14:16AM +0100, Maciej W. Rozycki wrote:
> > > So, the process can not be kill by SIGKILL. In 2.6.12, 'sigkill
> > > priority fix' was applied to __dequeue_signal(), but it does not help
> > > while the SIGTRAP is queued to tsk->pending but SIGKILL (by kill
> > > command) is queued to tsk->signal->shared_pending.
> >
> > The behaviour of not advancing the EPC beyond the faulting instruction is
> > part of the problem - but I believe that was the usual behaviour for
> > MIPS UNIXoid operating systems.
>
> Well, SIGKILL should always work and frankly I can't see a reason to
> return back to user space in the affected context in the first place.
> What's left in EPC shouldn't matter.
I said it's part of the problem - not that it should be changed. The
behaviour as far as I can say is also standard for MIPS unixoid operating
systems. That said, I definately prefer the approach of Atushi's suggested
fix #2. The other question is why we try to continue if delivering a
signal failed and we already know that repeated attempts would fail again.
Ralf
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: unkillable process due to setup_frame() failure
2005-09-07 13:47 ` Ralf Baechle
@ 2005-09-07 14:44 ` Atsushi Nemoto
2005-09-07 14:56 ` Atsushi Nemoto
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Atsushi Nemoto @ 2005-09-07 14:44 UTC (permalink / raw)
To: ralf; +Cc: macro, linux-mips
>>>>> On Wed, 7 Sep 2005 14:47:17 +0100, Ralf Baechle <ralf@linux-mips.org> said:
ralf> That said, I definately prefer the approach of Atushi's
ralf> suggested fix #2. The other question is why we try to continue
ralf> if delivering a signal failed and we already know that repeated
ralf> attempts would fail again.
The question is for my fix #1 ?
Well, let me explain more. With that fix, things will go like this:
1. The "break" instruction raises a exception.
2. The exception handler queues SIGTRAP(5).
3. dequeue_signal() dequeue a signal with LOWEST number (i.e. SIGTRAP).
4. setup_frame() fails due to bad stack pointer and queues SIGSEGV(11).
5. do_signal() called again (by fix #1)
6. dequeue_signal() dequeue a signal (i.e. SIGSEGV).
7. If SIGSEGV did not have a signal handler, default action (coredump
and exit) is taken. End.
8. If SIGSEGV had a signal handler, setup_frame() fails and queues
SIGSEGV again. It resets SIGSEGV's handler to SIG_DFL.
9. returns to user process (pc unchanged).
10. goto 1. Then ended at 7.
My fix #2 is more generic approach, but even with the fix, the test
program eats CPU until killed by SIGKILL. With my fix #1, the test
program will exit immediately.
So my "which is preferred" question was inappropriate. I had to ask
"#1 or #2 or both or other ?"
---
Atsushi Nemoto
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: unkillable process due to setup_frame() failure
2005-09-07 14:44 ` Atsushi Nemoto
@ 2005-09-07 14:56 ` Atsushi Nemoto
2005-09-07 15:24 ` Maciej W. Rozycki
2005-09-15 16:17 ` Atsushi Nemoto
2 siblings, 0 replies; 11+ messages in thread
From: Atsushi Nemoto @ 2005-09-07 14:56 UTC (permalink / raw)
To: ralf; +Cc: macro, linux-mips
>>>>> On Wed, 07 Sep 2005 23:44:13 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> said:
anemo> My fix #2 is more generic approach, but even with the fix, the
anemo> test program eats CPU until killed by SIGKILL. With my fix #1,
anemo> the test program will exit immediately.
Oh, I forgot to say: my fix #1 will not work on 64bit kernel due to
another problem. I posted a patch for it a few days ago.
http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=20050901.011551.96687558.anemo%40mba.ocn.ne.jp
Please apply this first. Thank you.
---
Atsushi Nemoto
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: unkillable process due to setup_frame() failure
2005-09-07 14:44 ` Atsushi Nemoto
2005-09-07 14:56 ` Atsushi Nemoto
@ 2005-09-07 15:24 ` Maciej W. Rozycki
2005-09-07 16:11 ` Ralf Baechle
2005-09-15 16:17 ` Atsushi Nemoto
2 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2005-09-07 15:24 UTC (permalink / raw)
To: Atsushi Nemoto; +Cc: ralf, linux-mips
On Wed, 7 Sep 2005, Atsushi Nemoto wrote:
> So my "which is preferred" question was inappropriate. I had to ask
> "#1 or #2 or both or other ?"
We should be consistent with other platforms -- having a look at e.g. the
i386 (as it used to be the reference) and the alpha (as close-enough to
MIPS) should reveal the answer. IIRC, a SIGSEGV that has a handler
installed, but which cannot be callled due to a bad stack pointer is
forced to SIG_DFL, but you may want to double-check it.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: unkillable process due to setup_frame() failure
2005-09-07 15:24 ` Maciej W. Rozycki
@ 2005-09-07 16:11 ` Ralf Baechle
2005-09-07 16:34 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: Ralf Baechle @ 2005-09-07 16:11 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Atsushi Nemoto, linux-mips
On Wed, Sep 07, 2005 at 04:24:05PM +0100, Maciej W. Rozycki wrote:
> > So my "which is preferred" question was inappropriate. I had to ask
> > "#1 or #2 or both or other ?"
>
> We should be consistent with other platforms -- having a look at e.g. the
> i386 (as it used to be the reference) and the alpha (as close-enough to
> MIPS) should reveal the answer. IIRC, a SIGSEGV that has a handler
> installed, but which cannot be callled due to a bad stack pointer is
> forced to SIG_DFL, but you may want to double-check it.
That's what's already happening. We call force_sigsegv which is like
force_sig unless it's trying to deliver a SIGSEGV in which case it'll
reset the handler to SIG_DFL, return to userspace where it hits the
break instruction and starts all over to process the SIGTRAP.
Ralf
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: unkillable process due to setup_frame() failure
2005-09-07 16:11 ` Ralf Baechle
@ 2005-09-07 16:34 ` Maciej W. Rozycki
0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2005-09-07 16:34 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Atsushi Nemoto, linux-mips
On Wed, 7 Sep 2005, Ralf Baechle wrote:
> That's what's already happening. We call force_sigsegv which is like
> force_sig unless it's trying to deliver a SIGSEGV in which case it'll
> reset the handler to SIG_DFL, return to userspace where it hits the
> break instruction and starts all over to process the SIGTRAP.
Except that SIG_DFL for SIGSEGV is killing the process (with a core
dump). Therefore user space shouldn't ever be reached again in this
context.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: unkillable process due to setup_frame() failure
2005-09-07 14:44 ` Atsushi Nemoto
2005-09-07 14:56 ` Atsushi Nemoto
2005-09-07 15:24 ` Maciej W. Rozycki
@ 2005-09-15 16:17 ` Atsushi Nemoto
2005-09-15 16:44 ` Ralf Baechle
2 siblings, 1 reply; 11+ messages in thread
From: Atsushi Nemoto @ 2005-09-15 16:17 UTC (permalink / raw)
To: ralf; +Cc: macro, linux-mips
>>>>> On Wed, 07 Sep 2005 23:44:13 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> said:
anemo> So my "which is preferred" question was inappropriate. I had
anemo> to ask "#1 or #2 or both or other ?"
In 2.6.14-rc1, another fix is done in arch/i386/kernel/entry.S.
It also fixes a race condition in signal delivery.
> [PATCH] i386: Don't miss pending signals returning to user mode after signal processing
>
> Signed-off-by: Roland McGrath <roland@redhat.com>
Let's follow.
--- linux-mips/arch/mips/kernel/entry.S 2005-03-04 22:17:29.000000000 +0900
+++ linux/arch/mips/kernel/entry.S 2005-09-16 01:04:52.365022536 +0900
@@ -105,7 +105,7 @@
move a0, sp
li a1, 0
jal do_notify_resume # a2 already loaded
- j restore_all
+ j resume_userspace
FEXPORT(syscall_exit_work_partial)
SAVE_STATIC
---
Atsushi Nemoto
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2005-09-15 16:44 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-06 16:42 unkillable process due to setup_frame() failure Atsushi Nemoto
2005-09-06 18:41 ` Ralf Baechle
2005-09-07 9:14 ` Maciej W. Rozycki
2005-09-07 13:47 ` Ralf Baechle
2005-09-07 14:44 ` Atsushi Nemoto
2005-09-07 14:56 ` Atsushi Nemoto
2005-09-07 15:24 ` Maciej W. Rozycki
2005-09-07 16:11 ` Ralf Baechle
2005-09-07 16:34 ` Maciej W. Rozycki
2005-09-15 16:17 ` Atsushi Nemoto
2005-09-15 16:44 ` Ralf Baechle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox