What can a signal handler do with SIGSTKSZ?

What can a signal handler do with SIGSTKSZ?

[Carlos O'Donell]

In the continuing series of "What can X do with Y?" [1]

I would like to get consensus on what a signal handler can do with
SIGSTKSZ amount of space.

I propose the following:
~~~
The implementation only guarantees that a signal handler can
manipulate a reasonable amount of local variables (no more than
2 KiB worth), and can read and write to memory, carry out atomic
operations, and call simple C library functions that do similar
memory and simple string operations e.g. memcpy, memset, strcmp,
strcpy. The amount of signal stack allocated for SIGSTKSZ is not
sufficient to call complex signal-safe functions e.g. fork, _exit,
abort, nor any that can be canceled (requires enough stack for
cancellation). Any other operations or function calls in the
signal handler should be evaluated for runtime stack usage and
additional stack beyond SIGSTKSZ should be allocated.
~~~

Thoughts?

-- 
Cheers,
Carlos.

[1] "What can a thread do with PTHREAD_STACK_MIN?"
https://sourceware.org/ml/libc-alpha/2017-12/msg00751.html

Re: What can a signal handler do with SIGSTKSZ?

[Szabolcs Nagy]

On 11/01/2019 17:44, Carlos O'Donell wrote:
> In the continuing series of "What can X do with Y?" [1]
> 
> I would like to get consensus on what a signal handler can do with
> SIGSTKSZ amount of space.
> 
> I propose the following:
> ~~~
> The implementation only guarantees that a signal handler can
> manipulate a reasonable amount of local variables (no more than
> 2 KiB worth), and can read and write to memory, carry out atomic
> operations, and call simple C library functions that do similar
> memory and simple string operations e.g. memcpy, memset, strcmp,
> strcpy. The amount of signal stack allocated for SIGSTKSZ is not
> sufficient to call complex signal-safe functions e.g. fork, _exit,
> abort, nor any that can be canceled (requires enough stack for
> cancellation). Any other operations or function calls in the
> signal handler should be evaluated for runtime stack usage and
> additional stack beyond SIGSTKSZ should be allocated.
> ~~~
> 
> Thoughts?

instead of "local variables" i'd say: user
code may use X bytes stack space and the
rest is for the runtime (which is guaranteed
to be enough for simple libc calls,...).

X can be 2048.

runtime stack usage vs user stack usage can
be accounted based on stack usage of libc code
vs user code.

Re: What can a signal handler do with SIGSTKSZ?

[Carlos O'Donell]

On 1/11/19 2:02 PM, Szabolcs Nagy wrote:
> On 11/01/2019 17:44, Carlos O'Donell wrote:
>> In the continuing series of "What can X do with Y?" [1]
>>
>> I would like to get consensus on what a signal handler can do with
>> SIGSTKSZ amount of space.
>>
>> I propose the following:
>> ~~~
>> The implementation only guarantees that a signal handler can
>> manipulate a reasonable amount of local variables (no more than
>> 2 KiB worth), and can read and write to memory, carry out atomic
>> operations, and call simple C library functions that do similar
>> memory and simple string operations e.g. memcpy, memset, strcmp,
>> strcpy. The amount of signal stack allocated for SIGSTKSZ is not
>> sufficient to call complex signal-safe functions e.g. fork, _exit,
>> abort, nor any that can be canceled (requires enough stack for
>> cancellation). Any other operations or function calls in the
>> signal handler should be evaluated for runtime stack usage and
>> additional stack beyond SIGSTKSZ should be allocated.
>> ~~~
>>
>> Thoughts?
> 
> instead of "local variables" i'd say: user
> code may use X bytes stack space and the
> rest is for the runtime (which is guaranteed
> to be enough for simple libc calls,...).
> 
> X can be 2048.
> 
> runtime stack usage vs user stack usage can
> be accounted based on stack usage of libc code
> vs user code.
 
I was wondering if I could simplify the language and just say:

There are (SIGSTKSZ - MINSTKSZ) bytes to use upon entry to the
first signal handler, with stack space consumption happening
as the handler executes. Each subsequent nested signal handler
uses an additional MINSTKSZ bytes. 

What do you think?

-- 
Cheers,
Carlos.

Fwd: What can a signal handler do with SIGSTKSZ?

[Zack Weinberg]

[accidental off-list reply, sorry about that]

On Fri, Jan 11, 2019 at 12:44 PM Carlos O'Donell <carlos@redhat.com> wrote:
> The implementation only guarantees that a signal handler can
> manipulate a reasonable amount of local variables (no more than
> 2 KiB worth), and can read and write to memory, carry out atomic
> operations, and call simple C library functions that do similar
> memory and simple string operations e.g. memcpy, memset, strcmp,
> strcpy. The amount of signal stack allocated for SIGSTKSZ is not
> sufficient to call complex signal-safe functions e.g. fork, _exit,
> abort, nor any that can be canceled (requires enough stack for
> cancellation). Any other operations or function calls in the
> signal handler should be evaluated for runtime stack usage and
> additional stack beyond SIGSTKSZ should be allocated.

With my application programmer hat on, my expectation is that SIGSTKSZ
is supposed to be a _reasonable default_ amount of stack space, not a
minimum.  MINSIGSTKSZ is the minimum.  And this is too restrictive
even as the definition of the minimum.  The minimum (MINSIGSTKSZ)
should include adequate space to call _any_ async-signal-safe
function, most definitely including `_exit` and `abort`, provided I am
cautious regarding local variables in the signal handler.

In SIGSTKSZ space I expect to be able to do arbitrary computation and
call arbitrary library functions (provided that the signal is
delivered synchronously) and to not have to worry about the amount of
space consumed by stack variables or procedure calls. The only things
I _wouldn't_ feel safe doing are `alloca` and input-bounded recursion.

Having said that, if I put my systems programmer hat back on and look
at the actual values for these constants in the existing library...

[sysdeps/unix/sysv/linux/...]
bits/sigstack.h          MINSIGSTKSZ    2048
alpha/bits/sigstack.h    MINSIGSTKSZ    4096
powerpc/bits/sigstack.h  MINSIGSTKSZ    4096
sparc/bits/sigstack.h    MINSIGSTKSZ    4096
aarch64/bits/sigstack.h  MINSIGSTKSZ    5120
ia64/bits/sigstack.h     MINSIGSTKSZ    131027

bits/sigstack.h          SIGSTKSZ    8192
aarch64/bits/sigstack.h  SIGSTKSZ    16384
alpha/bits/sigstack.h    SIGSTKSZ    16384
powerpc/bits/sigstack.h  SIGSTKSZ    16384
sparc/bits/sigstack.h    SIGSTKSZ    16384
ia64/bits/sigstack.h     SIGSTKSZ    262144

... I reach the conclusion that they are all way too small, except
ia64, and the generic bits/sigstack.h (currently used only by Hurd)
has the right idea:

#define MINSIGSTKSZ     8192
#define SIGSTKSZ        (MINSIGSTKSZ + 32768)

Maybe bump 32768 up to 131072 to account for bloat since 1998.

Now, if 8192 bytes is not enough to call some async-signal-safe
functions, that's another problem and one I would like to see
addressed by making the unwind library more space-efficient or
something along those lines.

zw

Re: What can a signal handler do with SIGSTKSZ?

[Florian Weimer]

* Carlos O'Donell:

> In the continuing series of "What can X do with Y?" [1]
>
> I would like to get consensus on what a signal handler can do with
> SIGSTKSZ amount of space.
>
> I propose the following:
> ~~~
> The implementation only guarantees that a signal handler can
> manipulate a reasonable amount of local variables (no more than
> 2 KiB worth), and can read and write to memory, carry out atomic
> operations, and call simple C library functions that do similar
> memory and simple string operations e.g. memcpy, memset, strcmp,
> strcpy. The amount of signal stack allocated for SIGSTKSZ is not
> sufficient to call complex signal-safe functions e.g. fork, _exit,
> abort, nor any that can be canceled (requires enough stack for
> cancellation). Any other operations or function calls in the
> signal handler should be evaluated for runtime stack usage and
> additional stack beyond SIGSTKSZ should be allocated.
> ~~~
>
> Thoughts?

I have posted some numbers here:

[PATCH] nptl: New test nptl/tst-stack-usage
<https://sourceware.org/ml/libc-alpha/2018-12/msg00271.html>

x86_64: MINSIGSTKSZ too small for AVX-512F support
<https://sourceware.org/bugzilla/show_bug.cgi?id=20305>

I did not measure dynamic linker overhead.  On x86-64, due to the use of
XSAVE, I expect it to be comparable to signal delivery, perhaps slightly
worse.  That means that the default SIGSTKSZ (8192 bytes I think) is
really tight with your 2 KiB rule and lazy binding.

It's also non-obvious that vfprintf needs more than 8192 bytes of stack
with some streams (which includes the fake stream in dprintf, I think).

Furthermore, you'd also have to use sigaction with a proper sa_mask
value for correctness, which I believe is rare.

Thanks,
Florian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Florian Weimer]

* Zack Weinberg:

> Now, if 8192 bytes is not enough to call some async-signal-safe
> functions, that's another problem and one I would like to see
> addressed by making the unwind library more space-efficient or
> something along those lines.

Small nit: This is unrelated to async-signal-safe functions because size
considerations also apply to synchronously delivered signals, where few
(if any) restrictions exist.

Thanks,
Florian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Christian Brauner]

