Re: c-stack and Irix - gnulib

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> Here's the minimal patch needed to CVS libsigsegv to expose the bug, as
> well as work around it for Irix 5.3.  I still need to port something like
> this for c-stack in the non-libsigsegv case.

Now that libsigsegv is cleaned up, here's what I'm applying to gnulib.  I've 
tested that on Irix 5.3, both with and without libsigsegv, a tarball of m4 
built today no longer fails any tests (but without libsigsegv, test-c-stack2.sh 
skips, since Irix does not provide the XSI hooks for detecting stack overflow 
from siginfo_t).

>From ad73682838ae0e66832aad97a8fbc5fb935130c6 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 23 Sep 2008 08:47:26 -0600
Subject: [PATCH] c-stack: work around Irix sigaltstack bug

* m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC): Check
whether sigaltstack uses wrong end of stack_t (copied in part from
libsigsegv).
* lib/c-stack.c (c_stack_action) [!HAVE_LIBSIGSEGV]: Work around
Irix bug, without requiring an over-allocation.
* doc/posix-functions/sigaltstack.texi (sigaltstack): Document the
bug.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                            |   13 ++++-
 doc/posix-functions/sigaltstack.texi |    8 +++-
 lib/c-stack.c                        |    7 +++
 m4/c-stack.m4                        |   94 ++++++++++++++++++++++++++++-----
 4 files changed, 104 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 38dd15e..1dbbb60 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2008-09-23  Eric Blake  <address@hidden>
 
+       c-stack: work around Irix sigaltstack bug
+       * m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC): Check
+       whether sigaltstack uses wrong end of stack_t (copied in part from
+       libsigsegv).
+       * lib/c-stack.c (c_stack_action) [!HAVE_LIBSIGSEGV]: Work around
+       Irix bug, without requiring an over-allocation.
+       * doc/posix-functions/sigaltstack.texi (sigaltstack): Document the
+       bug.
+
        fopen: document mingw bug on directories
        * doc/posix-functions/fopen.texi (fopen): Mention mingw bug for
        not allowing a stream visiting a directory, even though reading
diff --git a/doc/posix-functions/sigaltstack.texi b/doc/posix-
functions/sigaltstack.texi
index 53d91c7..e318ebe 100644
--- a/doc/posix-functions/sigaltstack.texi
+++ b/doc/posix-functions/sigaltstack.texi
@@ -16,5 +16,11 @@ Portability problems not fixed by Gnulib:
 This function is missing on some platforms:
 Cygwin, mingw, Interix 3.5, BeOS.
 @item
address@hidden doesn't work on HP-UX 11/IA-64 and OpenBSD 3.6/Sparc64.
address@hidden doesn't work on HP-UX 11/IA-64 and OpenBSD
+3.6/Sparc64.
address@hidden
+This function interprets the @code{ss_sp} member of @code{stack_t} as
+the upper bound instead of the lower bound of the alternate stack on
+some platforms:
+Irix 6.5
 @end itemize
diff --git a/lib/c-stack.c b/lib/c-stack.c
index 6802665..11cb9ef 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -291,8 +291,15 @@ c_stack_action (void (*action) (int))
   stack_t st;
   struct sigaction act;
   st.ss_flags = 0;
+# if SIGALTSTACK_SS_REVERSED
+  /* Irix mistakenly treats ss_sp as the upper bound, rather than
+     lower bound, of the alternate stack.  */
+  st.ss_sp = alternate_signal_stack.buffer + SIGSTKSZ - sizeof (void *);
+  st.ss_size = sizeof alternate_signal_stack.buffer - sizeof (void *);
+# else
   st.ss_sp = alternate_signal_stack.buffer;
   st.ss_size = sizeof alternate_signal_stack.buffer;
+# endif
   r = sigaltstack (&st, NULL);
   if (r != 0)
     return r;
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index 4f0300b..f1bda7b 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -7,7 +7,7 @@
 
 # Written by Paul Eggert.
 
