All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Hogan <james.hogan@imgtec.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	David Daney <ddaney@caviumnetworks.com>,
	David Daney <ddaney.cavm@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"Kees Cook" <keescook@chromium.org>,
	David Daney <david.daney@cavium.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	David Howells <dhowells@redhat.com>,
	Dave Jones <davej@redhat.com>, <linux-mips@linux-mips.org>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH v3] kernel/signal.c: fix BUG_ON with SIG128 (MIPS)
Date: Wed, 26 Jun 2013 12:07:44 +0100	[thread overview]
Message-ID: <51CACB80.5020706@imgtec.com> (raw)
In-Reply-To: <CAAG0J9-5J6=c=1VxEW6FevMHKsjShtbjM8G6Q1vu1P+LurQqoQ@mail.gmail.com>

On 25/06/13 23:13, James Hogan wrote:
> On 25 June 2013 22:40, Andrew Morton <akpm@linux-foundation.org> wrote:
>> Meanwhile, unprivileged users can make a MIPS kernel go BUG.
>>
>> How much of a problem is this?  Obviously less of a problem with MIPS
>> than it would be with some other CPU types, but I'd imagine it's still
>> awkward in some environments.
>>
>> If this _is_ considered a problem, can we think of some nasty little
>> hack which at least makes the effects less damaging, which we can also
>> put into -stable kernels?
> 
> The first rfc patch I sent sort of satisfies that by passing 127 if
> sig==128, or slightly better would be passing 126 if sig>=127 (so that
> SIFSIGNALED returns true). Effectively #ifdef'ing it on _NSIG>127 as
> this patch does may be preferable too.
> 
> That's probably the minimum change necessary to evade the BUG_ON
> without removing it. The wait status code will still be wrong, but it
> wasn't exactly right before so it's no worse.
> 
> IMO changing the ABI by reducing _NSIG to 127 or 126 isn't appropriate
> for stable.

How does this look for a nasty/stable fix?

WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	David Daney <ddaney@caviumnetworks.com>,
	David Daney <ddaney.cavm@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Kees Cook <keescook@chromium.org>,
	David Daney <david.daney@cavium.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	David Howells <dhowells@redhat.com>,
	Dave Jones <davej@redhat.com>,
	linux-mips@linux-mips.org, stable@vger.kernel.org
Subject: Re: [PATCH v3] kernel/signal.c: fix BUG_ON with SIG128 (MIPS)
Date: Wed, 26 Jun 2013 12:07:44 +0100	[thread overview]
Message-ID: <51CACB80.5020706@imgtec.com> (raw)
Message-ID: <20130626110744.q5eK40izMzJoc03ME5d5XXHRN7ZhDv3TWBC-0umPdfQ@z> (raw)
In-Reply-To: <CAAG0J9-5J6=c=1VxEW6FevMHKsjShtbjM8G6Q1vu1P+LurQqoQ@mail.gmail.com>

On 25/06/13 23:13, James Hogan wrote:
> On 25 June 2013 22:40, Andrew Morton <akpm@linux-foundation.org> wrote:
>> Meanwhile, unprivileged users can make a MIPS kernel go BUG.
>>
>> How much of a problem is this?  Obviously less of a problem with MIPS
>> than it would be with some other CPU types, but I'd imagine it's still
>> awkward in some environments.
>>
>> If this _is_ considered a problem, can we think of some nasty little
>> hack which at least makes the effects less damaging, which we can also
>> put into -stable kernels?
> 
> The first rfc patch I sent sort of satisfies that by passing 127 if
> sig==128, or slightly better would be passing 126 if sig>=127 (so that
> SIFSIGNALED returns true). Effectively #ifdef'ing it on _NSIG>127 as
> this patch does may be preferable too.
> 
> That's probably the minimum change necessary to evade the BUG_ON
> without removing it. The wait status code will still be wrong, but it
> wasn't exactly right before so it's no worse.
> 
> IMO changing the ABI by reducing _NSIG to 127 or 126 isn't appropriate
> for stable.

How does this look for a nasty/stable fix?

WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	David Daney <ddaney@caviumnetworks.com>,
	David Daney <ddaney.cavm@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"Kees Cook" <keescook@chromium.org>,
	David Daney <david.daney@cavium.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	David Howells <dhowells@redhat.com>,
	Dave Jones <davej@redhat.com>, <linux-mips@linux-mips.org>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH v3] kernel/signal.c: fix BUG_ON with SIG128 (MIPS)
Date: Wed, 26 Jun 2013 12:07:44 +0100	[thread overview]
Message-ID: <51CACB80.5020706@imgtec.com> (raw)
In-Reply-To: <CAAG0J9-5J6=c=1VxEW6FevMHKsjShtbjM8G6Q1vu1P+LurQqoQ@mail.gmail.com>

