Bug 20305 - x86: MINSIGSTKSZ too small for AVX-512F support
Summary: x86: MINSIGSTKSZ too small for AVX-512F support
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.24
: P2 normal
Target Milestone: 2.34
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-28 11:32 UTC by Florian Weimer
Modified: 2022-01-06 14:31 UTC (History)
9 users (show)

See Also:
Host:
Target: x86_64, i686
Build:
Last reconfirmed:
fweimer: security-


Attachments
tst-minsigstksz.c (617 bytes, text/plain)
2016-06-28 11:32 UTC, Florian Weimer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Weimer 2016-06-28 11:32:46 UTC
Created attachment 9368 [details]
tst-minsigstksz.c

The attached test case fails on a system with AVX512 support (both hardware and kernel) with the following error:

tst-minsigstksz: changed byte 1376 bytes below configured stack
Comment 1 Florian Weimer 2016-06-28 11:35:59 UTC
As a possible fix, the kernel could refuse to save FPU state to the signal handler stack if the specified stack is not large enough.

I do not see a good way to fix this in glibc.  Increasing MINSIGSTKSZ has potential impact on ABI.
Comment 2 Florian Weimer 2016-08-22 15:02:39 UTC
I filed a kernel bug: https://bugzilla.kernel.org/show_bug.cgi?id=153531
Comment 3 Florian Weimer 2020-06-23 13:15:28 UTC
Not surprisingly, this issue happens with 32-bit glibc builds, too.
Comment 4 H.J. Lu 2020-09-25 22:51:48 UTC
MINSIGSTKSZ is defined in <bits/sigstack.h>.  Changing it in the kernel
doesn't help.  However, for AT_MINSIGSTKSZ kernel, we can do

extern int getminsigstacksize (void);

#ifdef __USE_GNU
/* Minumum stack size for a signal handler.  */
# undef MINSIGSTKSZ
# define MINSIGSTKSZ (getminsigstacksize ())
#endif
Comment 5 Andreas Schwab 2020-09-26 06:18:24 UTC
MINSIGSTKSZ is required to be a constant.
Comment 6 H.J. Lu 2020-09-26 12:40:34 UTC
(In reply to Andreas Schwab from comment #5)
> MINSIGSTKSZ is required to be a constant.

True.  But this requirement is very old at the time a fixed signal stack
size was sufficient. We can define MINSIGSTKSZ as a function return
under -D_GNU_SOURCE or other macro.
Comment 7 Bruno Haible 2021-03-06 19:22:52 UTC
"#define SIGSTKSZ sysconf (_SC_SIGSTKSZ)" is a bad idea for several reasons:

1) It's not POSIX compliant. POSIX:2018 https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html defines two lists of macros:

  * "The <signal.h> header shall define the following macros which shall expand to integer constant expressions that need not be usable in #if preprocessing directives:"

  * "The <signal.h> header shall also define the following symbolic constants:"

SIGSTKSZ is in the second list. This implies that it must expand to a constant
and that it must be usable in #if preprocessing directives.

2) Even if you evade reason 1 by activating the definition only if _GNU_SOURCE is defined, the problem is that such a definition breaks the majority of the uses of sigaltstack(). Most programs allocate the alternate stack either inside the stack of an existing thread or as a global array, making use of the POSIX assertion that it is a constant. For example: https://git.savannah.gnu.org/gitweb/?p=libsigsegv.git;a=blob;f=tests/altstack.h
A typical bug report where the change causes a compilation error is here: https://lists.gnu.org/archive/html/bug-m4/2021-03/msg00000.html

3) The alternate signal stack needs to be dimensioned according to the CPU and ABI that is in use. For example, SPARC processors tend to use much more stack space than x86 per function invocation. Similarly, 64-bit execution on a bi-arch CPU tends to use more stack space than 32-bit execution, because return addresses and other pointers are 64-bit vs. 32-bit large. But once you have fixed the CPU and the ABI, there is no ambiguity any more. If you have a processor which requires a smaller or a larger initial stack frame, depending on the "mode" of the processor, just take the maximum.
Comment 8 Andreas Schwab 2021-03-06 20:13:33 UTC
A symbolic constant need _not_ be usable in #if, see 3.380.
Comment 9 Bruno Haible 2021-03-06 20:19:31 UTC
(In reply to Andreas Schwab from comment #8)
> A symbolic constant need _not_ be usable in #if, see 3.380.

But you do agree that 'sysconf (_SC_SIGSTKSZ)' is not a symbolic constant?
Comment 10 Ernestas Kulik 2021-03-23 17:07:57 UTC
(In reply to Andreas Schwab from comment #8)
> A symbolic constant need _not_ be usable in #if, see 3.380.

I think we’re focusing on irrelevant details, because sysconf() is hardly a symbolic constant. Symbolic, maybe, but not constant.
Comment 11 Luis Machado 2021-09-03 13:11:27 UTC
Build failure possibly related to the refactoring of these constants.

https://sourceware.org/bugzilla/show_bug.cgi?id=28302
Comment 12 Carlos O'Donell 2022-01-06 14:31:19 UTC
This was fixed with this commit in glibc 2.34 (2021-08-02):

commit 28d07380c2ae5786e242be336ccc1c9e3111f3fa
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Jul 9 14:17:04 2021 -0700

    support: Replace MINSIGSTKSZ with sysconf (_SC_MINSIGSTKSZ)
    
    Replace MINSIGSTKSZ with sysconf (_SC_MINSIGSTKSZ) since the constant
    MINSIGSTKSZ used in glibc build may be too small.
    
    Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Where we now resolve MINSIGSTKSZ dynamically to use AT_MINSIGSTKSZ or an emulated value based on the hardware features.