On January 11, 2019 9:00:29 PM GMT+01:00, Florian Weimer <fweimer@redhat.com> wrote:
>* Zack Weinberg:
>
>> Now, if 8192 bytes is not enough to call some async-signal-safe
>> functions, that's another problem and one I would like to see
>> addressed by making the unwind library more space-efficient or
>> something along those lines.
>
>Small nit: This is unrelated to async-signal-safe functions because
>size
>considerations also apply to synchronously delivered signals, where few
>(if any) restrictions exist.
>
>Thanks,
>Florian

Does this need kernel-side input?
Should we move parts of this to lkml or at least Cc a few people (Oleg, Andy, Eric)?
Just checking. :)

Thanks!
Christian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Zack Weinberg]

On Fri, Jan 11, 2019 at 3:00 PM Florian Weimer <fweimer@redhat.com> wrote:
> * Zack Weinberg:
> > Now, if 8192 bytes is not enough to call some async-signal-safe
> > functions, that's another problem and one I would like to see
> > addressed by making the unwind library more space-efficient or
> > something along those lines.
>
> Small nit: This is unrelated to async-signal-safe functions because size
> considerations also apply to synchronously delivered signals, where few
> (if any) restrictions exist.

Yeah, I was just using "async-signal-safe functions" as a convenient
proxy for "functions we know people are likely to try to call from a
signal handler of any kind."

