Revisiting sigaltstack and implementation-internal signals

Submitted by Olaf Flebbe on Aug. 10, 2020, 8:15 a.m.

Details

Message ID 2142D551-13BE-4033-94F7-80A7B2C01890@oflebbe.de
State New
Series "Revisiting sigaltstack and implementation-internal signals"
Headers show

Commit Message

Olaf Flebbe Aug. 10, 2020, 8:15 a.m.
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

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

Patch hide | download patch | download mbox

--- /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;
 

Comments

Szabolcs Nagy Aug. 10, 2020, 3:41 p.m.
* 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.
Olaf Flebbe Aug. 10, 2020, 3:45 p.m.
> 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
Szabolcs Nagy Aug. 10, 2020, 4:24 p.m.
* 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.
Rich Felker Aug. 10, 2020, 4:27 p.m.
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.
Rich Felker Aug. 10, 2020, 4:36 p.m.
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
Olaf Flebbe Aug. 10, 2020, 4:57 p.m.
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
Rich Felker Aug. 10, 2020, 5 p.m.
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
>
Olaf Flebbe Aug. 10, 2020, 5:04 p.m.
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
>>
Rich Felker Aug. 10, 2020, 6:32 p.m.
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
> >>
Olaf Flebbe Aug. 10, 2020, 7:29 p.m.
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
>>>>