-# serial 6
+# serial 7
 
 AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
   [# for STACK_DIRECTION
@@ -26,13 +26,13 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
         # include <sys/time.h>
         # include <sys/resource.h>
         #endif
-         #ifndef SIGSTKSZ
-         # define SIGSTKSZ 16384
-         #endif
+        #ifndef SIGSTKSZ
+        # define SIGSTKSZ 16384
+        #endif
 
         static union
         {
-          char buffer[SIGSTKSZ];
+          char buffer[2 * SIGSTKSZ];
           long double ld;
           long u;
           void *p;
@@ -52,8 +52,9 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
           int r;
 
           st.ss_flags = 0;
-          st.ss_sp = alternate_signal_stack.buffer;
-          st.ss_size = sizeof alternate_signal_stack.buffer;
+          /* Use the midpoint to avoid Irix sigaltstack bug.  */
+          st.ss_sp = alternate_signal_stack.buffer + SIGSTKSZ;
+          st.ss_size = SIGSTKSZ;
           r = sigaltstack (&st, 0);
           if (r != 0)
             return r;
@@ -98,6 +99,68 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
       a SIGSEGV which can be handled on an alternate stack established
       with sigaltstack.])
 
+    dnl The ss_sp field of a stack_t is, according to POSIX, the lowest address
+    dnl of the memory block designated as an alternate stack. But IRIX 5.3
+    dnl interprets it as the highest address!
+    AC_CACHE_CHECK([for correct stack_t interpretation],
+      [gl_cv_sigaltstack_low_base], [
+      AC_RUN_IFELSE([
+       AC_LANG_SOURCE([[
+#include <stdlib.h>
+#include <signal.h>
+#if HAVE_SYS_SIGNAL_H
+# include <sys/signal.h>
+#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)
+{
+  if (addr >= stack_lower_bound && addr <= stack_upper_bound)
+    exit (0);
+  else
+    exit (1);
+}
+static void stackoverflow_handler (int sig)
+{
+  char dummy;
+  check_stack_location (&dummy);
+}
+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;
+  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 */
+  if (sigaltstack (&altstack, NULL) < 0)
+    exit (2);
+  /* Install the SIGSEGV handler.  */
+  sigemptyset (&action.sa_mask);
+  action.sa_handler = &stackoverflow_handler;
+  action.sa_flags = SA_ONSTACK;
+  if (sigaction (SIGSEGV, &action, (struct sigaction *) NULL) < 0)
+    exit(3);
+  /* Provoke a SIGSEGV.  */
+  raise (SIGSEGV);
+  exit (3);
+}]])],
+      [gl_cv_sigaltstack_low_base=yes],
+      [gl_cv_sigaltstack_low_base=no],
+      [gl_cv_sigaltstack_low_base=cross-compiling])])
+   if test "$gl_cv_sigaltstack_low_base" = no; then
+      AC_DEFINE([SIGALTSTACK_SS_REVERSED], [1],
+       [Define if sigaltstack() interprets the stack_t.ss_sp field
+        incorrectly, as the highest address of the alternate stack range
+        rather than as the lowest address.])
+    fi
+
    AC_CACHE_CHECK([for precise C stack overflow detection],
      ac_cv_sys_xsi_stack_overflow_heuristic,
      [AC_TRY_RUN(
@@ -112,13 +175,13 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
         # include <sys/time.h>
         # include <sys/resource.h>
         #endif
-         #ifndef SIGSTKSZ
-         # define SIGSTKSZ 16384
-         #endif
+        #ifndef SIGSTKSZ
+        # define SIGSTKSZ 16384
+        #endif
 
         static union
         {
-          char buffer[SIGSTKSZ];
+          char buffer[2 * SIGSTKSZ];
           long double ld;
           long u;
           void *p;
@@ -167,8 +230,9 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
           int r;
 
           st.ss_flags = 0;
-          st.ss_sp = alternate_signal_stack.buffer;
-          st.ss_size = sizeof alternate_signal_stack.buffer;
+          /* Use the midpoint to avoid Irix sigaltstack bug.  */
+          st.ss_sp = alternate_signal_stack.buffer + SIGSTKSZ;
+          st.ss_size = SIGSTKSZ;
           r = sigaltstack (&st, 0);
           if (r != 0)
             return r;
-- 
1.6.0.2


>From 91a109c0426e8b8cd8b317e29c8a91bd99757f0a Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 23 Sep 2008 08:56:10 -0600
Subject: [PATCH] c-stack: avoid compiler optimizations when provoking overflow

* m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC): Make
recursion harder to optimize, to ensure a stack overflow occurs.
* tests/test-c-stack.c (recurse): Likewise.
Borrowed from libsigsegv.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog            |    6 ++++++
 m4/c-stack.m4        |   36 ++++++++++++++++++++++--------------
 tests/test-c-stack.c |   22 +++++++++++++++-------
 3 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1dbbb60..8586143 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2008-09-23  Eric Blake  <address@hidden>
 
+       c-stack: avoid compiler optimizations when provoking overflow
+       * m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC): Make
+       recursion harder to optimize, to ensure a stack overflow occurs.
+       * tests/test-c-stack.c (recurse): Likewise.
+       Borrowed from libsigsegv.
+
        c-stack: work around Irix sigaltstack bug
        * m4/c-stack.m4 (AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC): Check
        whether sigaltstack uses wrong end of stack_t (copied in part from
diff --git a/m4/c-stack.m4 b/m4/c-stack.m4
index f1bda7b..5069333 100644
--- a/m4/c-stack.m4
+++ b/m4/c-stack.m4
@@ -64,15 +64,19 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
           act.sa_handler = segv_handler;
           return sigaction (SIGSEGV, &act, 0);
         }
-
+        static volatile int *
+        recurse_1 (volatile int n, volatile int *p)
+        {
+          if (n >= 0)
+            *recurse_1 (n + 1, p) += n;
+          return p;
+        }
         static int
-        recurse (char *p)
+        recurse (volatile int n)
         {
-          char array[500];
-          array[0] = 1;
-          return *p + recurse (array);
+          int sum = 0;
+          return *recurse_1 (n, &sum);
         }
-
         int
         main ()
         {
@@ -86,7 +90,7 @@ AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
           setrlimit (RLIMIT_STACK, &rl);
           #endif
 
-          return c_stack_action () || recurse ("\1");
+          return c_stack_action () || recurse (0);
         }
        ],
        [ac_cv_sys_stack_overflow_works=yes],
@@ -242,15 +246,19 @@ int main ()
           act.sa_sigaction = segv_handler;
           return sigaction (SIGSEGV, &act, 0);
         }
