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