I was pretty harsh on Carlos's proposal but, on further reflection,
given the fairly nasty ABI compatibility constraints we're working
with here (SIGSTKSZ having to be usable as the size of a statically
allocated char[], for instance), I could live with _most_ of it.  The
only change I insist on is, by hook or by crook we _must_ find a way
to make it safe to call `_exit` and `abort`.

Do you think we could push the kernel people to expose the space
requirement of a signal frame in some fashion that we could wrap up in
a new sysconf() constant?  Then we could deprecate the constants, in
the same way that long ago PAGESIZE was replaced by
sysconf(_SC_PAGESIZE).

zw

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Florian Weimer]

* Christian Brauner:

> On January 11, 2019 9:00:29 PM GMT+01:00, Florian Weimer <fweimer@redhat.com> wrote:
>>* Zack Weinberg:
>>
>>> Now, if 8192 bytes is not enough to call some async-signal-safe
>>> functions, that's another problem and one I would like to see
>>> addressed by making the unwind library more space-efficient or
>>> something along those lines.
>>
>>Small nit: This is unrelated to async-signal-safe functions because
>>size
>>considerations also apply to synchronously delivered signals, where few
>>(if any) restrictions exist.
>>
>>Thanks,
>>Florian
>
> Does this need kernel-side input?

> Should we move parts of this to lkml or at least Cc a few people
> (Oleg, Andy, Eric)?  Just checking. :)

I think I tried to bring this up in the past.  The truth is that there
aren't any good options.  The proposal I made here (not preserve
AVX-512F state in the handler if in a squeeze) is likely quite bad, too:

  <https://bugzilla.kernel.org/show_bug.cgi?id=153531>

Signals are just very hard to get right.

Thanks,
Florian

Re: What can a signal handler do with SIGSTKSZ?

[Szabolcs Nagy]

On 11/01/2019 19:11, Carlos O'Donell wrote:
> On 1/11/19 2:02 PM, Szabolcs Nagy wrote:
>> On 11/01/2019 17:44, Carlos O'Donell wrote:
>>> In the continuing series of "What can X do with Y?" [1]
>>>
>>> I would like to get consensus on what a signal handler can do with
>>> SIGSTKSZ amount of space.
>>>
>>> I propose the following:
>>> ~~~
>>> The implementation only guarantees that a signal handler can
>>> manipulate a reasonable amount of local variables (no more than
>>> 2 KiB worth), and can read and write to memory, carry out atomic
>>> operations, and call simple C library functions that do similar
>>> memory and simple string operations e.g. memcpy, memset, strcmp,
>>> strcpy. The amount of signal stack allocated for SIGSTKSZ is not
>>> sufficient to call complex signal-safe functions e.g. fork, _exit,
>>> abort, nor any that can be canceled (requires enough stack for
>>> cancellation). Any other operations or function calls in the
>>> signal handler should be evaluated for runtime stack usage and
>>> additional stack beyond SIGSTKSZ should be allocated.
>>> ~~~
>>>
>>> Thoughts?
>>
>> instead of "local variables" i'd say: user
>> code may use X bytes stack space and the
>> rest is for the runtime (which is guaranteed
>> to be enough for simple libc calls,...).
>>
>> X can be 2048.
>>
>> runtime stack usage vs user stack usage can
>> be accounted based on stack usage of libc code
>> vs user code.
>  
> I was wondering if I could simplify the language and just say:
> 
> There are (SIGSTKSZ - MINSTKSZ) bytes to use upon entry to the
> first signal handler, with stack space consumption happening
> as the handler executes. Each subsequent nested signal handler
> uses an additional MINSTKSZ bytes. 
> 
> What do you think?

