All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] kernel/signal.c: avoid BUG_ON with SIG128 (MIPS)
@ 2013-05-29 17:01 ` James Hogan
  0 siblings, 0 replies; 6+ messages in thread
From: James Hogan @ 2013-05-29 17:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mips, James Hogan, Ralf Baechle, Al Viro, Andrew Morton,
	Oleg Nesterov, Kees Cook

MIPS has 128 signals, the highest of which has the number 128. The
following command causes get_signal_to_deliver() to pass this signal
number straight through to do_group_exit() as the exit code:

  strace sleep 10 & sleep 1 && kill -128 `pidof sleep`

However do_group_exit() checks for the core dump bit (0x80) in the exit
code which matches in this particular case and the kernel panics:

  BUG_ON(exit_code & 0x80); /* core dumps don't get here */

This is worked around by changing get_signal_to_deliver() to pass
min(info->si_signo, 127) instead of info->si_signo, so that this highest
of signal numbers get rounded down to 127. This makes the exit code
technically incorrect, but it's better than killing the whole kernel.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
---

This is based on v3.10-rc3.

It's a little hacky, but aside from reducing the number of signals to
127 to avoid this case (which isn't backwards compatible) I'm not sure
what else can be done. Any comments?

 kernel/signal.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 113411b..69bc00f 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2366,8 +2366,12 @@ relock:
 
 		/*
 		 * Death signals, no core dump.
+		 *
+		 * MIPS has a signal number 128 which clashes with the core dump
+		 * bit. If this was the signal we still want to report a valid
+		 * exit code, so round it down to 127.
 		 */
-		do_group_exit(info->si_signo);
+		do_group_exit(min(info->si_signo, 127));
 		/* NOTREACHED */
 	}
 	spin_unlock_irq(&sighand->siglock);
-- 
1.8.1.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH] kernel/signal.c: avoid BUG_ON with SIG128 (MIPS)
@ 2013-05-29 17:01 ` James Hogan
  0 siblings, 0 replies; 6+ messages in thread
From: James Hogan @ 2013-05-29 17:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mips, James Hogan, Ralf Baechle, Al Viro, Andrew Morton,
	Oleg Nesterov, Kees Cook

MIPS has 128 signals, the highest of which has the number 128. The
following command causes get_signal_to_deliver() to pass this signal
number straight through to do_group_exit() as the exit code:

  strace sleep 10 & sleep 1 && kill -128 `pidof sleep`

However do_group_exit() checks for the core dump bit (0x80) in the exit
code which matches in this particular case and the kernel panics:

  BUG_ON(exit_code & 0x80); /* core dumps don't get here */

This is worked around by changing get_signal_to_deliver() to pass
min(info->si_signo, 127) instead of info->si_signo, so that this highest
of signal numbers get rounded down to 127. This makes the exit code
technically incorrect, but it's better than killing the whole kernel.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
---

This is based on v3.10-rc3.

It's a little hacky, but aside from reducing the number of signals to
127 to avoid this case (which isn't backwards compatible) I'm not sure
what else can be done. Any comments?

 kernel/signal.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 113411b..69bc00f 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2366,8 +2366,12 @@ relock:
 
 		/*
 		 * Death signals, no core dump.
+		 *
+		 * MIPS has a signal number 128 which clashes with the core dump
+		 * bit. If this was the signal we still want to report a valid
+		 * exit code, so round it down to 127.
 		 */
-		do_group_exit(info->si_signo);
+		do_group_exit(min(info->si_signo, 127));
 		/* NOTREACHED */
 	}
 	spin_unlock_irq(&sighand->siglock);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] kernel/signal.c: avoid BUG_ON with SIG128 (MIPS)
  2013-05-29 17:01 ` James Hogan
  (?)
@ 2013-05-29 17:19 ` David Daney
  2013-05-29 17:36   ` Oleg Nesterov
  -1 siblings, 1 reply; 6+ messages in thread
From: David Daney @ 2013-05-29 17:19 UTC (permalink / raw)
  To: James Hogan
  Cc: linux-kernel, linux-mips, Ralf Baechle, Al Viro, Andrew Morton,
	Oleg Nesterov, Kees Cook

On 05/29/2013 10:01 AM, James Hogan wrote:
> MIPS has 128 signals, the highest of which has the number 128. The

I wonder if we should change the ABI and reduce the number of signals to 
127 instead of this patch.

David Daney



> following command causes get_signal_to_deliver() to pass this signal
> number straight through to do_group_exit() as the exit code:
>
>    strace sleep 10 & sleep 1 && kill -128 `pidof sleep`
>
> However do_group_exit() checks for the core dump bit (0x80) in the exit
> code which matches in this particular case and the kernel panics:
>
>    BUG_ON(exit_code & 0x80); /* core dumps don't get here */
>
> This is worked around by changing get_signal_to_deliver() to pass
> min(info->si_signo, 127) instead of info->si_signo, so that this highest
> of signal numbers get rounded down to 127. This makes the exit code
> technically incorrect, but it's better than killing the whole kernel.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Kees Cook <keescook@chromium.org>
> ---
>
> This is based on v3.10-rc3.
>
> It's a little hacky, but aside from reducing the number of signals to
> 127 to avoid this case (which isn't backwards compatible) I'm not sure
> what else can be done. Any comments?
>
>   kernel/signal.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 113411b..69bc00f 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2366,8 +2366,12 @@ relock:
>
>   		/*
>   		 * Death signals, no core dump.
> +		 *
> +		 * MIPS has a signal number 128 which clashes with the core dump
> +		 * bit. If this was the signal we still want to report a valid
> +		 * exit code, so round it down to 127.
>   		 */
> -		do_group_exit(info->si_signo);
> +		do_group_exit(min(info->si_signo, 127));
>   		/* NOTREACHED */
>   	}
>   	spin_unlock_irq(&sighand->siglock);
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] kernel/signal.c: avoid BUG_ON with SIG128 (MIPS)
  2013-05-29 17:19 ` David Daney
