mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Revisiting sigaltstack and implementation-internal signals
@ 2020-08-09  0:39 Rich Felker
  2020-08-09  7:54 ` Markus Wichmann
  2020-08-10  0:28 ` Ariadne Conill
  0 siblings, 2 replies; 18+ messages in thread
From: Rich Felker @ 2020-08-09  0:39 UTC (permalink / raw)
  To: musl

It's come up again, via Go this time (see
https://github.com/golang/go/issues/39857), that it would be nice to
have musl use the alternate signal stack for implementation-internal
signals. I've previously wanted to do this, but been unclear on (1)
whether it's permissible for the implementation to touch the
application-provided alternate stack when there is no signal delivered
on it (possibly not even any signal handlers installed), and (2)
whether we should care about breaking code that swaps off of and back
onto the alternate signal stack with swapcontext.

In regards to question (1), I believe this language from the
specification of sigaltstack is sufficient to resolve it:

    "The range of addresses starting at ss_sp up to but not including
    ss_sp+ ss_size is available to the implementation for use as the
    stack."

I read "available to the implementation" as implying that the
application can make no assumptions about values previously stored in
the memory being retained.

This still leaves (2) open, as well as whether there are any other
reasons why we shouldn't have implementation-internal signals using
the alternate stack.

Rich

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-09  0:39 [musl] Revisiting sigaltstack and implementation-internal signals Rich Felker
@ 2020-08-09  7:54 ` Markus Wichmann
  2020-08-10  0:06   ` Rich Felker
  2020-08-10  0:10   ` Ariadne Conill
  2020-08-10  0:28 ` Ariadne Conill
  1 sibling, 2 replies; 18+ messages in thread
From: Markus Wichmann @ 2020-08-09  7:54 UTC (permalink / raw)
  To: musl

On Sat, Aug 08, 2020 at 08:39:58PM -0400, Rich Felker wrote:
> on it (possibly not even any signal handlers installed), and (2)
> whether we should care about breaking code that swaps off of and back
> onto the alternate signal stack with swapcontext.

Would anything bad happen in that case? I thought, when a signal handler
with SA_ONSTACK is invoked, the altstack is marked with SS_ONSTACK and
will not be reset until the signal handler returns. If the handler does
not return, and does not call sigaltstack(), then the SS_ONSTACK remains
set, and therefore further signals with SA_ONSTACK will be delivered on
the current stack. Otherwise, if a signal were to arrive while the
altstack is in use, it would overwrite the old stack.

I cannot find a source code for swapcontext, but to my knowledge it
merely combines setjmp() and longjmp(), right? (setjmp() for the current
context and longjmp() for the other one). So no call to sigaltstack().

Ciao,
Markus

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-09  7:54 ` Markus Wichmann
@ 2020-08-10  0:06   ` Rich Felker
  2020-08-10 16:34     ` Markus Wichmann
  2020-08-10  0:10   ` Ariadne Conill
  1 sibling, 1 reply; 18+ messages in thread
From: Rich Felker @ 2020-08-10  0:06 UTC (permalink / raw)
  To: musl

On Sun, Aug 09, 2020 at 09:54:31AM +0200, Markus Wichmann wrote:
> On Sat, Aug 08, 2020 at 08:39:58PM -0400, Rich Felker wrote:
> > on it (possibly not even any signal handlers installed), and (2)
> > whether we should care about breaking code that swaps off of and back
> > onto the alternate signal stack with swapcontext.
> 
> Would anything bad happen in that case? I thought, when a signal handler
> with SA_ONSTACK is invoked, the altstack is marked with SS_ONSTACK and
> will not be reset until the signal handler returns. If the handler does
> not return, and does not call sigaltstack(), then the SS_ONSTACK remains
> set, and therefore further signals with SA_ONSTACK will be delivered on
> the current stack. Otherwise, if a signal were to arrive while the
> altstack is in use, it would overwrite the old stack.
> 
> I cannot find a source code for swapcontext, but to my knowledge it
> merely combines setjmp() and longjmp(), right? (setjmp() for the current
> context and longjmp() for the other one). So no call to sigaltstack().

My understanding is that SA_ONSTACK is just reported by the kernel if
the current stack pointer is inside the alternate stack. If the
application has moved off that stack and a signal arrives, it has
nowhere to know "where in the alternate stack it was" or that the
alternate stack was even already in use, and clobbers it from the
beginning if a new signal arrives that is to execute on the alternate
stack.

If you think this understanding is incorrect, we should research/test.

Rich

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-09  7:54 ` Markus Wichmann
  2020-08-10  0:06   ` Rich Felker
@ 2020-08-10  0:10   ` Ariadne Conill
  2020-08-10  0:26     ` Rich Felker
  1 sibling, 1 reply; 18+ messages in thread
From: Ariadne Conill @ 2020-08-10  0:10 UTC (permalink / raw)
  To: musl

Hello,

On 2020-08-09 01:54, Markus Wichmann wrote:
> On Sat, Aug 08, 2020 at 08:39:58PM -0400, Rich Felker wrote:
>> on it (possibly not even any signal handlers installed), and (2)
>> whether we should care about breaking code that swaps off of and back
>> onto the alternate signal stack with swapcontext.
> 
> Would anything bad happen in that case? I thought, when a signal handler
> with SA_ONSTACK is invoked, the altstack is marked with SS_ONSTACK and
> will not be reset until the signal handler returns. If the handler does
> not return, and does not call sigaltstack(), then the SS_ONSTACK remains
> set, and therefore further signals with SA_ONSTACK will be delivered on
> the current stack. Otherwise, if a signal were to arrive while the
> altstack is in use, it would overwrite the old stack.
> 
> I cannot find a source code for swapcontext, but to my knowledge it
> merely combines setjmp() and longjmp(), right? (setjmp() for the current
> context and longjmp() for the other one). So no call to sigaltstack().