works for me, but

MINSIGSTKSZ can be interpreted as just enough for the
kernel to deliver the signal and call the signal handler.
(currently it's not enough on x86_64 and on aarch64 with
sve. on aarch64 there is kernel provided AT_MINSIGSTKSZ
auxv value that is enough, but really posix requires a
sysconf for this instead of a fixed macro then.)

so users should be aware that there may be additional
overheads not visible in their code (we don't wrap
signal handlers currently but might, there is lazy
binding overhead for extern calls) so not all
SIGSTKSZ - MINSIGSTKSZ is for the user: it is for
user code + runtime.

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Christian Brauner]

On January 11, 2019 9:14:51 PM GMT+01:00, Florian Weimer <fweimer@redhat.com> wrote:
>* Christian Brauner:
>
>> On January 11, 2019 9:00:29 PM GMT+01:00, Florian Weimer
><fweimer@redhat.com> wrote:
>>>* Zack Weinberg:
>>>
>>>> Now, if 8192 bytes is not enough to call some async-signal-safe
>>>> functions, that's another problem and one I would like to see
>>>> addressed by making the unwind library more space-efficient or
>>>> something along those lines.
>>>
>>>Small nit: This is unrelated to async-signal-safe functions because
>>>size
>>>considerations also apply to synchronously delivered signals, where
>few
>>>(if any) restrictions exist.
>>>
>>>Thanks,
>>>Florian
>>
>> Does this need kernel-side input?
>
>> Should we move parts of this to lkml or at least Cc a few people
>> (Oleg, Andy, Eric)?  Just checking. :)
>
>I think I tried to bring this up in the past.  The truth is that there
>aren't any good options.  The proposal I made here (not preserve
>AVX-512F state in the handler if in a squeeze) is likely quite bad,
>too:
>
>  <https://bugzilla.kernel.org/show_bug.cgi?id=153531>
>
>Signals are just very hard to get right.
>
>Thanks,
>Florian

Hm it looks like the bug report didn't see any response.
This is the first time I heard about it.
Sorry for repeating: has this been brought up somewhere on a kernel mailing list?

Christian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Florian Weimer]

* Zack Weinberg:

> I was pretty harsh on Carlos's proposal but, on further reflection,
> given the fairly nasty ABI compatibility constraints we're working
> with here (SIGSTKSZ having to be usable as the size of a statically
> allocated char[], for instance), I could live with _most_ of it.  The
> only change I insist on is, by hook or by crook we _must_ find a way
> to make it safe to call `_exit` and `abort`.

abort delivers another signal, so that's going to be impossible to
support with a really small stack, I think.

> Do you think we could push the kernel people to expose the space
> requirement of a signal frame in some fashion that we could wrap up in
> a new sysconf() constant?  Then we could deprecate the constants, in
> the same way that long ago PAGESIZE was replaced by
> sysconf(_SC_PAGESIZE).

That's an interesting idea.  sigaltstack could also check that the size
is at least that large, but then the question is how many sigaltstack
users check the error return value.

However, based on what I saw in the kernel sources, it's not that they
have an exact upper bound in the sources or even at run time.  I think
the code simply uses space as it proceeds (at least on x86).  But
perhaps I misread it.

Thanks,
Florian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Zack Weinberg]

On Fri, Jan 11, 2019 at 3:29 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Zack Weinberg:
> > by hook or by crook we _must_ find a way
> > to make it safe to call `_exit` and `abort`.
>
> abort delivers another signal, so that's going to be impossible to
> support with a really small stack, I think.

see below

> > Do you think we could push the kernel people to expose the space
> > requirement of a signal frame in some fashion that we could wrap up in
> > a new sysconf() constant?  Then we could deprecate the constants, in
> > the same way that long ago PAGESIZE was replaced by
> > sysconf(_SC_PAGESIZE).
>
> That's an interesting idea.  sigaltstack could also check that the size
> is at least that large, but then the question is how many sigaltstack
> users check the error return value.

