c-stack: improve checking if !libsigsegv

If SIGINFO_WORKS, do not treat a null pointer dereference as if it
were a stack overflow.  Use uintptr_t and INT_ADD_WRAPV to avoid
unlikely pointer overflow.  Also, fix some obsolete code and typos.
I found these problems while looking into this bug report:
https://lists.gnu.org/r/grep-devel/2020-09/msg00053.html
* lib/c-stack.c: Include c-stack.h first, to test interface.
Include inttypes.h for UINTPTR_MAX, stdbool.h, stddef.h for
max_align_t, intprops.h for INT_ADD_WRAPV.
(USE_LIBSIGSEGV): New macro; use it to simplify later code.
(SIGSTKSZ): Simplify setup.  Work around libsigsegv bug only
for libsigsegv 2.8 and earlier since the bug should be fixed
after that.
(alternate_signal_stack): Use max_align_t instead of doing it by hand.
(segv_handler, overflow_handler, segv_handler) [DEBUG]:
Assume sprintf returns byte count; this assumption is safe now.
(page_size): New static volatile variable, since sysconf isn’t
documented to be async-signal-safe on Solaris.  This variable is
present and used if (!USE_LIBSIGSEGV && HAVE_SIGALTSTACK &&
HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING &&
SIGINFO_WORKS).
(segv_handler): Use it if present.  Never report null pointer
dereference as a stack overflow.  Check for (unlikely) unsigned
and/or pointer overflow.
* m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC):
Rename cache variables to gl_cv_sys_stack_overflow_works
and gl_cv_sys_xsi_stack_overflow_heuristic.
All uses changed.
(gl_PREREQ_C_STACK): Do not require AC_FUNC_ALLOCA, since
c-stack no longer uses STACK_DIRECTION.
Do not check for unistd.h, since we depend on unistd.
Fix shell typo ‘$"ac_cv_sys_xsi_stack_overflow_heuristic"’.
* modules/c-stack (Depends-on): Sort.  Add intprops, inttypes,
stdbool, stddef.

4 files changed, 139 insertions, 78 deletions

diff --git a/ChangeLog b/ChangeLog
index cc1cd4a40a..087b9232fd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,40 @@
+2020-09-20  Paul Eggert  <eggert@cs.ucla.edu>
+
+	c-stack: improve checking if !libsigsegv
+	If SIGINFO_WORKS, do not treat a null pointer dereference as if it
+	were a stack overflow.  Use uintptr_t and INT_ADD_WRAPV to avoid
+	unlikely pointer overflow.  Also, fix some obsolete code and typos.
+	I found these problems while looking into this bug report:
+	https://lists.gnu.org/r/grep-devel/2020-09/msg00053.html
+	* lib/c-stack.c: Include c-stack.h first, to test interface.
+	Include inttypes.h for UINTPTR_MAX, stdbool.h, stddef.h for
+	max_align_t, intprops.h for INT_ADD_WRAPV.
+	(USE_LIBSIGSEGV): New macro; use it to simplify later code.
+	(SIGSTKSZ): Simplify setup.  Work around libsigsegv bug only
+	for libsigsegv 2.8 and earlier since the bug should be fixed
+	after that.
+	(alternate_signal_stack): Use max_align_t instead of doing it by hand.
+	(segv_handler, overflow_handler, segv_handler) [DEBUG]:
+	Assume sprintf returns byte count; this assumption is safe now.
+	(page_size): New static volatile variable, since sysconf isn’t
+	documented to be async-signal-safe on Solaris.  This variable is
+	present and used if (!USE_LIBSIGSEGV && HAVE_SIGALTSTACK &&
+	HAVE_DECL_SIGALTSTACK && HAVE_STACK_OVERFLOW_HANDLING &&
+	SIGINFO_WORKS).
+	(segv_handler): Use it if present.  Never report null pointer
+	dereference as a stack overflow.  Check for (unlikely) unsigned
+	and/or pointer overflow.
+	* m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC):
+	Rename cache variables to gl_cv_sys_stack_overflow_works
+	and gl_cv_sys_xsi_stack_overflow_heuristic.
+	All uses changed.
+	(gl_PREREQ_C_STACK): Do not require AC_FUNC_ALLOCA, since
+	c-stack no longer uses STACK_DIRECTION.
+	Do not check for unistd.h, since we depend on unistd.
+	Fix shell typo ‘$"ac_cv_sys_xsi_stack_overflow_heuristic"’.
+	* modules/c-stack (Depends-on): Sort.  Add intprops, inttypes,
+	stdbool, stddef.
+
 2020-09-20  Bruno Haible  <bruno@clisp.org>
 
 	Revert now-unnecessary override of config.guess on Alpine Linux 3.10.
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 59606299bb..3649c1bfe0 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -35,30 +35,25 @@
 
 #include <config.h>
 