musl itself does not ship the ucontext.h functions, to get them users 
must install my libucontext library.

libucontext does not make use of setjmp/longjmp or sigaltstack.  For the 
most part we just provide a simple context-swapping implementation in 
assembly, except on POWER where we just wrap the SYS_swapcontext syscall.

So any behavior change with sigaltstack() should not effect libucontext 
behavior in theory.

Ariadne

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10  0:10   ` Ariadne Conill
@ 2020-08-10  0:26     ` Rich Felker
  0 siblings, 0 replies; 18+ messages in thread
From: Rich Felker @ 2020-08-10  0:26 UTC (permalink / raw)
  To: musl

On Sun, Aug 09, 2020 at 06:10:02PM -0600, Ariadne Conill wrote:
> Hello,
> 
> On 2020-08-09 01:54, Markus Wichmann wrote:
> >On Sat, Aug 08, 2020 at 08:39:58PM -0400, Rich Felker wrote:
> >>on it (possibly not even any signal handlers installed), and (2)
> >>whether we should care about breaking code that swaps off of and back
> >>onto the alternate signal stack with swapcontext.
> >
> >Would anything bad happen in that case? I thought, when a signal handler
> >with SA_ONSTACK is invoked, the altstack is marked with SS_ONSTACK and
> >will not be reset until the signal handler returns. If the handler does
> >not return, and does not call sigaltstack(), then the SS_ONSTACK remains
> >set, and therefore further signals with SA_ONSTACK will be delivered on
> >the current stack. Otherwise, if a signal were to arrive while the
> >altstack is in use, it would overwrite the old stack.
> >
> >I cannot find a source code for swapcontext, but to my knowledge it
> >merely combines setjmp() and longjmp(), right? (setjmp() for the current
> >context and longjmp() for the other one). So no call to sigaltstack().
> 
> musl itself does not ship the ucontext.h functions, to get them
> users must install my libucontext library.
> 
> libucontext does not make use of setjmp/longjmp or sigaltstack.  For
> the most part we just provide a simple context-swapping
> implementation in assembly, except on POWER where we just wrap the
> SYS_swapcontext syscall.
> 
> So any behavior change with sigaltstack() should not effect
> libucontext behavior in theory.

The question is not about the behavior of the ucontext implementation
but of applications which use the API or do equivalent context-switch
things.