Probably not very many...

> However, based on what I saw in the kernel sources, it's not that they
> have an exact upper bound in the sources or even at run time.  I think
> the code simply uses space as it proceeds (at least on x86).  But
> perhaps I misread it.

Yeesh.  I think that has got to get fixed, independent of everything else.

This is not really my area of expertise, but here's what comes to mind
for a way forward:

 - Kernel-side signal delivery code is revamped so it knows an upper
bound (perhaps not an exact one) on the space requirement for a signal
frame, and exposes that to user space in a way that we can wrap up in
a sysconf() query.
 - Kernel-side signal delivery code is revamped so that it knows how
much stack space is available (no matter where the stack came from --
in the absence of other information I suppose it can use the bottom of
the memory mapping, but for sigaltstack it ought to use the specified
size) and, if there isn't enough space to write a complete signal
frame, it terminates the process as-if by the default action of
SIGSEGV instead of clobbering anything.
 - We add glibc functions

    int alloc_sigstack(stack_t *ss, size_t scratch_needed);
    void free_sigstack(stack_t *ss);

alloc_sigstack allocates space for a signal stack, guaranteeing to
provide at least scratch_needed bytes of space for the signal
handler's local variables and any functions it calls, plus an
appropriate amount of overhead space for the signal frame.  The total
allocation will be rounded up to a whole number of pages and will have
no-access guard pages on either side of it.  It can fail.
free_sigstack, naturally, deallocates the stack again.

The second change is what deals with people trying to call abort()
from inside a signal handler when there might not be enough space for
another signal frame.  It also should solve the AVX2 lack-of-space
issue -- programs using the old constants may crash but they shouldn't
behave unpredictably.

The point of alloc_sigstack and free_sigstack, over just telling
people to use the new sysconf query, is that they give you guard
pages, which will help with programs underestimating the amount of
space they need.

Christian: I don't know if this stuff has been brought up on the
kernel lists before, but it probably does need to be.

zw

zw

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Szabolcs Nagy]

On 11/01/2019 23:59, Zack Weinberg wrote:
> On Fri, Jan 11, 2019 at 3:29 PM Florian Weimer <fweimer@redhat.com> wrote:
>> * Zack Weinberg:
>>> Do you think we could push the kernel people to expose the space
>>> requirement of a signal frame in some fashion that we could wrap up in
>>> a new sysconf() constant?  Then we could deprecate the constants, in
>>> the same way that long ago PAGESIZE was replaced by
>>> sysconf(_SC_PAGESIZE).
>>
>> That's an interesting idea.  sigaltstack could also check that the size
>> is at least that large, but then the question is how many sigaltstack
>> users check the error return value.
> 
> Probably not very many...
> 
>> However, based on what I saw in the kernel sources, it's not that they
>> have an exact upper bound in the sources or even at run time.  I think
>> the code simply uses space as it proceeds (at least on x86).  But
>> perhaps I misread it.
> 
> Yeesh.  I think that has got to get fixed, independent of everything else.
> 
> This is not really my area of expertise, but here's what comes to mind
> for a way forward:

as far as i know aarch64 kernel calculates and reports worst
case stack frame size precisely, so that's probably just an
x86 issue.

i think proposing sysconf(_SC_{MIN}SIGSTKSZ) for posix is the
right solution with the kernel providing an upper bound of the
stack frame in AT_MINSIGSTKSZ (as it already does on aarch64).

with the current wording of the standard SIGSTKSZ and MINSIGSTKSZ
definition cannot be omitted when they are runtime variables,
so posix needs to be updated.

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Adhemerval Zanella]

On 14/01/2019 09:18, Szabolcs Nagy wrote:
> On 11/01/2019 23:59, Zack Weinberg wrote:
>> On Fri, Jan 11, 2019 at 3:29 PM Florian Weimer <fweimer@redhat.com> wrote:
>>> * Zack Weinberg:
>>>> Do you think we could push the kernel people to expose the space
>>>> requirement of a signal frame in some fashion that we could wrap up in
>>>> a new sysconf() constant?  Then we could deprecate the constants, in
>>>> the same way that long ago PAGESIZE was replaced by
>>>> sysconf(_SC_PAGESIZE).
>>>
>>> That's an interesting idea.  sigaltstack could also check that the size
>>> is at least that large, but then the question is how many sigaltstack
>>> users check the error return value.
>>
>> Probably not very many...
>>
>>> However, based on what I saw in the kernel sources, it's not that they
>>> have an exact upper bound in the sources or even at run time.  I think
>>> the code simply uses space as it proceeds (at least on x86).  But
>>> perhaps I misread it.
>>
>> Yeesh.  I think that has got to get fixed, independent of everything else.
>>
>> This is not really my area of expertise, but here's what comes to mind
>> for a way forward:
> 
> as far as i know aarch64 kernel calculates and reports worst
> case stack frame size precisely, so that's probably just an
> x86 issue.
> 
> i think proposing sysconf(_SC_{MIN}SIGSTKSZ) for posix is the
> right solution with the kernel providing an upper bound of the
> stack frame in AT_MINSIGSTKSZ (as it already does on aarch64).
> 
> with the current wording of the standard SIGSTKSZ and MINSIGSTKSZ
> definition cannot be omitted when they are runtime variables,
> so posix needs to be updated.
> 