+#include "c-stack.h"
+
 #include "gettext.h"
 #define _(msgid) gettext (msgid)
 
 #include <errno.h>
+#include <inttypes.h>
 
 #include <signal.h>
 #if ! HAVE_STACK_T && ! defined stack_t
 typedef struct sigaltstack stack_t;
 #endif
-#ifndef SIGSTKSZ
-# define SIGSTKSZ 16384
-#elif HAVE_LIBSIGSEGV && SIGSTKSZ < 16384
-/* libsigsegv 2.6 through 2.8 have a bug where some architectures use
-   more than the Linux default of an 8k alternate stack when deciding
-   if a fault was caused by stack overflow.  */
-# undef SIGSTKSZ
-# define SIGSTKSZ 16384
-#endif
 
+#include <stdbool.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
 
-/* Posix 2001 declares ucontext_t in <ucontext.h>, Posix 200x in
-   <signal.h>.  */
+/* Pre-2008 POSIX declared ucontext_t in ucontext.h instead of signal.h.  */
 #if HAVE_UCONTEXT_H
 # include <ucontext.h>
 #endif
@@ -69,13 +64,26 @@ typedef struct sigaltstack stack_t;
 # include <stdio.h>
 #endif
 
-#if HAVE_LIBSIGSEGV
+/* Use libsigsegv only if needed; kernels like Solaris can detect
+   stack overflow without the overhead of an external library.  */
+#define USE_LIBSIGSEGV (HAVE_XSI_STACK_OVERFLOW_HEURISTIC && HAVE_LIBSIGSEGV)
+
+#if USE_LIBSIGSEGV
 # include <sigsegv.h>
+/* libsigsegv 2.6 through 2.8 have a bug where some architectures use
+   more than the Linux default of an 8k alternate stack when deciding
+   if a fault was caused by stack overflow.  */
+# if LIBSIGSEGV_VERSION <= 0x0208 && SIGSTKSZ < 16384
+#  undef SIGSTKSZ
+# endif
+#endif
+#ifndef SIGSTKSZ
+# define SIGSTKSZ 16384
 #endif
 
-#include "c-stack.h"
 #include "exitfail.h"
 #include "ignore-value.h"
+#include "intprops.h"
 #include "getprogname.h"
 
 #if defined SA_ONSTACK && defined SA_SIGINFO
@@ -97,7 +105,7 @@ static _GL_ASYNC_SAFE void (* volatile segv_action) (int);
 static char const * volatile program_error_message;
 static char const * volatile stack_overflow_message;
 
-#if ((HAVE_LIBSIGSEGV && ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC) \
+#if (USE_LIBSIGSEGV                                           \
      || (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK            \
          && HAVE_STACK_OVERFLOW_HANDLING))
 
@@ -111,12 +119,12 @@ static _GL_ASYNC_SAFE _Noreturn void
 die (int signo)
 {
   char const *message;
-#if !SIGINFO_WORKS && !HAVE_LIBSIGSEGV
+# if !SIGINFO_WORKS && !USE_LIBSIGSEGV
   /* We can't easily determine whether it is a stack overflow; so
      assume that the rest of our program is perfect (!) and that
      this segmentation violation is a stack overflow.  */
   signo = 0;
-#endif /* !SIGINFO_WORKS && !HAVE_LIBSIGSEGV */
+# endif
   segv_action (signo);
   message = signo ? program_error_message : stack_overflow_message;
   ignore_value (write (STDERR_FILENO, progname, strlen (progname)));
@@ -128,22 +136,12 @@ die (int signo)
   raise (signo);
   abort ();
 }
