All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Hogan <james.hogan@imgtec.com>
To: David Daney <ddaney.cavm@gmail.com>
Cc: <linux-kernel@vger.kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	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>
Subject: Re: [PATCH v3] kernel/signal.c: fix BUG_ON with SIG128 (MIPS)
Date: Mon, 24 Jun 2013 10:26:37 +0100	[thread overview]
Message-ID: <51C810CD.1070101@imgtec.com> (raw)
In-Reply-To: <51C47864.9030200@gmail.com>

On 21/06/13 16:59, David Daney wrote:
> On 06/21/2013 06:39 AM, James Hogan wrote:
>> 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 */
>>
>> Fundamentally the exit / wait status code cannot represent SIG128. In
>> fact it cannot represent SIG127 either as 0x7f represents a stopped
>> child.
>>
>> Therefore add sig_to_exitcode() and exitcode_to_sig() functions which
>> map signal numbers > 126 to exit code 126 and puts the remainder (i.e.
>> sig - 126) in higher bits. This allows WIFSIGNALED() to return true for
>> both SIG127 and SIG128, and allows WTERMSIG to be later updated to read
>> the correct signal number for SIG127 and SIG128.
> 
> I really hate this approach.
> 
> Can we just change the ABI to reduce the number of signals so that all
> the standard C library wait related macros don't have to be changed?
> 
> Think about it, any user space program using signal numbers 127 and 128
> doesn't work correctly as things exist today, so removing those two will
> be no great loss.

To clarify, I believe signals 127 and 128 do work during normal
operation. It's only if they cause program termination that odd things
happen in the wait status code (and of course the BUG_ON).

So reducing the number of signals will have potential to break things,
especially reducing below 127 (because glibc only reports SIGRTMAX()=127
already).

Cheers
James


WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com>
To: David Daney <ddaney.cavm@gmail.com>
Cc: linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	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
Subject: Re: [PATCH v3] kernel/signal.c: fix BUG_ON with SIG128 (MIPS)
Date: Mon, 24 Jun 2013 10:26:37 +0100	[thread overview]
Message-ID: <51C810CD.1070101@imgtec.com> (raw)
Message-ID: <20130624092637.STKHP3gjxRQdTbECw5grdctm1q5IQqBAS2z6fmRrIfc@z> (raw)
In-Reply-To: <51C47864.9030200@gmail.com>

On 21/06/13 16:59, David Daney wrote:
> On 06/21/2013 06:39 AM, James Hogan wrote:
>> 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 */
>>
>> Fundamentally the exit / wait status code cannot represent SIG128. In
>> fact it cannot represent SIG127 either as 0x7f represents a stopped
>> child.
>>
>> Therefore add sig_to_exitcode() and exitcode_to_sig() functions which
>> map signal numbers > 126 to exit code 126 and puts the remainder (i.e.
>> sig - 126) in higher bits. This allows WIFSIGNALED() to return true for
>> both SIG127 and SIG128, and allows WTERMSIG to be later updated to read
>> the correct signal number for SIG127 and SIG128.
> 
> I really hate this approach.
> 
> Can we just change the ABI to reduce the number of signals so that all
> the standard C library wait related macros don't have to be changed?
> 
> Think about it, any user space program using signal numbers 127 and 128
> doesn't work correctly as things exist today, so removing those two will
> be no great loss.

To clarify, I believe signals 127 and 128 do work during normal
operation. It's only if they cause program termination that odd things
happen in the wait status code (and of course the BUG_ON).

So reducing the number of signals will have potential to break things,
especially reducing below 127 (because glibc only reports SIGRTMAX()=127
already).

Cheers
James

  parent reply	other threads:[~2013-06-24  9:26 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
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 [this message]
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=51C810CD.1070101@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=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=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.