-
+        static volatile int *
+        recurse_1 (volatile int n, volatile int *p)
+        {
+          if (n >= 0)
+            *recurse_1 (n + 1, p) += n;
+          return p;
+        }
         static int
-        recurse (char *p)
+        recurse (volatile int n)
         {
-          char array[500];
-          array[0] = 1;
-          return *p + recurse (array);
+          int sum = 0;
+          return *recurse_1 (n, &sum);
         }
-
         int
         main ()
         {
@@ -264,7 +272,7 @@ int main ()
           setrlimit (RLIMIT_STACK, &rl);
           #endif
 
-          return c_stack_action () || recurse ("\1");
+          return c_stack_action () || recurse (0);
         }
        ],
        [ac_cv_sys_xsi_stack_overflow_heuristic=yes],
diff --git a/tests/test-c-stack.c b/tests/test-c-stack.c
index fe782c9..96ab152 100644
--- a/tests/test-c-stack.c
+++ b/tests/test-c-stack.c
@@ -41,15 +41,22 @@
     }                                                                       \
   while (0)
 
-static long
-recurse (char *p)
+char *program_name;
+
+static volatile int *
+recurse_1 (volatile int n, volatile int *p)
 {
-  char array[500];
-  array[0] = 1;
-  return *p + recurse (array);
+  if (n >= 0)
+    *recurse_1 (n + 1, p) += n;
+  return p;
 }
 
-char *program_name;
+static int
+recurse (volatile int n)
+{
+  int sum = 0;
+  return *recurse_1 (n, &sum);
+}
 
 int
 main (int argc, char **argv)
@@ -72,8 +79,9 @@ main (int argc, char **argv)
          exit_failure = 77;
          ++*argv[argc]; /* Intentionally dereference NULL.  */
        }
-      return recurse ("\1");
+      return recurse (0);
     }
+  fputs ("skipping test: ", stderr);
   perror ("c_stack_action");
   return 77;
 }
-- 
1.6.0.2