@ 2013-05-29 17:36   ` Oleg Nesterov
  2013-05-29 21:56     ` James Hogan
  0 siblings, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2013-05-29 17:36 UTC (permalink / raw)
  To: David Daney
  Cc: James Hogan, linux-kernel, linux-mips, Ralf Baechle, Al Viro,
	Andrew Morton, Kees Cook

On 05/29, David Daney wrote:
>
> On 05/29/2013 10:01 AM, James Hogan wrote:
>> MIPS has 128 signals, the highest of which has the number 128. The
>
> I wonder if we should change the ABI and reduce the number of signals to
> 127 instead of this patch.

Same thoughts...

>> @@ -2366,8 +2366,12 @@ relock:
>>
>>   		/*
>>   		 * Death signals, no core dump.
>> +		 *
>> +		 * MIPS has a signal number 128 which clashes with the core dump
>> +		 * bit. If this was the signal we still want to report a valid
>> +		 * exit code, so round it down to 127.
>>   		 */
>> -		do_group_exit(info->si_signo);
>> +		do_group_exit(min(info->si_signo, 127));

This avoids BUG_ON() but obviously fools WIFSIGNALED(), doesn't look
very nice.

Oleg.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] kernel/signal.c: avoid BUG_ON with SIG128 (MIPS)
  2013-05-29 17:36   ` Oleg Nesterov
@ 2013-05-29 21:56     ` James Hogan
  2013-06-28 20:03       ` Denys Vlasenko
  0 siblings, 1 reply; 6+ messages in thread
From: James Hogan @ 2013-05-29 21:56 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: David Daney, LKML, linux-mips, Ralf Baechle, Al Viro,
	Andrew Morton, Kees Cook, James Hogan

On 29 May 2013 18:36, Oleg Nesterov <oleg@redhat.com> wrote:
> On 05/29, David Daney wrote:
>>
>> On 05/29/2013 10:01 AM, James Hogan wrote:
>>> MIPS has 128 signals, the highest of which has the number 128. The
>>
>> I wonder if we should change the ABI and reduce the number of signals to
>> 127 instead of this patch.
>
> Same thoughts...

I'll give it a try. I wouldn't have thought it'd break anything, but
you never know. glibc (incorrectly) sets [__]SIGRTMAX to 127 already.
On the other hand uClibc sets it to 128, so anything built against
uClibc that uses signals SIGRTMAX-n (where n may be 0) or uses an
excessive number of rt signals starting from SIGRTMIN (sounds
unlikely) could well need an updated uClibc (or a full rebuild if it's
crazy enough to use __SIGRTMAX).

>>> @@ -2366,8 +2366,12 @@ relock:
>>>
>>>              /*
>>>               * Death signals, no core dump.
>>> +             *
>>> +             * MIPS has a signal number 128 which clashes with the core dump
>>> +             * bit. If this was the signal we still want to report a valid
>>> +             * exit code, so round it down to 127.
>>>               */
>>> -            do_group_exit(info->si_signo);
>>> +            do_group_exit(min(info->si_signo, 127));
>
> This avoids BUG_ON() but obviously fools WIFSIGNALED(), doesn't look
> very nice.

Agreed.

Cheers
James

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] kernel/signal.c: avoid BUG_ON with SIG128 (MIPS)
  2013-05-29 21:56     ` James Hogan
@ 2013-06-28 20:03       ` Denys Vlasenko
  0 siblings, 0 replies; 6+ messages in thread
From: Denys Vlasenko @ 2013-06-28 20:03 UTC (permalink / raw)
  To: James Hogan
  Cc: Oleg Nesterov, David Daney, LKML, linux-mips, Ralf Baechle,
	Al Viro, Andrew Morton, Kees Cook

On Wednesday 29 May 2013 23:56, James Hogan wrote:
> On 29 May 2013 18:36, Oleg Nesterov <oleg@redhat.com> wrote:
> > On 05/29, David Daney wrote:
> >>
> >> On 05/29/2013 10:01 AM, James Hogan wrote:
> >>> MIPS has 128 signals, the highest of which has the number 128. The
> >>
> >> I wonder if we should change the ABI and reduce the number of signals to
> >> 127 instead of this patch.
> >
> > Same thoughts...
> 
> I'll give it a try. I wouldn't have thought it'd break anything, but
> you never know. glibc (incorrectly) sets [__]SIGRTMAX to 127 already.
> On the other hand uClibc sets it to 128, so anything built against
> uClibc that uses signals SIGRTMAX-n (where n may be 0) or uses an
> excessive number of rt signals starting from SIGRTMIN (sounds
> unlikely) could well need an updated uClibc (or a full rebuild if it's
> crazy enough to use __SIGRTMAX).

Fixed in uclibc git: _NSIG is 128, __SIGRTMAX is 127
(_NSIG in libc is not the same as in kernel, but +1).

While at it, added extensive comment why it is so.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-06-28 20:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-29 17:01 [RFC PATCH] kernel/signal.c: avoid BUG_ON with SIG128 (MIPS) James Hogan
2013-05-29 17:01 ` James Hogan
2013-05-29 17:19 ` David Daney
2013-05-29 17:36   ` Oleg Nesterov
2013-05-29 21:56     ` James Hogan
2013-06-28 20:03       ` Denys Vlasenko

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.