On 25/06/13 23:13, James Hogan wrote:
> On 25 June 2013 22:40, Andrew Morton <akpm@linux-foundation.org> wrote:
>> Meanwhile, unprivileged users can make a MIPS kernel go BUG.
>>
>> How much of a problem is this?  Obviously less of a problem with MIPS
>> than it would be with some other CPU types, but I'd imagine it's still
>> awkward in some environments.
>>
>> If this _is_ considered a problem, can we think of some nasty little
>> hack which at least makes the effects less damaging, which we can also
>> put into -stable kernels?
> 
> The first rfc patch I sent sort of satisfies that by passing 127 if
> sig==128, or slightly better would be passing 126 if sig>=127 (so that
> SIFSIGNALED returns true). Effectively #ifdef'ing it on _NSIG>127 as
> this patch does may be preferable too.
> 
> That's probably the minimum change necessary to evade the BUG_ON
> without removing it. The wait status code will still be wrong, but it
> wasn't exactly right before so it's no worse.
> 
> IMO changing the ABI by reducing _NSIG to 127 or 126 isn't appropriate
> for stable.

How does this look for a nasty/stable fix?

>From 94d734526d61f5c74fd2df1c3ecb677495fc7a23 Mon Sep 17 00:00:00 2001
From: James Hogan <james.hogan@imgtec.com>
Date: Wed, 26 Jun 2013 11:48:11 +0100
Subject: [PATCH 1/1] kernel/signal.c: fix BUG_ON with SIG128 (MIPS)

MIPS has 128 signals, the highest of which has the number 128 (they
start from 1). 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 */

As a quick fix, mask out higher bits in the signal number. This
effectively matches the exit code from other code paths but avoids the
BUG_ON.

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>
Cc: David Daney <david.daney@cavium.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Dave Jones <davej@redhat.com>
Cc: linux-mips@linux-mips.org
Cc: stable@vger.kernel.org
---
 kernel/signal.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 113411b..9ea8f4f 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2366,8 +2366,14 @@ relock:
 
 		/*
 		 * Death signals, no core dump.
+		 *
+		 * Some architectures (MIPS) have 128 signals which doesn't play
+		 * nicely with the exit code since there are only 7 bits to
+		 * store the terminating signal number. Mask out higher bits to
+		 * avoid overflowing into the core dump bit and triggering
+		 * BUG_ON in do_group_exit.
 		 */
-		do_group_exit(info->si_signo);
+		do_group_exit(info->si_signo & 0x7f);
 		/* NOTREACHED */
 	}
 	spin_unlock_irq(&sighand->siglock);
-- 
1.8.1.2


  reply	other threads:[~2013-06-26 11:09 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-21 13:39 [PATCH v3] kernel/signal.c: fix BUG_ON with SIG128 (MIPS) James Hogan
2013-06-21 13:39 ` James Hogan
2013-06-21 15:59 ` David Daney
2013-06-21 16:12   ` Ralf Baechle
2013-06-21 20:22   ` Oleg Nesterov
2013-06-21 20:45     ` David Daney
2013-06-21 20:45       ` David Daney
2013-06-22 19:09       ` Oleg Nesterov
2013-06-24  9:10         ` James Hogan
2013-06-24  9:10           ` James Hogan
2013-06-25 21:40           ` Andrew Morton
2013-06-25 21:40             ` Andrew Morton
2013-06-25 22:13             ` James Hogan
2013-06-26 11:07               ` James Hogan [this message]
2013-06-26 11:07                 ` James Hogan
2013-06-26 11:07                 ` James Hogan
2013-06-26 16:01                 ` Ralf Baechle
2013-06-26 16:14                 ` Oleg Nesterov
2013-06-26 16:59                   ` Ralf Baechle
2013-06-26 17:15                     ` Oleg Nesterov
2013-06-28 12:07                       ` James Hogan
2013-06-28 12:07                         ` James Hogan
2013-06-28 17:55                         ` Oleg Nesterov
2013-06-28 20:09                     ` Denys Vlasenko
2013-06-24  9:26   ` James Hogan
2013-06-24  9:26     ` James Hogan

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=51CACB80.5020706@imgtec.com \
    --to=james.hogan@imgtec.com \
    --cc=akpm@linux-foundation.org \
    --cc=davej@redhat.com \
    --cc=david.daney@cavium.com \
    --cc=ddaney.cavm@gmail.com \
    --cc=ddaney@caviumnetworks.com \
    --cc=dhowells@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=ralf@linux-mips.org \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.