Hi Paul,
What you are doing now is to deconstruct
- GCC extensions, and
- POSIX.
1) Regarding GCC extensions:
> > malloc'ed memory is not executable. But GCC's trampolines (used by nested
> > functions) require an executable stack. When an object file has such nested
> > functions, the toolchain arranges for the executable to ask the OS to
> > allocate an executable stack. But that only has an effect on the primary
> > stack of each thread. It does not have an effect on an alternate stack
> > (AFAIK).
>
> That should be OK so long as stack-overflow handlers don't used nested
> functions.
Wait a moment. GCC's nested functions are a documented GCC feature [1] for
ca. 30 years. When "security" became an important topic and people demanded
non-executable stacks for web browsers, GCC and the binutils were extended
in such a way that
- on one hand, programs that use GCC nested functions (on most architectures)
got a bit set in the executable, which has the consequence of an
executable stack,
- on the other hand, programs that don't need this — such as web browsers —
can have a non-executable stack.
Now, you are arguing "let's ignore whether programs use nested functions".
As if that wasn't a documented way, in the GNU system, of writing programs.??
> > I wouldn't mind, if it's done properly:
> > - The problem with non-executable malloc'ed memory would need to be
> > solved.
>
> Or, we could just document that stack-overflow handers can't use
> trampolines. That is a reasonable restriction for most apps, I would think.
It adds complexity to the calling program. The developer has to check whether
their code uses nested functions.
2) Regarding POSIX:
> The only place I found that assumed that SIGSTKSZ is a constant, was in
> Gnulib test cases. I removed that assumption by installing the first
> attached patch.
By doing so, you moved the Gnulib code away from POSIX. In so many occasions,
you voted for making Gnulib closer to POSIX, and that is a significant
reason why Gnulib is successful.
Here,
- The current POSIX specifies a constant SIGSTKSZ,
- Eric Blake is proposing that POSIX drops the constant-ness, and that
SIGSTKSZ becomes a macro that merely expands to an expression. [2]
So, being close to POSIX would mean to use SIGSTKSZ for the allocation
of an alternate stack, most likely through malloc.
Whereas your patch removes all traces of SIGSTKSZ and hardcodes a constant
(1 << 24) in various places instead.
The point of your patch appears to be to proactively react to other systems
(FreeBSD, Solaris, etc.) to follow glibc's move and make SIGSTKSZ non-
constant. But then, still, it would have been more maintainable to write
/* SIGSTKSZ can no longer be guaranteed to be a constant. Until POSIX
clarifies it, use a large constant anyway. */
#undef SIGSTKSZ
#define SIGSTKSZ (1 << 24)
3) The approach you used now, to use a 16 MB space for sigaltstack(),
is not portable to multithreaded situations. For example, I have a test
program (attached) that I consider to add as a unit test to gnulib.
It uses stack space from the thread itself as alternate signal stack.
That is a perfectly natural thing to do, because
- the permissions of the alternate stack are automatically right,
- this alternate stack gets deallocated automatically when the thread
dies (no memory leak).
But allocating 16 MB on a thread's stack is not portable. On Solaris/SPARC,
the main thread's size is only 8 MB.
Really, my goals here are that the unit tests in Gnulib
- use POSIX the best they can,
- give a coding pattern that people can use in their applications,
be they single-threaded or multi-threaded, and interoperable with
whatever GCC extensions the programmer wants.
Bruno
[1] https://gcc.gnu.org/onlinedocs/gcc-11.1.0/gcc/Nested-Functions.html
[2] https://sourceware.org/pipermail/libc-alpha/2021-March/123553.html
/* Test whether sigaltstack works per thread. */
#include <errno.h>
#include <pthread.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/select.h>
#include <sys/time.h>
/* Correct the value of SIGSTKSZ on some systems.
AIX 64-bit: original value 4096 is too small.
HP-UX: original value 8192 is too small. */
#if defined _AIX && defined _ARCH_PPC64
# undef SIGSTKSZ
# define SIGSTKSZ 8192
#endif
#if defined __hpux || (defined __sun && (defined __x86_64__ || defined __amd64__))
# undef SIGSTKSZ
# define SIGSTKSZ 16384
#endif
static void*
thread2_func (void *ignored)
{
stack_t current;
stack_t alt_stack;
char mystack_storage[SIGSTKSZ + 31];
char *mystack = (char *)((uintptr_t) mystack_storage | 31);
fprintf (stderr, "thread2 running\n");
if (sigaltstack (NULL, ¤t) < 0)
{
perror ("thread2 sigaltstack");
exit (1);
}
fprintf (stderr, "thread2 sigaltstack: enabled=%s onstack=%s sp=0x%lx size=0x%lx\n",
(current.ss_flags & SS_DISABLE) ? "no" : "yes",
(current.ss_flags & SS_ONSTACK) ? "yes" : "no",
(unsigned long) current.ss_sp,
(unsigned long) current.ss_size);
if ((current.ss_flags & SS_DISABLE) == 0 && current.ss_size > 0)
{
fprintf (stderr, "thread2 sigaltstack already enabled! Per-thread not supported.\n");
exit (1);
}
alt_stack.ss_flags = 0;
alt_stack.ss_sp = mystack;
alt_stack.ss_size = SIGSTKSZ;
if (sigaltstack (&alt_stack, NULL) < 0)
{
perror ("thread2 sigaltstack set");
exit (1);
}
fprintf (stderr, "thread2 alternate stack installed\n");
if (sigaltstack (NULL, ¤t) < 0)
{
perror ("thread2 sigaltstack");
exit (1);
}
fprintf (stderr, "thread2 sigaltstack now: enabled=%s onstack=%s sp=0x%lx size=0x%lx\n",
(current.ss_flags & SS_DISABLE) ? "no" : "yes",
(current.ss_flags & SS_ONSTACK) ? "yes" : "no",
(unsigned long) current.ss_sp,
(unsigned long) current.ss_size);
return 0;
}
int
main ()
{
stack_t alt_stack;
stack_t current;
stack_t last;
pthread_t thread2;
struct timeval sleeptime;
char mystack_storage[SIGSTKSZ + 31];
char *mystack = (char *)((uintptr_t) mystack_storage | 31);
alt_stack.ss_flags = 0;
alt_stack.ss_sp = mystack;
alt_stack.ss_size = SIGSTKSZ;
if (sigaltstack (&alt_stack, NULL) < 0)
{
perror ("main-thread sigaltstack set");
exit (1);
}
fprintf (stderr, "alternate stack installed\n");
if (sigaltstack (NULL, ¤t) < 0)
{
perror ("main-thread sigaltstack");
exit (1);
}
fprintf (stderr, "main-thread sigaltstack: enabled=%s onstack=%s sp=0x%lx size=0x%lx\n",
(current.ss_flags & SS_DISABLE) ? "no" : "yes",
(current.ss_flags & SS_ONSTACK) ? "yes" : "no",
(unsigned long) current.ss_sp,
(unsigned long) current.ss_size);
last = current;
if (pthread_create (&thread2, NULL, thread2_func, NULL))
{
fprintf (stderr, "creating of thread2 failed\n");
exit (1);
}
/* Sleep 2 sec. */
sleeptime.tv_sec = 2;
sleeptime.tv_usec = 0;
if (select (0, NULL, NULL, NULL, &sleeptime) < 0)
{
perror ("main-thread sleep");
exit (1);
}
if (sigaltstack (NULL, ¤t) < 0)
{
perror ("main-thread sigaltstack");
exit (1);
}
fprintf (stderr, "main-thread sigaltstack: enabled=%s onstack=%s sp=0x%lx size=0x%lx\n",
(current.ss_flags & SS_DISABLE) ? "no" : "yes",
(current.ss_flags & SS_ONSTACK) ? "yes" : "no",
(unsigned long) current.ss_sp,
(unsigned long) current.ss_size);
if (!(current.ss_flags == last.ss_flags
&& current.ss_sp == last.ss_sp
&& current.ss_size == last.ss_size))
{
fprintf (stderr, "main-thread sigaltstack has changed!\n");
exit (1);
}
fprintf (stderr, "SUPPORTED\n");
exit (0);
}
/* Results:
Linux SUPPORTED
Hurd SUPPORTED although "enabled=yes onstack=no sp=0x0 size=0x0"
Mac OS X 10.5 SUPPORTED
FreeBSD 11 SUPPORTED
NetBSD 7 thread2 sigaltstack already enabled! Per-thread not supported.
NetBSD 8 SUPPORTED
OpenBSD 6.0 SUPPORTED
AIX 7.1 SUPPORTED
HP-UX 11.31 SUPPORTED
IRIX 6.5 thread2 sigaltstack already enabled! Per-thread not supported.
Solaris 9, 10 SUPPORTED
Haiku SUPPORTED
Cygwin SUPPORTED
*/