-#endif
-
-#if (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK \
-     && HAVE_STACK_OVERFLOW_HANDLING) || HAVE_LIBSIGSEGV
 
 /* Storage for the alternate signal stack.  */
 static union
 {
   char buffer[SIGSTKSZ];
-
-  /* These other members are for proper alignment.  There's no
-     standard way to guarantee stack alignment, but this seems enough
-     in practice.  */
-  long double ld;
-  long l;
-  void *p;
+  max_align_t align;
 } alternate_signal_stack;
 
 static _GL_ASYNC_SAFE void
@@ -153,10 +151,7 @@ null_action (int signo _GL_UNUSED)
 
 #endif /* SIGALTSTACK || LIBSIGSEGV */
 
-/* Only use libsigsegv if we need it; platforms like Solaris can
-   detect stack overflow without the overhead of an external
-   library.  */
-#if HAVE_LIBSIGSEGV && ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC
+#if USE_LIBSIGSEGV
 
 /* Nonzero if general segv handler could not be installed.  */
 static volatile int segv_handler_missing;
@@ -171,8 +166,8 @@ segv_handler (void *address _GL_UNUSED, int serious)
   {
     char buf[1024];
     int saved_errno = errno;
-    sprintf (buf, "segv_handler serious=%d\n", serious);
-    ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
+    ignore_value (write (STDERR_FILENO, buf,
+                         sprintf (buf, "segv_handler serious=%d\n", serious)));
     errno = saved_errno;
   }
 # endif
@@ -193,9 +188,10 @@ overflow_handler (int emergency, stackoverflow_context_t context _GL_UNUSED)
 # if DEBUG
   {
     char buf[1024];
-    sprintf (buf, "overflow_handler emergency=%d segv_handler_missing=%d\n",
-             emergency, segv_handler_missing);
-    ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
+    ignore_value (write (STDERR_FILENO, buf,
+                         sprintf (buf, ("overflow_handler emergency=%d"
+                                        " segv_handler_missing=%d\n"),
+                                  emergency, segv_handler_missing)));
   }
 # endif
 
@@ -228,6 +224,8 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
 
 # if SIGINFO_WORKS
 
+static size_t volatile page_size;
+
 /* Handle a segmentation violation and exit.  This function is
    async-signal-safe.  */
 
