On 5/18/21 10:34 AM, Bruno Haible wrote:
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. Conversely, even programs that assume SIGSTKSZ is a constant
(and allocate fixed-size arrays on the stack or in static storage) will
have problems on such platforms, no? Stack and static storage won't be
executable any more than heap storage is.
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.
- The declaration in sigsegv.h needs to stay, at least for the next
couple of years, because it is necessary for programs that use SIGSTKSZ
(such as GNU m4 and GNU clisp) to compile fine.
Why is it needed for GNU m4 and GNU clisp? I just looked at the latest
development source code for these programs, and neither mentions
SIGSTKSZ. GNU Emacs doesn't either (except in a configure.ac test that
does not assume SIGSTKSZ is a constant).
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.
And I propose the second attached patch to remove sigsegv.h's guarantee
that SIGSTKSZ is a constant, as glibc has gone a different way and it's
better if Gnulib tracks glibc.
From 97b0d8ad7078daafc1911ada2a53bf3d642a6cde Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Fri, 21 May 2021 14:41:42 -0700
Subject: [PATCH 1/2] =?UTF-8?q?sigsegv:=20don=E2=80=99t=20assume=20SIGSTKS?=
=?UTF-8?q?Z=20is=20a=20constant?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* m4/sigaltstack.m4 (SV_SIGALTSTACK): Don’t attempt to override
SIGSTKSZ. Instead, use an array that is plenty large, while
checking that it’s large enough. Also, be consistent about
putting that array in static storage rather than on the stack.
* tests/altstack-util.h (SIGSTKSZ): Don’t define.
(MYSTACK_SIZE): New macro, used consistently instead of SIGSTKSZ.
(mystack_storage, mystack): Now static.
(prepare_alternate_stack) [defined SIGSTKSZ]:
Check that MYSTACK_SIZE is large enough.
---
ChangeLog | 13 +++++++
m4/sigaltstack.m4 | 44 ++++++-----------------
tests/altstack-util.h | 18 ++++++----
tests/test-sigsegv-catch-stackoverflow1.c | 4 +--
tests/test-sigsegv-catch-stackoverflow2.c | 2 +-
5 files changed, 39 insertions(+), 42 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index fc67bc69a..50e376176 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2021-05-21 Paul Eggert <egg...@cs.ucla.edu>
+
+ sigsegv: don’t assume SIGSTKSZ is a constant
+ * m4/sigaltstack.m4 (SV_SIGALTSTACK): Don’t attempt to override
+ SIGSTKSZ. Instead, use an array that is plenty large, while
+ checking that it’s large enough. Also, be consistent about
+ putting that array in static storage rather than on the stack.
+ * tests/altstack-util.h (SIGSTKSZ): Don’t define.
+ (MYSTACK_SIZE): New macro, used instead of SIGSTKSZ.
+ (mystack_storage, mystack): Now static.
+ (prepare_alternate_stack) [SIGSTKSZ]:
+ Check that MYSTACK_SIZE is large enough.
+
2021-05-20 Paul Eggert <egg...@cs.ucla.edu>
fstatat: doc improvement
diff --git a/m4/sigaltstack.m4 b/m4/sigaltstack.m4
index 837191e5f..47e900079 100644
--- a/m4/sigaltstack.m4
+++ b/m4/sigaltstack.m4
@@ -1,4 +1,4 @@
-# sigaltstack.m4 serial 13
+# sigaltstack.m4 serial 14
dnl Copyright (C) 2002-2021 Bruno Haible <br...@clisp.org>
dnl Copyright (C) 2008 Eric Blake <e...@byu.net>
dnl This file is free software, distributed under the terms of the GNU
@@ -53,19 +53,6 @@ AC_DEFUN([SV_SIGALTSTACK],
# include <sys/time.h>
# include <sys/resource.h>
#endif
-/* In glibc >= 2.34, when _GNU_SOURCE is defined, SIGSTKSZ is no longer a
- compile-time constant. But we need a simple constant here. */
-#if __GLIBC__ >= 2
-# undef SIGSTKSZ
-# if defined __ia64__
-# define SIGSTKSZ 262144
-# else
-# define SIGSTKSZ 16384
-# endif
-#endif
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
void stackoverflow_handler (int sig)
{
/* If we get here, the stack overflow was caught. */
@@ -82,7 +69,7 @@ int recurse (volatile int n)
int sum = 0;
return *recurse_1 (n, &sum);
}
-char mystack[2 * SIGSTKSZ];
+char mystack[2 * (1 << 24)];
int main ()
{
stack_t altstack;
@@ -97,8 +84,12 @@ int main ()
#endif
/* Install the alternate stack. Use the midpoint of mystack, to guard
against a buggy interpretation of ss_sp on IRIX. */
- altstack.ss_sp = mystack + SIGSTKSZ;
- altstack.ss_size = SIGSTKSZ;
+#ifdef SIGSTKSZ
+ if (sizeof mystack / 2 < SIGSTKSZ)
+ exit (3);
+#endif
+ altstack.ss_sp = mystack + sizeof mystack / 2;
+ altstack.ss_size = sizeof mystack / 2;
altstack.ss_flags = 0; /* no SS_DISABLE */
if (sigaltstack (&altstack, NULL) < 0)
exit (1);
@@ -148,19 +139,6 @@ int main ()
#if HAVE_SYS_SIGNAL_H
# include <sys/signal.h>
#endif
-/* In glibc >= 2.34, when _GNU_SOURCE is defined, SIGSTKSZ is no longer a
- compile-time constant. But we need a simple constant here. */
-#if __GLIBC__ >= 2
-# undef SIGSTKSZ
-# if defined __ia64__
-# define SIGSTKSZ 262144
-# else
-# define SIGSTKSZ 16384
-# endif
-#endif
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
volatile char *stack_lower_bound;
volatile char *stack_upper_bound;
static void check_stack_location (volatile char *addr)
@@ -175,14 +153,14 @@ static void stackoverflow_handler (int sig)
char dummy;
check_stack_location (&dummy);
}
+char mystack[2 * (1 << 24)];
int main ()
{
- char mystack[2 * SIGSTKSZ];
stack_t altstack;
struct sigaction action;
/* Install the alternate stack. */
- altstack.ss_sp = mystack + SIGSTKSZ;
- altstack.ss_size = SIGSTKSZ;
+ altstack.ss_sp = mystack + sizeof mystack / 2;
+ altstack.ss_size = sizeof mystack / 2;
stack_lower_bound = (char *) altstack.ss_sp;
stack_upper_bound = (char *) altstack.ss_sp + altstack.ss_size - 1;
altstack.ss_flags = 0; /* no SS_DISABLE */
diff --git a/tests/altstack-util.h b/tests/altstack-util.h
index 513064506..f91072611 100644
--- a/tests/altstack-util.h
+++ b/tests/altstack-util.h
@@ -18,9 +18,7 @@
#include <stdint.h> /* uintptr_t */
#include <string.h> /* for memset */
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
+#define MYSTACK_SIZE (1 << 24)
/* glibc says: Users should use SIGSTKSZ as the size of user-supplied
buffers. We want to detect stack overflow of the alternate stack
@@ -29,12 +27,20 @@
an unaligned pointer, to ensure the alternate stack still ends up
aligned. */
#define MYSTACK_CRUMPLE_ZONE 8192
-char mystack_storage[SIGSTKSZ + 2 * MYSTACK_CRUMPLE_ZONE + 31];
-char *mystack; /* SIGSTKSZ bytes in the middle of storage. */
+static char mystack_storage[MYSTACK_SIZE + 2 * MYSTACK_CRUMPLE_ZONE + 31];
+static char *mystack; /* MYSTACK_SIZE bytes in the middle of storage. */
static void
prepare_alternate_stack (void)
{
+#ifdef SIGSTKSZ
+ if (MYSTACK_SIZE < SIGSTKSZ)
+ {
+ size_t size = SIGSTKSZ;
+ printf ("SIGSTKSZ=%zu exceeds MYSTACK_SIZE=%d\n", size, MYSTACK_SIZE);
+ exit (1);
+ }
+#endif
memset (mystack_storage, 's', sizeof mystack_storage);
mystack = (char *) ((uintptr_t) (mystack_storage + MYSTACK_CRUMPLE_ZONE) | 31);
}
@@ -51,7 +57,7 @@ check_alternate_stack_no_overflow (void)
exit (1);
}
for (i = MYSTACK_CRUMPLE_ZONE; i > 0; i--)
- if (*(mystack + SIGSTKSZ - 1 + i) != 's')
+ if (*(mystack + MYSTACK_SIZE - 1 + i) != 's')
{
printf ("Alternate stack was exceeded by %u bytes!!\n", i);
exit (1);
diff --git a/tests/test-sigsegv-catch-stackoverflow1.c b/tests/test-sigsegv-catch-stackoverflow1.c
index 141dfd44d..c828ed282 100644
--- a/tests/test-sigsegv-catch-stackoverflow1.c
+++ b/tests/test-sigsegv-catch-stackoverflow1.c
@@ -105,11 +105,11 @@ main ()
/* Install the stack overflow handler. */
if (stackoverflow_install_handler (&stackoverflow_handler,
- mystack, SIGSTKSZ)
+ mystack, MYSTACK_SIZE)
< 0)
exit (2);
stack_lower_bound = mystack;
- stack_upper_bound = mystack + SIGSTKSZ - 1;
+ stack_upper_bound = mystack + MYSTACK_SIZE - 1;
/* Save the current signal mask. */
sigemptyset (&emptyset);
diff --git a/tests/test-sigsegv-catch-stackoverflow2.c b/tests/test-sigsegv-catch-stackoverflow2.c
index 1db2d44fc..b94d1310b 100644
--- a/tests/test-sigsegv-catch-stackoverflow2.c
+++ b/tests/test-sigsegv-catch-stackoverflow2.c
@@ -129,7 +129,7 @@ main ()
/* Install the stack overflow handler. */
if (stackoverflow_install_handler (&stackoverflow_handler,
- mystack, SIGSTKSZ)
+ mystack, MYSTACK_SIZE)
< 0)
exit (2);
--
2.27.0
From 8f14518191893837d93bf723703e2ad207ee817e Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Fri, 21 May 2021 15:33:19 -0700
Subject: [PATCH 2/2] sigsegv: do not guarantee SIGSTKSZ is constant
glibc no longer guarantees that SIGSTKSZ is a constant,
and it appears that few programs still need it to be a constant.
* NEWS: Mention this.
* lib/sigsegv.in.h (SIGSTKSZ) [__GLIBC__ >= 2]: Do not redefine.
---
ChangeLog | 6 ++++++
NEWS | 3 +++
lib/sigsegv.in.h | 11 +----------
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 50e376176..843b2e267 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2021-05-21 Paul Eggert <egg...@cs.ucla.edu>
+ sigsegv: do not guarantee SIGSTKSZ is constant
+ glibc no longer guarantees that SIGSTKSZ is a constant,
+ and it appears that few programs still need it to be a constant.
+ * NEWS: Mention this.
+ * lib/sigsegv.in.h (SIGSTKSZ) [__GLIBC__ >= 2]: Do not redefine.
+
sigsegv: don’t assume SIGSTKSZ is a constant
* m4/sigaltstack.m4 (SV_SIGALTSTACK): Don’t attempt to override
SIGSTKSZ. Instead, use an array that is plenty large, while
diff --git a/NEWS b/NEWS
index 98931a345..e02ebf42e 100644
--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,9 @@ User visible incompatible changes
Date Modules Changes
+2021-05-21 sigsegv sigsegv.h no longer guarantees that SIGSTKSZ
+ is an integer constant expression.
+
2021-03-21 fatal-signal The function at_fatal_signal now returns an error
indicator.
diff --git a/lib/sigsegv.in.h b/lib/sigsegv.in.h
index e442f4dfa..e5fac651b 100644
--- a/lib/sigsegv.in.h
+++ b/lib/sigsegv.in.h
@@ -37,20 +37,11 @@
#endif
/* Correct the value of SIGSTKSZ on some systems.
- glibc >= 2.34: When _GNU_SOURCE is defined, SIGSTKSZ is no longer a
- compile-time constant. But most programs need a simple constant.
AIX 64-bit: original value 4096 is too small.
HP-UX: original value 8192 is too small.
Solaris 11/x86_64: original value 8192 is too small. */
+
#include <signal.h>
-#if __GLIBC__ >= 2
-# undef SIGSTKSZ
-# if defined __ia64__
-# define SIGSTKSZ 262144
-# else
-# define SIGSTKSZ 65536
-# endif
-#endif
#if defined _AIX && defined _ARCH_PPC64
# undef SIGSTKSZ
# define SIGSTKSZ 8192
--
2.27.0