Rich

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-09  0:39 [musl] Revisiting sigaltstack and implementation-internal signals Rich Felker
  2020-08-09  7:54 ` Markus Wichmann
@ 2020-08-10  0:28 ` Ariadne Conill
  2020-08-10  8:15   ` Olaf Flebbe
  1 sibling, 1 reply; 18+ messages in thread
From: Ariadne Conill @ 2020-08-10  0:28 UTC (permalink / raw)
  To: musl

Hello,

On 2020-08-08 18:39, Rich Felker wrote:
> It's come up again, via Go this time (see
> https://github.com/golang/go/issues/39857), that it would be nice to
> have musl use the alternate signal stack for implementation-internal
> signals. I've previously wanted to do this, but been unclear on (1)
> whether it's permissible for the implementation to touch the
> application-provided alternate stack when there is no signal delivered
> on it (possibly not even any signal handlers installed), and (2)
> whether we should care about breaking code that swaps off of and back
> onto the alternate signal stack with swapcontext.
> 
> In regards to question (1), I believe this language from the
> specification of sigaltstack is sufficient to resolve it:
> 
>      "The range of addresses starting at ss_sp up to but not including
>      ss_sp+ ss_size is available to the implementation for use as the
>      stack."
> 
> I read "available to the implementation" as implying that the
> application can make no assumptions about values previously stored in
> the memory being retained.

This seems like a reasonable position.

> This still leaves (2) open, as well as whether there are any other
> reasons why we shouldn't have implementation-internal signals using
> the alternate stack.

In my opinion, mixing stacks with ucontext calls and sigaltstack is 
undefined behavior.  There is no way to guarantee the safety of such 
operations, or at least none that I can think of.

So personally, I think if people do that, they are basically asking for 
problems, and we have no obligation to fix those problems.

Ariadne

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10  0:28 ` Ariadne Conill
@ 2020-08-10  8:15   ` Olaf Flebbe
  2020-08-10 15:41     ` Szabolcs Nagy
  2020-08-10 16:36     ` Rich Felker
  0 siblings, 2 replies; 18+ messages in thread
From: Olaf Flebbe @ 2020-08-10  8:15 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2712 bytes --]

Hi, 

I have some problems to follow the discussion here.

It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.

I am proposing smthg like

--- /oss/musl-1.2.1/src/thread/synccall.c
+++ /work/musl/src/thread/synccall.c
@@ -45,7 +45,7 @@
 {
 	sigset_t oldmask;
 	int cs, i, r;
-	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
+	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, .sa_handler = handler };
 	pthread_t self = __pthread_self(), td;
 	int count = 0;
 
This will fix the problem with dynamic stacks, like go implements it. 
If the application does not install one, kernel will ignore SA_ONSTACK. (This is even specified by POSIX, since there is no error condition mentioned in man page specifically for this).

Tested with go and a glibc threaded setuid test tst-setuid3.c .

Best,
Olaf


> Am 10.08.2020 um 02:28 schrieb Ariadne Conill <ariadne@dereferenced.org>:
> 
> Hello,
> 
> On 2020-08-08 18:39, Rich Felker wrote:
>> It's come up again, via Go this time (see
>> https://github.com/golang/go/issues/39857), that it would be nice to
>> have musl use the alternate signal stack for implementation-internal
>> signals. I've previously wanted to do this, but been unclear on (1)
>> whether it's permissible for the implementation to touch the
>> application-provided alternate stack when there is no signal delivered
>> on it (possibly not even any signal handlers installed), and (2)
>> whether we should care about breaking code that swaps off of and back
>> onto the alternate signal stack with swapcontext.
>> In regards to question (1), I believe this language from the
>> specification of sigaltstack is sufficient to resolve it:
>>     "The range of addresses starting at ss_sp up to but not including
>>     ss_sp+ ss_size is available to the implementation for use as the
>>     stack."
>> I read "available to the implementation" as implying that the
>> application can make no assumptions about values previously stored in
>> the memory being retained.
> 
> This seems like a reasonable position.
> 
>> This still leaves (2) open, as well as whether there are any other
>> reasons why we shouldn't have implementation-internal signals using
>> the alternate stack.
> 
> In my opinion, mixing stacks with ucontext calls and sigaltstack is undefined behavior.  There is no way to guarantee the safety of such operations, or at least none that I can think of.
> 
> So personally, I think if people do that, they are basically asking for problems, and we have no obligation to fix those problems.
> 
> Ariadne


[-- Attachment #2: Type: text/html, Size: 6708 bytes --]

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10  8:15   ` Olaf Flebbe
@ 2020-08-10 15:41     ` Szabolcs Nagy
  2020-08-10 15:45       ` Olaf Flebbe
  2020-08-10 16:36     ` Rich Felker
  1 sibling, 1 reply; 18+ messages in thread
From: Szabolcs Nagy @ 2020-08-10 15:41 UTC (permalink / raw)
  To: Olaf Flebbe; +Cc: musl

* Olaf Flebbe <of@oflebbe.de> [2020-08-10 10:15:13 +0200]:
> I have some problems to follow the discussion here.
> 
> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
> 
> I am proposing smthg like
> 
> --- /oss/musl-1.2.1/src/thread/synccall.c
> +++ /work/musl/src/thread/synccall.c
> @@ -45,7 +45,7 @@
>  {
>  	sigset_t oldmask;
>  	int cs, i, r;
> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, .sa_handler = handler };
>  	pthread_t self = __pthread_self(), td;
>  	int count = 0;
>  
> This will fix the problem with dynamic stacks, like go implements it. 
> If the application does not install one, kernel will ignore SA_ONSTACK. (This is even specified by POSIX, since there is no error condition mentioned in man page specifically for this).
> 
> Tested with go and a glibc threaded setuid test tst-setuid3.c .

this will fail if an application calls sigaltstack,
then blocks all user signals that are SA_ONSTACK and
then deallocates the stack passed to sigaltstack.

it is important to discuss what an application may
or may not do, because the proposed change observably
modifies the behaviour.


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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 15:41     ` Szabolcs Nagy
@ 2020-08-10 15:45       ` Olaf Flebbe
  2020-08-10 16:24         ` Szabolcs Nagy
  2020-08-10 16:27         ` Rich Felker
  0 siblings, 2 replies; 18+ messages in thread
From: Olaf Flebbe @ 2020-08-10 15:45 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: musl



> Am 10.08.2020 um 17:41 schrieb Szabolcs Nagy <nsz@port70.net>:
> 
> * Olaf Flebbe <of@oflebbe.de> [2020-08-10 10:15:13 +0200]:
>> I have some problems to follow the discussion here.
>> 
>> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
>> 
>> I am proposing smthg like
>> 
>> --- /oss/musl-1.2.1/src/thread/synccall.c
>> +++ /work/musl/src/thread/synccall.c
>> @@ -45,7 +45,7 @@
>> {
>> 	sigset_t oldmask;
>> 	int cs, i, r;
>> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
>> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, .sa_handler = handler };
>> 	pthread_t self = __pthread_self(), td;
>> 	int count = 0;
>> 
>> This will fix the problem with dynamic stacks, like go implements it. 
>> If the application does not install one, kernel will ignore SA_ONSTACK. (This is even specified by POSIX, since there is no error condition mentioned in man page specifically for this).
>> 
>> Tested with go and a glibc threaded setuid test tst-setuid3.c .
> 
> this will fail if an application calls sigaltstack,
> then blocks all user signals that are SA_ONSTACK and
> then deallocates the stack passed to sigaltstack.
> 
> it is important to discuss what an application may
> or may not do, because the proposed change observably
> modifies the behaviour.


Deallocating an assigned sigaltstack without resetting sigaltstack  is undefined behaviour.

Olaf


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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 15:45       ` Olaf Flebbe
@ 2020-08-10 16:24         ` Szabolcs Nagy
  2020-08-10 16:27         ` Rich Felker
  1 sibling, 0 replies; 18+ messages in thread