@@ -235,8 +233,17 @@ static _GL_ASYNC_SAFE _Noreturn void
 segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED)
 {
   /* Clear SIGNO if it seems to have been a stack overflow.  */
-#  if ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC
-  /* We can't easily determine whether it is a stack overflow; so
+
+  /* If si_code is nonpositive, something like raise (SIGSEGV) occurred
+     so it cannot be a stack overflow.  */
+  bool cannot_be_stack_overflow = info->si_code <= 0;
+
+  /* An unaligned address cannot be a stack overflow.  */
+#  if FAULT_YIELDS_SIGBUS
+  cannot_be_stack_overflow |= signo == SIGBUS && info->si_code == BUS_ADRALN;
+#  endif
+
+  /* If we can't easily determine that it is not a stack overflow,
      assume that the rest of our program is perfect (!) and that
      this segmentation violation is a stack overflow.
 
@@ -246,33 +253,44 @@ segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED)
      Solaris populates uc_stack with the details of the
      interrupted stack, while Linux populates it with the details
      of the current stack.  */
-  signo = 0;
-#  else
-  if (0 < info->si_code)
+  if (!cannot_be_stack_overflow)
     {
       /* If the faulting address is within the stack, or within one
          page of the stack, assume that it is a stack overflow.  */
+      uintptr_t faulting_address = (uintptr_t) info->si_addr;
+
+      /* On all platforms we know of, the first page is not in the
+         stack to catch null pointer dereferening.  However, all other
+         pages might be in the stack.  */
+      void *stack_base = (void *) (uintptr_t) page_size;
+      uintptr_t stack_size = 0; stack_size -= page_size;
+#  if HAVE_XSI_STACK_OVERFLOW_HEURISTIC
+      /* Tighten the stack bounds via the XSI heuristic.  */
       ucontext_t const *user_context = context;
-      char const *stack_base = user_context->uc_stack.ss_sp;
-      size_t stack_size = user_context->uc_stack.ss_size;
-      char const *faulting_address = info->si_addr;
-      size_t page_size = sysconf (_SC_PAGESIZE);
-      size_t s = faulting_address - stack_base + page_size;
-      if (s < stack_size + 2 * page_size)
+      stack_base = user_context->uc_stack.ss_sp;
+      stack_size = user_context->uc_stack.ss_size;
+#  endif
+      uintptr_t base = (uintptr_t) stack_base,
+        lo = (INT_SUBTRACT_WRAPV (base, page_size, &lo) || lo < page_size
+              ? page_size : lo),
+        hi = ((INT_ADD_WRAPV (base, stack_size, &hi)
+               || INT_ADD_WRAPV (hi, page_size - 1, &hi))
+              ? UINTPTR_MAX : hi);
+      if (lo <= faulting_address && faulting_address <= hi)
         signo = 0;
 
 #   if DEBUG
       {
         char buf[1024];
-        sprintf (buf,
-                 "segv_handler fault=%p base=%p size=%lx page=%lx signo=%d\n",
-                 faulting_address, stack_base, (unsigned long) stack_size,
-                 (unsigned long) page_size, signo);
-        ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
+        ignore_value (write (STDERR_FILENO, buf,
+                             sprintf (buf,
+                                      ("segv_handler code=%d fault=%p base=%p"
+                                       " size=0x%zx page=0x%zx signo=%d\n"),
+                                      info->si_code, info->si_addr, stack_base,
+                                      stack_size, page_size, signo)));
       }
-#   endif
-    }
 #  endif
+    }
 
   die (signo);
 }
@@ -303,6 +321,10 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
   stack_overflow_message = _("stack overflow");
   progname = getprogname ();
 
+# if SIGINFO_WORKS
+  page_size = sysconf (_SC_PAGESIZE);
+# endif
+
   sigemptyset (&act.sa_mask);
 
 # if SIGINFO_WORKS
@@ -323,8 +345,9 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int))
   return sigaction (SIGSEGV, &act, NULL);
 }
 
-#else /* ! ((HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK
-             && HAVE_STACK_OVERFLOW_HANDLING) || HAVE_LIBSIGSEGV) */
+#else /* ! (USE_LIBSIGSEGV
+            || (HAVE_SIGALTSTACK && HAVE_DECL_SIGALTSTACK
+                && HAVE_STACK_OVERFLOW_HANDLING)) */
 
 int
 c_stack_action (_GL_ASYNC_SAFE void (*action) (int)  _GL_UNUSED)
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index c3e2bddd38..e98f80fffb 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -7,7 +7,7 @@
 
 # Written by Paul Eggert.
 
-# serial 17
+# serial 18
 
 AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
   [
@@ -34,7 +34,7 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
      [Define to 1 if an invalid memory address access may yield a SIGBUS.])
 
    AC_CACHE_CHECK([for working C stack overflow detection],
-     [ac_cv_sys_stack_overflow_works],
+     [gl_cv_sys_stack_overflow_works],
      [AC_RUN_IFELSE([AC_LANG_SOURCE(
            [[
             #include <unistd.h>
@@ -121,17 +121,17 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
               return recurse (0);
             }
            ]])],
-        [ac_cv_sys_stack_overflow_works=yes],
-        [ac_cv_sys_stack_overflow_works=no],
+        [gl_cv_sys_stack_overflow_works=yes],
+        [gl_cv_sys_stack_overflow_works=no],
         [case "$host_os" in
                    # Guess no on native Windows.
-           mingw*) ac_cv_sys_stack_overflow_works="guessing no" ;;
-           *)      ac_cv_sys_stack_overflow_works=cross-compiling ;;
+           mingw*) gl_cv_sys_stack_overflow_works="guessing no" ;;
+           *)      gl_cv_sys_stack_overflow_works=cross-compiling ;;
          esac
         ])
      ])
 
