>From 8ba9126d00bfe1ab77a5c820c58c68933d4df85c Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 20 Sep 2020 11:48:17 -0700 Subject: [PATCH 1/2] c-stack: improve checking if !libsigsegv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ChangeLog | 37 +++++++++++++ lib/c-stack.c | 135 ++++++++++++++++++++++++++++-------------------- m4/c-stack.m4 | 33 ++++++------ modules/c-stack | 12 +++-- 4 files changed, 139 insertions(+), 78 deletions(-) diff --git a/ChangeLog b/ChangeLog index cc1cd4a40..087b9232f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,40 @@ +2020-09-20 Paul Eggert + + 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 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 59606299b..3649c1bfe 100644 --- a/lib/c-stack.c +++ b/lib/c-stack.c @@ -35,30 +35,25 @@ #include +#include "c-stack.h" + #include "gettext.h" #define _(msgid) gettext (msgid) #include +#include #include #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 +#include #include #include -/* Posix 2001 declares ucontext_t in , Posix 200x in - . */ +/* Pre-2008 POSIX declared ucontext_t in ucontext.h instead of signal.h. */ #if HAVE_UCONTEXT_H # include #endif @@ -69,13 +64,26 @@ typedef struct sigaltstack stack_t; # include #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 +/* 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 c3e2bddd3..e98f80fff 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 @@ -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 ]]) - AC_CHECK_HEADERS_ONCE([unistd.h ucontext.h]) + AC_CHECK_HEADERS_ONCE([ucontext.h]) AC_CHECK_TYPES([stack_t], , , [#include ]) 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 ca2c208e1..8c898eb9e 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 -- 2.17.1