From: Szabolcs Nagy @ 2020-08-10 16:24 UTC (permalink / raw)
  To: Olaf Flebbe; +Cc: musl

* Olaf Flebbe <of@oflebbe.de> [2020-08-10 17:45:06 +0200]:
> > Am 10.08.2020 um 17:41 schrieb Szabolcs Nagy <nsz@port70.net>:
> > * Olaf Flebbe <of@oflebbe.de> [2020-08-10 10:15:13 +0200]:
> >> I have some problems to follow the discussion here.
> >> 
> >> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
> >> 
> >> I am proposing smthg like
> >> 
> >> --- /oss/musl-1.2.1/src/thread/synccall.c
> >> +++ /work/musl/src/thread/synccall.c
> >> @@ -45,7 +45,7 @@
> >> {
> >> 	sigset_t oldmask;
> >> 	int cs, i, r;
> >> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
> >> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, .sa_handler = handler };
> >> 	pthread_t self = __pthread_self(), td;
> >> 	int count = 0;
> >> 
> >> This will fix the problem with dynamic stacks, like go implements it. 
> >> If the application does not install one, kernel will ignore SA_ONSTACK. (This is even specified by POSIX, since there is no error condition mentioned in man page specifically for this).
> >> 
> >> Tested with go and a glibc threaded setuid test tst-setuid3.c .
> > 
> > this will fail if an application calls sigaltstack,
> > then blocks all user signals that are SA_ONSTACK and
> > then deallocates the stack passed to sigaltstack.
> > 
> > it is important to discuss what an application may
> > or may not do, because the proposed change observably
> > modifies the behaviour.
> 
> 
> Deallocating an assigned sigaltstack without resetting sigaltstack  is undefined behaviour.

i don't see where posix specifies the lifetime of the stack
registered with sigaltstack.

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 15:45       ` Olaf Flebbe
  2020-08-10 16:24         ` Szabolcs Nagy
@ 2020-08-10 16:27         ` Rich Felker
  1 sibling, 0 replies; 18+ messages in thread
From: Rich Felker @ 2020-08-10 16:27 UTC (permalink / raw)
  To: musl

On Mon, Aug 10, 2020 at 05:45:06PM +0200, Olaf Flebbe wrote:
> 
> 
> > Am 10.08.2020 um 17:41 schrieb Szabolcs Nagy <nsz@port70.net>:
> > 
> > * Olaf Flebbe <of@oflebbe.de> [2020-08-10 10:15:13 +0200]:
> >> I have some problems to follow the discussion here.
> >> 
> >> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
> >> 
> >> I am proposing smthg like
> >> 
> >> --- /oss/musl-1.2.1/src/thread/synccall.c
> >> +++ /work/musl/src/thread/synccall.c
> >> @@ -45,7 +45,7 @@
> >> {
> >> 	sigset_t oldmask;
> >> 	int cs, i, r;
> >> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
> >> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, ..sa_handler = handler };
> >> 	pthread_t self = __pthread_self(), td;
> >> 	int count = 0;
> >> 
> >> This will fix the problem with dynamic stacks, like go implements it. 
> >> If the application does not install one, kernel will ignore SA_ONSTACK. (This is even specified by POSIX, since there is no error condition mentioned in man page specifically for this).
> >> 
> >> Tested with go and a glibc threaded setuid test tst-setuid3.c .
> > 
> > this will fail if an application calls sigaltstack,
> > then blocks all user signals that are SA_ONSTACK and
> > then deallocates the stack passed to sigaltstack.
> > 
> > it is important to discuss what an application may
> > or may not do, because the proposed change observably
> > modifies the behaviour.
> 
> 
> Deallocating an assigned sigaltstack without resetting sigaltstack  is undefined behaviour.

This is entirely correct and is not the relevant case.

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10  0:06   ` Rich Felker
@ 2020-08-10 16:34     ` Markus Wichmann
  0 siblings, 0 replies; 18+ messages in thread
From: Markus Wichmann @ 2020-08-10 16:34 UTC (permalink / raw)
  To: musl

On Sun, Aug 09, 2020 at 08:06:22PM -0400, Rich Felker wrote:
> My understanding is that SA_ONSTACK is just reported by the kernel if
> the current stack pointer is inside the alternate stack. If the
> application has moved off that stack and a signal arrives, it has
> nowhere to know "where in the alternate stack it was" or that the
> alternate stack was even already in use, and clobbers it from the
> beginning if a new signal arrives that is to execute on the alternate
> stack.
>
> If you think this understanding is incorrect, we should research/test.
>
> Rich

I thought the kernel would set the flag SS_ONSTACK in the sigaltstack
flags when switching to the altstack, and only remove it through
sigaltstack() or sigreturn(). However, this was merely based on what
understanding I managed to absorb from the manpages. So I had a look at
the Linux kernel code, specifically signal delivery. Now, that is
arch-specific code. But I had a look at two architectures and in both of
them you are right, so I guess the other architectures will handle it
the same way.

There is a function called get_sigframe(), that is present both in
arch/x86/kernel/signal.c and arch/powerpc/kernel/signal.c. It determines
where the signal frame is. In the x86 version, we can see that it will
choose the current stack by default, but it will choose the top of the
altstack if SA_ONSTACK is set in the sigaction flags and the current
ss_flags are 0. How are those calculated? Partly by calling
on_sig_stack(), which will test if the current SP is on the signal
stack. So it is not a sticky note in the kernel, it is indeed calculated
from SP.

In the PowerPC version, get_sigframe() calls sigsp(), which basically
does the same thing as above, but streamlined.

So yeah, if you are executing on altstack, and then you call a coroutine
on another stack, you just lost your altstack contents. And if another
signal arrives, the altstack will be clobbered again. Therefore mixing
coroutines and altstack is not safe, unless all signals with SA_ONSTACK
are blocked while the altstack is live but not in active use. Which is
impossible once musl uses altstack as well.

Ciao,
Markus

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10  8:15   ` Olaf Flebbe
  2020-08-10 15:41     ` Szabolcs Nagy
@ 2020-08-10 16:36     ` Rich Felker
  2020-08-10 16:57       ` Olaf Flebbe
  1 sibling, 1 reply; 18+ messages in thread
From: Rich Felker @ 2020-08-10 16:36 UTC (permalink / raw)
  To: musl

On Mon, Aug 10, 2020 at 10:15:13AM +0200, Olaf Flebbe wrote:
> Hi, 
> 
> I have some problems to follow the discussion here.
> 
> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
> 
> I am proposing smthg like
> 
> --- /oss/musl-1.2.1/src/thread/synccall.c
> +++ /work/musl/src/thread/synccall.c
> @@ -45,7 +45,7 @@
>  {
>  	sigset_t oldmask;
>  	int cs, i, r;
> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, ..sa_handler = handler };
>  	pthread_t self = __pthread_self(), td;
>  	int count = 0;
>  
> This will fix the problem with dynamic stacks, like go implements it. 
> If the application does not install one, kernel will ignore
> SA_ONSTACK. (This is even specified by POSIX, since there is no
> error condition mentioned in man page specifically for this).

It's fundamental, since presence and identity of an alternate stack
are thread-local properties and SA_ONSTACK is global to the signal
disposition.

The behavior we're concerned about this alterring is not the case
where an application does not install an alternate stack; of course
that's unaffected. The interesting case is where an application does
install one, but expects (albeit IMO wrongly; that's what we're trying
to establish) that the stack memory is not touched/clobbered unless
there's actually an SA_ONSTACK signal handler present to run on it and
such a signal arrives. With the proposed change, the memory for the
alternate stack can be clobbered asynchronously with no such signal
handler existing. (In case it's not clear, the above code is *not a
signal handler* from the perspective that's relevant; it's an
implementation detail internal to the implementation.)

One way such clobbering could manifest is when a signal handler
running on the alternate stack temporarily moves the stack pointer to
somewhere else (not on the alternate stack), via swapcontext or some
other method. In this case, if a signal for cancellation or synccall
arrives, the kernel will consider the alt stack not in use, and will
start using it again from the beginning, clobbering the still-running
frames.

Rich

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 16:36     ` Rich Felker
@ 2020-08-10 16:57       ` Olaf Flebbe
  2020-08-10 17:00         ` Rich Felker
  0 siblings, 1 reply; 18+ messages in thread