-  if test "$ac_cv_sys_stack_overflow_works" = yes; then
+  if test "$gl_cv_sys_stack_overflow_works" = yes; then
    AC_DEFINE([HAVE_STACK_OVERFLOW_HANDLING], [1],
      [Define to 1 if extending the stack slightly past the limit causes
       a SIGSEGV which can be handled on an alternate stack established
@@ -200,14 +200,14 @@ int main ()
     fi
 
    AC_CACHE_CHECK([for precise C stack overflow detection],
-     [ac_cv_sys_xsi_stack_overflow_heuristic],
+     [gl_cv_sys_xsi_stack_overflow_heuristic],
      [dnl On Linux/sparc64 (both in 32-bit and 64-bit mode), it would be wrong
       dnl to set HAVE_XSI_STACK_OVERFLOW_HEURISTIC to 1, because the third
       dnl argument passed to the segv_handler is a 'struct sigcontext *', not
       dnl an 'ucontext_t *'.  It would lead to a failure of test-c-stack2.sh.
       case "${host_os}--${host_cpu}" in
         linux*--sparc*)
-          ac_cv_sys_xsi_stack_overflow_heuristic=no
+          gl_cv_sys_xsi_stack_overflow_heuristic=no
           ;;
         *)
           AC_RUN_IFELSE(
@@ -329,14 +329,14 @@ int main ()
                   return recurse (0);
                 }
                ]])],
-            [ac_cv_sys_xsi_stack_overflow_heuristic=yes],
-            [ac_cv_sys_xsi_stack_overflow_heuristic=no],
-            [ac_cv_sys_xsi_stack_overflow_heuristic=cross-compiling])
+            [gl_cv_sys_xsi_stack_overflow_heuristic=yes],
+            [gl_cv_sys_xsi_stack_overflow_heuristic=no],
+            [gl_cv_sys_xsi_stack_overflow_heuristic=cross-compiling])
           ;;
       esac
      ])
 
-   if test $ac_cv_sys_xsi_stack_overflow_heuristic = yes; then
+   if test "$gl_cv_sys_xsi_stack_overflow_heuristic" = yes; then
      AC_DEFINE([HAVE_XSI_STACK_OVERFLOW_HEURISTIC], [1],
        [Define to 1 if extending the stack slightly past the limit causes
         a SIGSEGV, and an alternate stack can be established with sigaltstack,
@@ -353,19 +353,16 @@ AC_DEFUN([gl_PREREQ_C_STACK],
   [AC_REQUIRE([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC])
    AC_REQUIRE([gl_LIBSIGSEGV])
 
-   # for STACK_DIRECTION
-   AC_REQUIRE([AC_FUNC_ALLOCA])
-
    AC_CHECK_FUNCS_ONCE([sigaltstack])
    AC_CHECK_DECLS([sigaltstack], , , [[#include <signal.h>]])
 
-   AC_CHECK_HEADERS_ONCE([unistd.h ucontext.h])
+   AC_CHECK_HEADERS_ONCE([ucontext.h])
 
    AC_CHECK_TYPES([stack_t], , , [#include <signal.h>])
 
    dnl c-stack does not need -lsigsegv if the system has XSI heuristics.
    if test "$gl_cv_lib_sigsegv" = yes \
-       && test $"ac_cv_sys_xsi_stack_overflow_heuristic" != yes ; then
+       && test "$gl_cv_sys_xsi_stack_overflow_heuristic" != yes; then
      AC_SUBST([LIBCSTACK], [$LIBSIGSEGV])
      AC_SUBST([LTLIBCSTACK], [$LTLIBSIGSEGV])
    fi
diff --git a/modules/c-stack b/modules/c-stack
index ca2c208e1a..8c898eb9e9 100644
--- a/modules/c-stack
+++ b/modules/c-stack
@@ -7,15 +7,19 @@ lib/c-stack.c
 m4/c-stack.m4
 
 Depends-on:
-gettext-h
 errno
 exitfail
+getprogname
+gettext-h
 ignore-value
-unistd
+intprops
+inttypes
+libsigsegv
 raise
 sigaction
-libsigsegv
-getprogname
+stdbool
+stddef
+unistd
 
 configure.ac:
 gl_C_STACK