From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] powerpc/signal: Fix handling of SA_RESTORER sigaction flag
Date: Fri, 4 Feb 2022 11:00:32 +0000 [thread overview]
Message-ID: <381e3dc3-350c-c373-bc45-8dafd72ec011@csgroup.eu> (raw)
In-Reply-To: <87a6f7lynn.fsf@mpe.ellerman.id.au>
Le 04/02/2022 à 11:22, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> powerpc advertises support of SA_RESTORER sigaction flag.
>>
>> Make it the truth.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/kernel/signal_32.c | 8 ++++++--
>> arch/powerpc/kernel/signal_64.c | 4 +++-
>> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> Hi Christophe,
>
> I dug into the history a bit on this.
>
> The 32-bit port originally did not define SA_RESTORER in
> include/asm-ppc/signal.h, but it was added in 2.1.79.
>
> https://github.com/mpe/linux-fullhistory/commit/4e7e9c0d54ff5725a73d2210a950f8bc0f225073
>
> That commit added SA_RESTORER to the header, added code to get/set it in
> sys_sigaction(), but didn't add any code to use it for signal delivery.
>
>
> The 64-bit port was merged with SA_RESTORER already defined in
> include/asm-ppc64/signal.h:
>
> https://github.com/mpe/linux-fullhistory/commit/c3aa9878533e724f639852c3d951e6a169e04081
>
> Similarly there was code to set/get it in sys_sigaction(), but no code
> to use it for signal delivery.
>
>
> Later the two ports were merged, and the headers were moved and
> disintegrated into uapi, so we end up today with SA_RESTORER defined in
> arch/powerpc/include/uapi/asm/signal.h, but no code to use it.
>
> So essentially we've had SA_RESTORER defined since ancient kernels, but
> never actually supported using it for anything.
>
>
> One problem with enabling it now is there's no way for userspace to
> determine if it's on a fixed kernel or not. That makes it unusable for
> userspace, unless it does version checks, or is happy to break on all
> old kernels (not likely). We could solve that by defining
> SA_RESTORER_FIXED or something, but that's slightly gross.
>
> It's also described in the man page as "Not intended for application
> use", ie. it's intended for use by libc. I'm not sure there's any value
> in adding support for it to the kernel unless we know there's interest
> from glibc/musl in using it.
>
> So my inclination is that we should *not* add support for it, rather we
> should leave it unimplemented and remove SA_RESTORER from the header.
> There's precedent in riscv for not supporting it at all.
>
Nowadays, stacks are mapped noexec, so the fallback stack trampoline
cannot work anymore. If a process doesn't want exec stack and doesn't
want to map the VDSO, the SA_RESTORER is the only alternative to get
signal working.
On several architectures including arm64 and s390 only VDSO and
SA_RESTORER are supported, on stack signal trampoline is not supported
anymore.
So my idea was to first implement SA_RESTORER and then as a second step
to retire the on stack signal trampoline which has become useless with
noexec stacks.
See
https://elixir.bootlin.com/linux/v5.17-rc1/source/arch/arm64/kernel/signal.c#L761
or
https://elixir.bootlin.com/linux/v5.17-rc1/source/arch/s390/kernel/signal.c#L337
Christophe
WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>
Subject: Re: [PATCH 1/2] powerpc/signal: Fix handling of SA_RESTORER sigaction flag
Date: Fri, 4 Feb 2022 11:00:32 +0000 [thread overview]
Message-ID: <381e3dc3-350c-c373-bc45-8dafd72ec011@csgroup.eu> (raw)
In-Reply-To: <87a6f7lynn.fsf@mpe.ellerman.id.au>
Le 04/02/2022 à 11:22, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> powerpc advertises support of SA_RESTORER sigaction flag.
>>
>> Make it the truth.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/kernel/signal_32.c | 8 ++++++--
>> arch/powerpc/kernel/signal_64.c | 4 +++-
>> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> Hi Christophe,
>
> I dug into the history a bit on this.
>
> The 32-bit port originally did not define SA_RESTORER in
> include/asm-ppc/signal.h, but it was added in 2.1.79.
>
> https://github.com/mpe/linux-fullhistory/commit/4e7e9c0d54ff5725a73d2210a950f8bc0f225073
>
> That commit added SA_RESTORER to the header, added code to get/set it in
> sys_sigaction(), but didn't add any code to use it for signal delivery.
>
>
> The 64-bit port was merged with SA_RESTORER already defined in
> include/asm-ppc64/signal.h:
>
> https://github.com/mpe/linux-fullhistory/commit/c3aa9878533e724f639852c3d951e6a169e04081
>
> Similarly there was code to set/get it in sys_sigaction(), but no code
> to use it for signal delivery.
>
>
> Later the two ports were merged, and the headers were moved and
> disintegrated into uapi, so we end up today with SA_RESTORER defined in
> arch/powerpc/include/uapi/asm/signal.h, but no code to use it.
>
> So essentially we've had SA_RESTORER defined since ancient kernels, but
> never actually supported using it for anything.
>
>
> One problem with enabling it now is there's no way for userspace to
> determine if it's on a fixed kernel or not. That makes it unusable for
> userspace, unless it does version checks, or is happy to break on all
> old kernels (not likely). We could solve that by defining
> SA_RESTORER_FIXED or something, but that's slightly gross.
>
> It's also described in the man page as "Not intended for application
> use", ie. it's intended for use by libc. I'm not sure there's any value
> in adding support for it to the kernel unless we know there's interest
> from glibc/musl in using it.
>
> So my inclination is that we should *not* add support for it, rather we
> should leave it unimplemented and remove SA_RESTORER from the header.
> There's precedent in riscv for not supporting it at all.
>
Nowadays, stacks are mapped noexec, so the fallback stack trampoline
cannot work anymore. If a process doesn't want exec stack and doesn't
want to map the VDSO, the SA_RESTORER is the only alternative to get
signal working.
On several architectures including arm64 and s390 only VDSO and
SA_RESTORER are supported, on stack signal trampoline is not supported
anymore.
So my idea was to first implement SA_RESTORER and then as a second step
to retire the on stack signal trampoline which has become useless with
noexec stacks.
See
https://elixir.bootlin.com/linux/v5.17-rc1/source/arch/arm64/kernel/signal.c#L761
or
https://elixir.bootlin.com/linux/v5.17-rc1/source/arch/s390/kernel/signal.c#L337
Christophe
next prev parent reply other threads:[~2022-02-04 11:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-25 10:49 [PATCH 1/2] powerpc/signal: Fix handling of SA_RESTORER sigaction flag Christophe Leroy
2021-06-25 10:49 ` Christophe Leroy
2021-06-25 10:49 ` [PATCH RFC 2/2] powerpc/signal: Retire signal trampoline on stack Christophe Leroy
2021-06-25 10:49 ` Christophe Leroy
2022-02-04 10:22 ` [PATCH 1/2] powerpc/signal: Fix handling of SA_RESTORER sigaction flag Michael Ellerman
2022-02-04 10:22 ` Michael Ellerman
2022-02-04 11:00 ` Christophe Leroy [this message]
2022-02-04 11:00 ` Christophe Leroy
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=381e3dc3-350c-c373-bc45-8dafd72ec011@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=tuliom@linux.ibm.com \
/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.