From an implementation standpoint, how the lib would calculate _SC_SIGSTKSZ?
Just plus constant based on _SC_MINSIGSTKSZ? If it is the idea we might go
only with _SC_MINSIGSSTKSZ and export the value adjustment instead.

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Florian Weimer]

* Christian Brauner:

>>  <https://bugzilla.kernel.org/show_bug.cgi?id=153531>

> Hm it looks like the bug report didn't see any response.  This is the
> first time I heard about it.  Sorry for repeating: has this been
> brought up somewhere on a kernel mailing list?

I don't remember a discussion.

Thanks,
Florian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Florian Weimer]

* Szabolcs Nagy:

> i think proposing sysconf(_SC_{MIN}SIGSTKSZ) for posix is the
> right solution with the kernel providing an upper bound of the
> stack frame in AT_MINSIGSTKSZ (as it already does on aarch64).

It's still a bit nasty for process migration, but at least it should for
VM migration.  I think that would be a reasonable compromise.

Of course, we'd still need a way to determine how much we should add on
top of AT_MINSIGSTKSZ for the application developer's benefit. 8-)

Thanks,
Florian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Carlos O'Donell]

On 1/14/19 11:18 AM, Florian Weimer wrote:
> * Szabolcs Nagy:
> 
>> i think proposing sysconf(_SC_{MIN}SIGSTKSZ) for posix is the
>> right solution with the kernel providing an upper bound of the
>> stack frame in AT_MINSIGSTKSZ (as it already does on aarch64).
> 
> It's still a bit nasty for process migration, but at least it should for
> VM migration.  I think that would be a reasonable compromise.
> 
> Of course, we'd still need a way to determine how much we should add on
> top of AT_MINSIGSTKSZ for the application developer's benefit. 8-)

The value of SIGSTKSZ seems to be a "culturally relevant detail" that will
probably be impossible to pin down to a real value.

If *I* were a developer I might expect as Zach pointed out, that I can call
every function on the list of callable AS-safe functions, at least once,
without recursion, and expect them to operate correctly.

A test case for this would therefore be a main, that register a handler
that exercises *all* functions in the AS-safe list, and then looks for
stack corruption at each execution.

The test could also be used to set the value to some accepted value for
all machines.

Can we do this in practice?

-- 
Cheers,
Carlos.

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Florian Weimer]

* Carlos O'Donell:

> On 1/14/19 11:18 AM, Florian Weimer wrote:
>> * Szabolcs Nagy:
>> 
>>> i think proposing sysconf(_SC_{MIN}SIGSTKSZ) for posix is the
>>> right solution with the kernel providing an upper bound of the
>>> stack frame in AT_MINSIGSTKSZ (as it already does on aarch64).
>> 
>> It's still a bit nasty for process migration, but at least it should for
>> VM migration.  I think that would be a reasonable compromise.
>> 
>> Of course, we'd still need a way to determine how much we should add on
>> top of AT_MINSIGSTKSZ for the application developer's benefit. 8-)
>
> The value of SIGSTKSZ seems to be a "culturally relevant detail" that will
> probably be impossible to pin down to a real value.

The embedded folks have tooling for this: avoid recursive calls,
documenting stack size requirements, all that.  There are few things in
GCC we could use (but I don't think upstream focus is on C/C++ in this
area).  Then we could have some hard numbers and also statically verify
them.

With callbacks, things get more complicated obviously, but I think it
would still be manageable.

Thanks,
Florian

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Zack Weinberg]

On Mon, Jan 14, 2019 at 6:29 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 14/01/2019 09:18, Szabolcs Nagy wrote:
> >
> > as far as i know aarch64 kernel calculates and reports worst
> > case stack frame size precisely, so that's probably just an
> > x86 issue.

