Re: [PATCH 01/23] signal: Add an optional check for altstack size

From: Dave Hansen
Date: Fri Oct 22 2021 - 18:51:06 EST


Hi Eric,

> First the reason for the new locking is not really explained, it talks
> about serialization but it does not talk about what is protected.
> Especially given that the signal delivery code already has to check if
> the signal frame on the stack when pushing a new signal I don't
> understand what the code is trying to prevent.

Yeah, the basic idea is to ensure that there are no races between
do_sigaltstack() and enabling the new "dynamic features".

> Third the issues with modifying the userspace ABI are not discussed.
> Frankly that is a pretty big consideration. MINSIGSTKSZ is exported
> to userspace and userspace fundamentally needs to allocate the
> alternate signal frame.
Agreed. This is what we settled on to both respect old programs that
have MINSIGSTKSZ=2k compiled in *and* support new ones that need bigger
stacks.

> Forth the sigframe size on x86 is already dynamic and is already
> computed by get_sigframe_size.

Right. This is much more about making the altstack size checks dynamic
than the sigframe size.

> So can we please please please have a better description of what
> is going on and the trade offs that are being made.

I've got a suggested replacement changelog below. Please let me know if
it clarifies things, or leaves anything out.

How would this be a better subject?

signal: Add optional dynamic altstack size checks

And this for a changelog?

--

New x86 FPU features will be very large, requiring ~10k of stack in
signal handlers. These new features require a new approach called
"dynamic features".

The kernel currently tries to ensure that altstacks are reasonably
sized. Right now, on x86, sys_sigaltstack() requires a size of >=2k.
However, that 2k is a constant. Simply raising that 2k requirement to
>10k for the new features would break existing apps which have a
compiled-in size of 2k.

Instead of universally enforcing a larger stack, prohibit a process from
using dynamic features without properly-sized altstacks. This must be
enforced in two places:

* A dynamic feature can not be enabled without an large-enough altstack
for each process thread.
* Once a dynamic feature is enabled, any request to install a too-small
altstack will be rejected

The dynamic feature enabling code must examine each thread in a process
to ensure that the altstacks are large enough. Add a new lock
(sigaltstack_lock()) to ensure that threads can not race and change
their altstack after being examined.

Add the infrastructure in form of a config option and provide empty
stubs for architectures which do not need dynamic altstack size checks.

This implementation will be fleshed out for x86 in:

x86/arch_prctl: Add controls for dynamic XSTATE components