From: Olaf Flebbe @ 2020-08-10 16:57 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2620 bytes --]

Hi Rick ,

While the alternate stack is in use on cannot change the alternate stack.

See https://pubs.opengroup.org/onlinepubs/9699919799/ 
EPERM Error.

Olaf


> Am 10.08.2020 um 18:36 schrieb Rich Felker <dalias@libc.org>:
> 
> On Mon, Aug 10, 2020 at 10:15:13AM +0200, Olaf Flebbe wrote:
>> Hi, 
>> 
>> I have some problems to follow the discussion here.
>> 
>> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
>> 
>> I am proposing smthg like
>> 
>> --- /oss/musl-1.2.1/src/thread/synccall.c
>> +++ /work/musl/src/thread/synccall.c
>> @@ -45,7 +45,7 @@
>> {
>> 	sigset_t oldmask;
>> 	int cs, i, r;
>> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
>> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, ..sa_handler = handler };
>> 	pthread_t self = __pthread_self(), td;
>> 	int count = 0;
>> 
>> This will fix the problem with dynamic stacks, like go implements it. 
>> If the application does not install one, kernel will ignore
>> SA_ONSTACK. (This is even specified by POSIX, since there is no
>> error condition mentioned in man page specifically for this).
> 
> It's fundamental, since presence and identity of an alternate stack
> are thread-local properties and SA_ONSTACK is global to the signal
> disposition.
> 
> The behavior we're concerned about this alterring is not the case
> where an application does not install an alternate stack; of course
> that's unaffected. The interesting case is where an application does
> install one, but expects (albeit IMO wrongly; that's what we're trying
> to establish) that the stack memory is not touched/clobbered unless
> there's actually an SA_ONSTACK signal handler present to run on it and
> such a signal arrives. With the proposed change, the memory for the
> alternate stack can be clobbered asynchronously with no such signal
> handler existing. (In case it's not clear, the above code is *not a
> signal handler* from the perspective that's relevant; it's an
> implementation detail internal to the implementation.)
> 
> One way such clobbering could manifest is when a signal handler
> running on the alternate stack temporarily moves the stack pointer to
> somewhere else (not on the alternate stack), via swapcontext or some
> other method. In this case, if a signal for cancellation or synccall
> arrives, the kernel will consider the alt stack not in use, and will
> start using it again from the beginning, clobbering the still-running
> frames.
> 
> Rich