That's good to hear.  All of the architectures' signal frame
construction code should be checked and updated if necessary -- alas,
I doubt there's any good way to automate the problem, since signal
delivery is so low-level and arch-specific (but I'm not a kernel
hacker).

> > i think proposing sysconf(_SC_{MIN}SIGSTKSZ) for posix is the
> > right solution with the kernel providing an upper bound of the
> > stack frame in AT_MINSIGSTKSZ (as it already does on aarch64).
> >
> > with the current wording of the standard SIGSTKSZ and MINSIGSTKSZ
> > definition cannot be omitted when they are runtime variables,
> > so posix needs to be updated.
>
> From an implementation standpoint, how the lib would calculate _SC_SIGSTKSZ?
> Just plus constant based on _SC_MINSIGSTKSZ? If it is the idea we might go
> only with _SC_MINSIGSSTKSZ and export the value adjustment instead.

That seems fine to me.

For this new interface, the backward compatibility concerns I raised
regarding what you can do in MINSIGSTKSZ don't necessarily apply, and
it would make sense for it to be truly a minimum.  I would suggest we
define "minimum" in terms of what the C standard - not POSIX - allows
you to do in a signal handler, which is almost nothing:  you're
guaranteed to have enough space in sysconf(_SC_MINSIGSTKSZ) for this:

    static volatile sig_atomic_t signal_flag = 0;
    static void handler(int unused)
    {
       flag = 1;
    }

and this:

   static atomic_uint signal_count = 0;
   static void handler (int unused)
   {
      atomic_fetch_add (&signal_count, 1);
   }

but *not* for this:

   static int sockets[MAX_SOCKETS];
   static void handler (int sig)
   {
      for (int i = 0; i < MAX_SOCKETS; i++)
      {
          if (sockets[i] == -1) continue;
          write (sockets[i], "\r\n500 Service shutting down
unexpectedly\r\n\r\n", 44);
          close (sockets[i]);
      }
      signal (sig, SIG_DFL);
      raise (sig);
   }

or for anything for which you would need to use SA_SIGINFO, or for
recursive signal delivery.

zw

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Szabolcs Nagy]

On 14/01/2019 16:22, Carlos O'Donell wrote:
> On 1/14/19 11:18 AM, Florian Weimer wrote:
>> * Szabolcs Nagy:
>>
>>> i think proposing sysconf(_SC_{MIN}SIGSTKSZ) for posix is the
>>> right solution with the kernel providing an upper bound of the
>>> stack frame in AT_MINSIGSTKSZ (as it already does on aarch64).
>>
>> It's still a bit nasty for process migration, but at least it should for
>> VM migration.  I think that would be a reasonable compromise.
>>
>> Of course, we'd still need a way to determine how much we should add on
>> top of AT_MINSIGSTKSZ for the application developer's benefit. 8-)
> 
> The value of SIGSTKSZ seems to be a "culturally relevant detail" that will
> probably be impossible to pin down to a real value.
> 
> If *I* were a developer I might expect as Zach pointed out, that I can call
> every function on the list of callable AS-safe functions, at least once,
> without recursion, and expect them to operate correctly.
> 
> A test case for this would therefore be a main, that register a handler
> that exercises *all* functions in the AS-safe list, and then looks for
> stack corruption at each execution.
> 
> The test could also be used to set the value to some accepted value for
> all machines.
> 
> Can we do this in practice?

e.g. exec functions that need to copy their
arguments to an on stack argv array may fail
depending on how many args are passed.

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Joseph Myers]

On Mon, 14 Jan 2019, Carlos O'Donell wrote:

> If *I* were a developer I might expect as Zach pointed out, that I can call
> every function on the list of callable AS-safe functions, at least once,
> without recursion, and expect them to operate correctly.
> 
> A test case for this would therefore be a main, that register a handler
> that exercises *all* functions in the AS-safe list, and then looks for
> stack corruption at each execution.

The POSIX AS-safe list, or the functions that are currently documented as 
AS-safe in glibc (an observed property, not necessarily a commitment to an 
API)?  (strtold uses a fair amount of stack, for example, and is listed as 
AS-safe in glibc.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Carlos O'Donell]

On 1/14/19 11:34 AM, Zack Weinberg wrote:
> For this new interface, the backward compatibility concerns I raised
> regarding what you can do in MINSIGSTKSZ don't necessarily apply, and
> it would make sense for it to be truly a minimum.  I would suggest we
> define "minimum" in terms of what the C standard - not POSIX - allows
> you to do in a signal handler, which is almost nothing:  you're
> guaranteed to have enough space in sysconf(_SC_MINSIGSTKSZ) for this:
> 
>     static volatile sig_atomic_t signal_flag = 0;
>     static void handler(int unused)
>     {
>        flag = 1;
>     }
> 
> and this:
> 
>    static atomic_uint signal_count = 0;
>    static void handler (int unused)
>    {
>       atomic_fetch_add (&signal_count, 1);
>    }

By my reading we'd also have to support:  abort, _Exit, 
quick_exit, and signal.

What I'm not quite sure about is that errno has visible
changes from these functions, particularly signal failing
and if we must also exercise writing to a TLS errno.

-- 
Cheers,
Carlos.

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Carlos O'Donell]

On 1/14/19 1:19 PM, Joseph Myers wrote:
> On Mon, 14 Jan 2019, Carlos O'Donell wrote:
> 
>> If *I* were a developer I might expect as Zach pointed out, that I can call
>> every function on the list of callable AS-safe functions, at least once,
>> without recursion, and expect them to operate correctly.
>>
>> A test case for this would therefore be a main, that register a handler
>> that exercises *all* functions in the AS-safe list, and then looks for
>> stack corruption at each execution.
> 
> The POSIX AS-safe list, or the functions that are currently documented as 
> AS-safe in glibc (an observed property, not necessarily a commitment to an 
> API)?  (strtold uses a fair amount of stack, for example, and is listed as 
> AS-safe in glibc.)
 
In the comment above I was only considering the POSIX AS-safe list.

-- 
Cheers,
Carlos.

Re: Fwd: What can a signal handler do with SIGSTKSZ?

[Christian Brauner]

On Fri, Jan 11, 2019 at 06:59:42PM -0500, Zack Weinberg wrote:
> On Fri, Jan 11, 2019 at 3:29 PM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * Zack Weinberg:
> > > by hook or by crook we _must_ find a way
> > > to make it safe to call `_exit` and `abort`.
> >
> > abort delivers another signal, so that's going to be impossible to
> > support with a really small stack, I think.
> 
> see below
> 
> > > Do you think we could push the kernel people to expose the space
> > > requirement of a signal frame in some fashion that we could wrap up in
> > > a new sysconf() constant?  Then we could deprecate the constants, in
> > > the same way that long ago PAGESIZE was replaced by
> > > sysconf(_SC_PAGESIZE).
> >
> > That's an interesting idea.  sigaltstack could also check that the size
> > is at least that large, but then the question is how many sigaltstack
> > users check the error return value.
> 
> Probably not very many...
> 
> > However, based on what I saw in the kernel sources, it's not that they
> > have an exact upper bound in the sources or even at run time.  I think
> > the code simply uses space as it proceeds (at least on x86).  But
> > perhaps I misread it.
> 
> Yeesh.  I think that has got to get fixed, independent of everything else.
> 
> This is not really my area of expertise, but here's what comes to mind
> for a way forward:
> 
>  - Kernel-side signal delivery code is revamped so it knows an upper
> bound (perhaps not an exact one) on the space requirement for a signal
> frame, and exposes that to user space in a way that we can wrap up in
> a sysconf() query.
>  - Kernel-side signal delivery code is revamped so that it knows how
> much stack space is available (no matter where the stack came from --
> in the absence of other information I suppose it can use the bottom of
> the memory mapping, but for sigaltstack it ought to use the specified
> size) and, if there isn't enough space to write a complete signal
> frame, it terminates the process as-if by the default action of
> SIGSEGV instead of clobbering anything.
>  - We add glibc functions
> 
>     int alloc_sigstack(stack_t *ss, size_t scratch_needed);
>     void free_sigstack(stack_t *ss);
> 
> alloc_sigstack allocates space for a signal stack, guaranteeing to
> provide at least scratch_needed bytes of space for the signal
> handler's local variables and any functions it calls, plus an
> appropriate amount of overhead space for the signal frame.  The total
> allocation will be rounded up to a whole number of pages and will have
> no-access guard pages on either side of it.  It can fail.
> free_sigstack, naturally, deallocates the stack again.
> 
> The second change is what deals with people trying to call abort()
> from inside a signal handler when there might not be enough space for
> another signal frame.  It also should solve the AVX2 lack-of-space
> issue -- programs using the old constants may crash but they shouldn't
> behave unpredictably.
> 
> The point of alloc_sigstack and free_sigstack, over just telling
> people to use the new sysconf query, is that they give you guard
> pages, which will help with programs underestimating the amount of
> space they need.
> 
> Christian: I don't know if this stuff has been brought up on the
> kernel lists before, but it probably does need to be.

I'll bring it up soon(ish). There's a bunch of stuff that I have on my
(kernel) plate that I need to prioritize before I can free up memory for
that discussion. :)

Christian