[-- Attachment #2: Type: text/html, Size: 21815 bytes --]

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 16:57       ` Olaf Flebbe
@ 2020-08-10 17:00         ` Rich Felker
  2020-08-10 17:04           ` Olaf Flebbe
  0 siblings, 1 reply; 18+ messages in thread
From: Rich Felker @ 2020-08-10 17:00 UTC (permalink / raw)
  To: musl

On Mon, Aug 10, 2020 at 06:57:21PM +0200, Olaf Flebbe wrote:
> Hi Rick ,
> 
> While the alternate stack is in use on cannot change the alternate stack.
> 
> See https://pubs.opengroup.org/onlinepubs/9699919799/ 
> EPERM Error.

No change of the alternate stack is described here. The minimal
example of the scenario only has one call to sigaltstack in the whole
program.


> > Am 10.08.2020 um 18:36 schrieb Rich Felker <dalias@libc.org>:
> > 
> > On Mon, Aug 10, 2020 at 10:15:13AM +0200, Olaf Flebbe wrote:
> >> Hi, 
> >> 
> >> I have some problems to follow the discussion here.
> >> 
> >> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
> >> 
> >> I am proposing smthg like
> >> 
> >> --- /oss/musl-1.2.1/src/thread/synccall.c
> >> +++ /work/musl/src/thread/synccall.c
> >> @@ -45,7 +45,7 @@
> >> {
> >> 	sigset_t oldmask;
> >> 	int cs, i, r;
> >> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
> >> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, ...sa_handler = handler };
> >> 	pthread_t self = __pthread_self(), td;
> >> 	int count = 0;
> >> 
> >> This will fix the problem with dynamic stacks, like go implements it. 
> >> If the application does not install one, kernel will ignore
> >> SA_ONSTACK. (This is even specified by POSIX, since there is no
> >> error condition mentioned in man page specifically for this).
> > 
> > It's fundamental, since presence and identity of an alternate stack
> > are thread-local properties and SA_ONSTACK is global to the signal
> > disposition.
> > 
> > The behavior we're concerned about this alterring is not the case
> > where an application does not install an alternate stack; of course
> > that's unaffected. The interesting case is where an application does
> > install one, but expects (albeit IMO wrongly; that's what we're trying
> > to establish) that the stack memory is not touched/clobbered unless
> > there's actually an SA_ONSTACK signal handler present to run on it and
> > such a signal arrives. With the proposed change, the memory for the
> > alternate stack can be clobbered asynchronously with no such signal
> > handler existing. (In case it's not clear, the above code is *not a
> > signal handler* from the perspective that's relevant; it's an
> > implementation detail internal to the implementation.)
> > 
> > One way such clobbering could manifest is when a signal handler
> > running on the alternate stack temporarily moves the stack pointer to
> > somewhere else (not on the alternate stack), via swapcontext or some
> > other method. In this case, if a signal for cancellation or synccall
> > arrives, the kernel will consider the alt stack not in use, and will
> > start using it again from the beginning, clobbering the still-running
> > frames.
> > 
> > Rich
> 

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 17:00         ` Rich Felker
@ 2020-08-10 17:04           ` Olaf Flebbe
  2020-08-10 18:32             ` Rich Felker
  0 siblings, 1 reply; 18+ messages in thread
From: Olaf Flebbe @ 2020-08-10 17:04 UTC (permalink / raw)
  To: musl

Hi Rick,

Thanks for explanation, indeed: This might be a problem, if the business logic of the handler is under application control.
But I was assuming that the handler context of __synccall is under musl control .

Olaf

> Am 10.08.2020 um 19:00 schrieb Rich Felker <dalias@libc.org>:
> 
> On Mon, Aug 10, 2020 at 06:57:21PM +0200, Olaf Flebbe wrote:
>> Hi Rick ,
>> 
>> While the alternate stack is in use on cannot change the alternate stack.
>> 
>> See https://pubs.opengroup.org/onlinepubs/9699919799/ 
>> EPERM Error.
> 
> No change of the alternate stack is described here. The minimal
> example of the scenario only has one call to sigaltstack in the whole
> program.
> 
> 
>>> Am 10.08.2020 um 18:36 schrieb Rich Felker <dalias@libc.org>:
>>> 
>>> On Mon, Aug 10, 2020 at 10:15:13AM +0200, Olaf Flebbe wrote:
>>>> Hi, 
>>>> 
>>>> I have some problems to follow the discussion here.
>>>> 
>>>> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
>>>> 
>>>> I am proposing smthg like
>>>> 
>>>> --- /oss/musl-1.2.1/src/thread/synccall.c
>>>> +++ /work/musl/src/thread/synccall.c
>>>> @@ -45,7 +45,7 @@
>>>> {
>>>> 	sigset_t oldmask;
>>>> 	int cs, i, r;
>>>> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
>>>> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, ...sa_handler = handler };
>>>> 	pthread_t self = __pthread_self(), td;
>>>> 	int count = 0;
>>>> 
>>>> This will fix the problem with dynamic stacks, like go implements it. 
>>>> If the application does not install one, kernel will ignore
>>>> SA_ONSTACK. (This is even specified by POSIX, since there is no
>>>> error condition mentioned in man page specifically for this).
>>> 
>>> It's fundamental, since presence and identity of an alternate stack
>>> are thread-local properties and SA_ONSTACK is global to the signal
>>> disposition.
>>> 
>>> The behavior we're concerned about this alterring is not the case
>>> where an application does not install an alternate stack; of course
>>> that's unaffected. The interesting case is where an application does
>>> install one, but expects (albeit IMO wrongly; that's what we're trying
>>> to establish) that the stack memory is not touched/clobbered unless
>>> there's actually an SA_ONSTACK signal handler present to run on it and
>>> such a signal arrives. With the proposed change, the memory for the
>>> alternate stack can be clobbered asynchronously with no such signal
>>> handler existing. (In case it's not clear, the above code is *not a
>>> signal handler* from the perspective that's relevant; it's an
>>> implementation detail internal to the implementation.)
>>> 
>>> One way such clobbering could manifest is when a signal handler
>>> running on the alternate stack temporarily moves the stack pointer to
>>> somewhere else (not on the alternate stack), via swapcontext or some
>>> other method. In this case, if a signal for cancellation or synccall
>>> arrives, the kernel will consider the alt stack not in use, and will
>>> start using it again from the beginning, clobbering the still-running
>>> frames.
>>> 
>>> Rich
>> 


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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 17:04           ` Olaf Flebbe
@ 2020-08-10 18:32             ` Rich Felker
  2020-08-10 19:29               ` Olaf Flebbe
  0 siblings, 1 reply; 18+ messages in thread
From: Rich Felker @ 2020-08-10 18:32 UTC (permalink / raw)
  To: musl

On Mon, Aug 10, 2020 at 07:04:36PM +0200, Olaf Flebbe wrote:
> Hi Rick,
> 
> Thanks for explanation, indeed: This might be a problem, if the
> business logic of the handler is under application control.
> But I was assuming that the handler context of __synccall is under
> musl control .

The handler in question is the one that's under application control
because the application installed it with intent for it to run on the
alternate stack. __synccall is the asynchronous clobbering of its
stack.

> > Am 10.08.2020 um 19:00 schrieb Rich Felker <dalias@libc.org>:
> > 
> > On Mon, Aug 10, 2020 at 06:57:21PM +0200, Olaf Flebbe wrote:
> >> Hi Rick ,
> >> 
> >> While the alternate stack is in use on cannot change the alternate stack.
> >> 
> >> See https://pubs.opengroup.org/onlinepubs/9699919799/ 
> >> EPERM Error.
> > 
> > No change of the alternate stack is described here. The minimal
> > example of the scenario only has one call to sigaltstack in the whole
> > program.
> > 
> > 
> >>> Am 10.08.2020 um 18:36 schrieb Rich Felker <dalias@libc.org>:
> >>> 
> >>> On Mon, Aug 10, 2020 at 10:15:13AM +0200, Olaf Flebbe wrote:
> >>>> Hi, 
> >>>> 
> >>>> I have some problems to follow the discussion here.
> >>>> 
> >>>> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
> >>>> 
> >>>> I am proposing smthg like
> >>>> 
> >>>> --- /oss/musl-1.2.1/src/thread/synccall.c
> >>>> +++ /work/musl/src/thread/synccall.c
> >>>> @@ -45,7 +45,7 @@
> >>>> {
> >>>> 	sigset_t oldmask;
> >>>> 	int cs, i, r;
> >>>> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
> >>>> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, ....sa_handler = handler };
> >>>> 	pthread_t self = __pthread_self(), td;
> >>>> 	int count = 0;
> >>>> 
> >>>> This will fix the problem with dynamic stacks, like go implements it. 
> >>>> If the application does not install one, kernel will ignore
> >>>> SA_ONSTACK. (This is even specified by POSIX, since there is no
> >>>> error condition mentioned in man page specifically for this).
> >>> 
> >>> It's fundamental, since presence and identity of an alternate stack
> >>> are thread-local properties and SA_ONSTACK is global to the signal
> >>> disposition.
> >>> 
> >>> The behavior we're concerned about this alterring is not the case
> >>> where an application does not install an alternate stack; of course
> >>> that's unaffected. The interesting case is where an application does
> >>> install one, but expects (albeit IMO wrongly; that's what we're trying
> >>> to establish) that the stack memory is not touched/clobbered unless
> >>> there's actually an SA_ONSTACK signal handler present to run on it and
> >>> such a signal arrives. With the proposed change, the memory for the
> >>> alternate stack can be clobbered asynchronously with no such signal
> >>> handler existing. (In case it's not clear, the above code is *not a
> >>> signal handler* from the perspective that's relevant; it's an
> >>> implementation detail internal to the implementation.)
> >>> 
> >>> One way such clobbering could manifest is when a signal handler
> >>> running on the alternate stack temporarily moves the stack pointer to
> >>> somewhere else (not on the alternate stack), via swapcontext or some
> >>> other method. In this case, if a signal for cancellation or synccall
> >>> arrives, the kernel will consider the alt stack not in use, and will
> >>> start using it again from the beginning, clobbering the still-running
> >>> frames.
> >>> 
> >>> Rich
> >> 

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

* Re: [musl] Revisiting sigaltstack and implementation-internal signals
  2020-08-10 18:32             ` Rich Felker
@ 2020-08-10 19:29               ` Olaf Flebbe
  0 siblings, 0 replies; 18+ messages in thread
From: Olaf Flebbe @ 2020-08-10 19:29 UTC (permalink / raw)
  To: musl

Hi Rick,

Since the alternate stack is only used for signal handlers, one can limit to the allowed signal safe functions:
There is a list of async-safe-signal functions copied from the OpenGroup documents.

https://man7.org/linux/man-pages/man7/signal-safety.7.html

swapcontext is not mentioned here :)

Sounds to me moving the stack pointer like you are describing is not allowed.

Installing the same stack both as regular and alternate stack sound to me like asking for trouble as well.

Best
   Olaf

> Am 10.08.2020 um 20:32 schrieb Rich Felker <dalias@libc.org>:
> 
> On Mon, Aug 10, 2020 at 07:04:36PM +0200, Olaf Flebbe wrote:
>> Hi Rick,
>> 
>> Thanks for explanation, indeed: This might be a problem, if the
>> business logic of the handler is under application control.
>> But I was assuming that the handler context of __synccall is under
>> musl control .
> 
> The handler in question is the one that's under application control
> because the application installed it with intent for it to run on the
> alternate stack. __synccall is the asynchronous clobbering of its
> stack.
> 
>>> Am 10.08.2020 um 19:00 schrieb Rich Felker <dalias@libc.org>:
>>> 
>>> On Mon, Aug 10, 2020 at 06:57:21PM +0200, Olaf Flebbe wrote:
>>>> Hi Rick ,
>>>> 
>>>> While the alternate stack is in use on cannot change the alternate stack.
>>>> 
>>>> See https://pubs.opengroup.org/onlinepubs/9699919799/ 
>>>> EPERM Error.
>>> 
>>> No change of the alternate stack is described here. The minimal
>>> example of the scenario only has one call to sigaltstack in the whole
>>> program.
>>> 
>>> 
>>>>> Am 10.08.2020 um 18:36 schrieb Rich Felker <dalias@libc.org>:
>>>>> 
>>>>> On Mon, Aug 10, 2020 at 10:15:13AM +0200, Olaf Flebbe wrote:
>>>>>> Hi, 
>>>>>> 
>>>>>> I have some problems to follow the discussion here.
>>>>>> 
>>>>>> It is not about musl to create an alternate stack, it is to *honor* the alternate stack, if the application installed one, for a reason.
>>>>>> 
>>>>>> I am proposing smthg like
>>>>>> 
>>>>>> --- /oss/musl-1.2.1/src/thread/synccall.c
>>>>>> +++ /work/musl/src/thread/synccall.c
>>>>>> @@ -45,7 +45,7 @@
>>>>>> {
>>>>>> 	sigset_t oldmask;
>>>>>> 	int cs, i, r;
>>>>>> -	struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler };
>>>>>> +	struct sigaction sa = { .sa_flags = SA_RESTART|SA_ONSTACK, ....sa_handler = handler };
>>>>>> 	pthread_t self = __pthread_self(), td;
>>>>>> 	int count = 0;
>>>>>> 
>>>>>> This will fix the problem with dynamic stacks, like go implements it. 
>>>>>> If the application does not install one, kernel will ignore
>>>>>> SA_ONSTACK. (This is even specified by POSIX, since there is no
>>>>>> error condition mentioned in man page specifically for this).
>>>>> 
>>>>> It's fundamental, since presence and identity of an alternate stack
>>>>> are thread-local properties and SA_ONSTACK is global to the signal
>>>>> disposition.
>>>>> 
>>>>> The behavior we're concerned about this alterring is not the case
>>>>> where an application does not install an alternate stack; of course
>>>>> that's unaffected. The interesting case is where an application does
>>>>> install one, but expects (albeit IMO wrongly; that's what we're trying
>>>>> to establish) that the stack memory is not touched/clobbered unless
>>>>> there's actually an SA_ONSTACK signal handler present to run on it and
>>>>> such a signal arrives. With the proposed change, the memory for the
>>>>> alternate stack can be clobbered asynchronously with no such signal
>>>>> handler existing. (In case it's not clear, the above code is *not a
>>>>> signal handler* from the perspective that's relevant; it's an
>>>>> implementation detail internal to the implementation.)
>>>>> 
>>>>> One way such clobbering could manifest is when a signal handler
>>>>> running on the alternate stack temporarily moves the stack pointer to
>>>>> somewhere else (not on the alternate stack), via swapcontext or some
>>>>> other method. In this case, if a signal for cancellation or synccall
>>>>> arrives, the kernel will consider the alt stack not in use, and will
>>>>> start using it again from the beginning, clobbering the still-running
>>>>> frames.
>>>>> 
>>>>> Rich
>>>> 


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

end of thread, other threads:[~2020-08-10 19:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-09  0:39 [musl] Revisiting sigaltstack and implementation-internal signals Rich Felker
2020-08-09  7:54 ` Markus Wichmann
2020-08-10  0:06   ` Rich Felker
2020-08-10 16:34     ` Markus Wichmann
2020-08-10  0:10   ` Ariadne Conill
2020-08-10  0:26     ` Rich Felker
2020-08-10  0:28 ` Ariadne Conill
2020-08-10  8:15   ` Olaf Flebbe
2020-08-10 15:41     ` Szabolcs Nagy
2020-08-10 15:45       ` Olaf Flebbe
2020-08-10 16:24         ` Szabolcs Nagy
2020-08-10 16:27         ` Rich Felker
2020-08-10 16:36     ` Rich Felker
2020-08-10 16:57       ` Olaf Flebbe
2020-08-10 17:00         ` Rich Felker
2020-08-10 17:04           ` Olaf Flebbe
2020-08-10 18:32             ` Rich Felker
2020-08-10 19:29               ` Olaf Flebbe

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).