* Re: new snapshot available: grep-3.4-almost.26-5419 [not found] ` <994fd316-0420-4b94-a1de-fea7d891c4ac@gmail.com> @ 2020-09-18 22:16 ` Paul Eggert 2020-09-18 22:42 ` Bruce Dubbs 0 siblings, 1 reply; 16+ messages in thread From: Paul Eggert @ 2020-09-18 22:16 UTC (permalink / raw) To: Bruce Dubbs, Jim Meyering; +Cc: Gnulib bugs, GNU grep developers [-- Attachment #1: Type: text/plain, Size: 2318 bytes --] In <https://lists.gnu.org/r/grep-devel/2020-09/msg00052.html> on 9/18/20 2:00 PM, Bruce Dubbs wrote: > ... if I run ./configure --prefix=/usr --bindir=/bin grep wants to link with > /usr/lib/libsigsegv.so /usr/lib/libc.a instead of just -lsigsegv > In this later case, grep segfaults early. > > If I remove /usr/lib/libc.a, everything is fine. I do not know why configure > wants to use a static libc if the destination libraries are specified. > > Is this possibly a gnulib issue? Yes, most likely it's induced by grep's use of Gnulib's libsigsegv module. I'll cc. this email to the Gnulib mailing list. To help narrow this down, let's try to reproduce the problem in Gnulib, without having grep be involved. I created the attached tarball from Gnulib master with the commands: ./gnulib-tool --create-testdir --dir c-stack-test c-stack tar czf c-stack-test.tgz c-stack-test Please run the following commands on your end and see whether they cause a similar failure: tar xf c-stack-test.tgz cd c-stack-test ./configure --prefix=/usr --bindir=/bin make check I ran that on my Ubuntu 18.04.5 x86-64 platform and got output that contained this: ... checking for working C stack overflow detection... yes checking for correct stack_t interpretation... yes checking for precise C stack overflow detection... no checking for ld used by gcc... /usr/bin/ld checking if the linker (/usr/bin/ld) is GNU ld... yes checking for shared library run path origin... done checking 32-bit host C ABI... no checking for ELF binary format... yes checking for the common suffixes of directories in the library search path... lib,lib,lib checking for libsigsegv... yes checking how to link with libsigsegv... -lsigsegv ... and all 93 tests succeeded. Here are a few more things I tried and where I got the expected results; please compare them to your platform too. $ ldd gltests/test-c-stack linux-vdso.so.1 (0x00007ffcf55e9000) libsigsegv.so.2 => /usr/lib/x86_64-linux-gnu/libsigsegv.so.2 (0x00007f28136f9000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f2813308000) /lib64/ld-linux-x86-64.so.2 (0x00007f2813b04000) $ gltests/test-c-stack test-c-stack: stack overflow $ gltests/test-c-stack 1 test-c-stack: program error Aborted (core dumped) [-- Attachment #2: c-stack-test.tgz --] [-- Type: application/x-compressed-tar, Size: 836712 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: new snapshot available: grep-3.4-almost.26-5419 2020-09-18 22:16 ` new snapshot available: grep-3.4-almost.26-5419 Paul Eggert @ 2020-09-18 22:42 ` Bruce Dubbs 2020-09-18 23:24 ` libsigsegv on LinuxFromScratch Bruno Haible 0 siblings, 1 reply; 16+ messages in thread From: Bruce Dubbs @ 2020-09-18 22:42 UTC (permalink / raw) To: Paul Eggert, Jim Meyering; +Cc: Gnulib bugs, GNU grep developers On 9/18/20 5:16 PM, Paul Eggert wrote: > In <https://lists.gnu.org/r/grep-devel/2020-09/msg00052.html> on 9/18/20 > 2:00 PM, Bruce Dubbs wrote: >> ... if I run ./configure --prefix=/usr --bindir=/bin grep wants to >> link with /usr/lib/libsigsegv.so /usr/lib/libc.a instead of just >> -lsigsegv >> In this later case, grep segfaults early. >> >> If I remove /usr/lib/libc.a, everything is fine. I do not know why >> configure wants to use a static libc if the destination libraries are >> specified. >> >> Is this possibly a gnulib issue? > > Yes, most likely it's induced by grep's use of Gnulib's libsigsegv > module. I'll cc. this email to the Gnulib mailing list. > > To help narrow this down, let's try to reproduce the problem in Gnulib, > without having grep be involved. I created the attached tarball from > Gnulib master with the commands: > > ./gnulib-tool --create-testdir --dir c-stack-test c-stack > tar czf c-stack-test.tgz c-stack-test > > Please run the following commands on your end and see whether they cause > a similar failure: > > tar xf c-stack-test.tgz > cd c-stack-test > ./configure --prefix=/usr --bindir=/bin > make check > > I ran that on my Ubuntu 18.04.5 x86-64 platform and got output that > contained this: > > ... > checking for working C stack overflow detection... yes > checking for correct stack_t interpretation... yes > checking for precise C stack overflow detection... no > checking for ld used by gcc... /usr/bin/ld > checking if the linker (/usr/bin/ld) is GNU ld... yes > checking for shared library run path origin... done > checking 32-bit host C ABI... no > checking for ELF binary format... yes > checking for the common suffixes of directories in the library search > path... lib,lib,lib > checking for libsigsegv... yes > checking how to link with libsigsegv... -lsigsegv I get: checking for libsigsegv... yes checking how to link with libsigsegv... /usr/lib/libsigsegv.so /usr/lib/libc.a Do you have /usr/lib/libc.a ? I'll note that we do 'strip --strip-debug' on libc.a > and all 93 tests succeeded. dummy 0: ./test-suite.log =============================== # TOTAL: 93 # PASS: 91 # SKIP: 0 # XFAIL: 0 # FAIL: 2 # XPASS: 0 # ERROR: 0 .. contents:: :depth: 2 FAIL: test-c-stack.sh ===================== ./test-c-stack.sh: line 7: 18000 Segmentation fault ${CHECKER} ./test-c-stack${EXEEXT} 2> t-c-stack.tmp FAIL test-c-stack.sh (exit status: 1) FAIL: test-c-stack2.sh ====================== FAIL test-c-stack2.sh (exit status: 1) > Here are a few more things I tried and > where I got the expected results; please compare them to your platform too. > > $ ldd gltests/test-c-stack > linux-vdso.so.1 (0x00007ffcf55e9000) > libsigsegv.so.2 => /usr/lib/x86_64-linux-gnu/libsigsegv.so.2 > (0x00007f28136f9000) > libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f2813308000) > /lib64/ld-linux-x86-64.so.2 (0x00007f2813b04000) $ ldd gltests/test-c-stack linux-vdso.so.1 (0x00007ffd62100000) libsigsegv.so.2 => /usr/lib/libsigsegv.so.2 (0x00007f3d0f843000) libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007f3d0f829000) libc.so.6 => /lib/libc.so.6 (0x00007f3d0f663000) /lib64/ld-linux-x86-64.so.2 (0x00007f3d0f86f000 > $ gltests/test-c-stack > test-c-stack: stack overflow $ gltests/test-c-stack Segmentation fault > $ gltests/test-c-stack 1 > test-c-stack: program error > Aborted (core dumped) Segmentation fault -- Bruce ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libsigsegv on LinuxFromScratch 2020-09-18 22:42 ` Bruce Dubbs @ 2020-09-18 23:24 ` Bruno Haible 2020-09-18 23:47 ` Bruce Dubbs 0 siblings, 1 reply; 16+ messages in thread From: Bruno Haible @ 2020-09-18 23:24 UTC (permalink / raw) To: bug-gnulib; +Cc: GNU grep developers, Paul Eggert, Bruce Dubbs, Jim Meyering Bruce Dubbs wrote in <https://lists.gnu.org/r/grep-devel/2020-09/msg00052.html> <https://lists.gnu.org/r/grep-devel/2020-09/msg00053.html> : > > checking for libsigsegv... yes > > checking how to link with libsigsegv... -lsigsegv > > I get: > > checking for libsigsegv... yes > checking how to link with libsigsegv... /usr/lib/libsigsegv.so > /usr/lib/libc.a Is this really a multiline string (also in the value of LIBSIGSEGV in config.status)? Or are these space-separated values? In the latter case, is linking with libc.a normally working? Also, if there is a file /usr/lib/libsigsegv.la, can you please show its contents? Bruno ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libsigsegv on LinuxFromScratch 2020-09-18 23:24 ` libsigsegv on LinuxFromScratch Bruno Haible @ 2020-09-18 23:47 ` Bruce Dubbs 2020-09-19 8:06 ` Bruce Dubbs 0 siblings, 1 reply; 16+ messages in thread From: Bruce Dubbs @ 2020-09-18 23:47 UTC (permalink / raw) To: Bruno Haible, bug-gnulib; +Cc: Paul Eggert, Jim Meyering, GNU grep developers On 9/18/20 6:24 PM, Bruno Haible wrote: > Bruce Dubbs wrote in > <https://lists.gnu.org/r/grep-devel/2020-09/msg00052.html> > <https://lists.gnu.org/r/grep-devel/2020-09/msg00053.html> : > >>> checking for libsigsegv... yes >>> checking how to link with libsigsegv... -lsigsegv >> >> I get: >> >> checking for libsigsegv... yes >> checking how to link with libsigsegv... /usr/lib/libsigsegv.so >> /usr/lib/libc.a > > Is this really a multiline string (also in the value of LIBSIGSEGV in > .sconfigtatus)? Or are these space-separated values? It's email wrapping. It is one line in the log. $ grep LIBSIGSEGV config.status S["LIBSIGSEGV_PREFIX"]="/usr" S["LTLIBSIGSEGV"]="-L/usr/lib -lsigsegv -lc" S["LIBSIGSEGV"]="/usr/lib/libsigsegv.so -lc" S["HAVE_LIBSIGSEGV"]="yes" D["HAVE_LIBSIGSEGV"]=" 1" D["HAVE_LIBSIGSEGV"]=" 1" > In the latter case, is linking with libc.a normally working? No it is not. > Also, if there is a file /usr/lib/libsigsegv.la, can you please show > its contents? # libsigsegv.la - a libtool library file # Generated by libtool (GNU libtool) 2.4.6 # # Please DO NOT delete this file! # It is necessary for linking the library. # The name that we can dlopen(3). dlname='libsigsegv.so.2' # Names of this library. library_names='libsigsegv.so.2.0.5 libsigsegv.so.2 libsigsegv.so' # The name of the static archive. old_library='' # Linker flags that cannot go in dependency_libs. inherited_linker_flags='' # Libraries that this one depends upon. dependency_libs=' -lc' # Names of additional weak libraries provided by this library weak_library_names='' # Version information for libsigsegv. current=2 age=0 revision=5 # Is this an already installed library? installed=yes # Should we warn about portability when linking against -modules? shouldnotlink=no # Files to dlopen/dlpreopen dlopen='' dlpreopen='' # Directory that this library needs to be installed in: libdir='/usr/lib' ---------- But I think the problem is libc.a, not libsigsegv. One additional note: adding --prefix=/usr to configure causes the problem. If I only add --bindir=/bin, then the problem does not occur. -- Bruce ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libsigsegv on LinuxFromScratch 2020-09-18 23:47 ` Bruce Dubbs @ 2020-09-19 8:06 ` Bruce Dubbs 2020-09-19 23:47 ` Bruno Haible 0 siblings, 1 reply; 16+ messages in thread From: Bruce Dubbs @ 2020-09-19 8:06 UTC (permalink / raw) To: Bruno Haible, bug-gnulib; +Cc: Paul Eggert, Jim Meyering, GNU grep developers On 9/18/20 6:47 PM, Bruce Dubbs wrote: > On 9/18/20 6:24 PM, Bruno Haible wrote: >> Bruce Dubbs wrote in >> <https://lists.gnu.org/r/grep-devel/2020-09/msg00052.html> >> <https://lists.gnu.org/r/grep-devel/2020-09/msg00053.html> : >> >>>> checking for libsigsegv... yes >>>> checking how to link with libsigsegv... -lsigsegv >>> >>> I get: >>> >>> checking for libsigsegv... yes >>> checking how to link with libsigsegv... /usr/lib/libsigsegv.so >>> /usr/lib/libc.a Here is some background. When we build grep on LFS, we normally do not have either pcre or libsegsev installed. It has built fine for years. We have had no complaints. In the latest grep-3.4-almost.31-fe6c I have been testing after both pcre and libsigsevv have been. I haven't been installing the new grep versions, only going through make check. Here is what I found. If ./configure is run without options, the build and checks are fine. If ./configure is run with --prefix=/usr with both libsigsegv and /usr/lib/libc.a are present, configure gives: configure:10063: checking for libsigsegv configure:10086: gcc -o conftest -g -O2 conftest.c /usr/lib/libsigsegv.so /usr/lib/libc.a >&5 configure:10086: $? = 0 configure:10097: result: yes configure:10104: checking how to link with libsigsegv configure:10106: result: /usr/lib/libsigsegv.so /usr/lib/libc.a Which causes grep to segfault immediately. If ./configure is run with --prefix=/usr AND --without-libsigsegv-prefix the result is: configure:10063: checking for libsigsegv configure:10086: gcc -o conftest -g -O2 conftest.c -lsigsegv >&5 configure:10086: $? = 0 configure:10097: result: yes configure:10104: checking how to link with libsigsegv configure:10106: result: -lsigsegv And the results of the checks are as expected. If I move all libtool .la files (I really dislike libtool) out of the way, then things are fine again with just --prefix=/usr: configure:10063: checking for libsigsegv configure:10086: gcc -o conftest -g -O2 conftest.c /usr/lib/libsigsegv.so >&5 configure:10086: $? = 0 configure:10097: result: yes configure:10104: checking how to link with libsigsegv configure:10106: result: /usr/lib/libsigsegv.so So the bottom line is there is something interacting between libtool and teh default libsigsegv procedures, however none of the .la files references libc.a directly. I hope this helps. -- Bruce ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libsigsegv on LinuxFromScratch 2020-09-19 8:06 ` Bruce Dubbs @ 2020-09-19 23:47 ` Bruno Haible 2020-09-20 1:16 ` Bruce Dubbs 2020-09-20 23:15 ` Paul Eggert 0 siblings, 2 replies; 16+ messages in thread From: Bruno Haible @ 2020-09-19 23:47 UTC (permalink / raw) To: Bruce Dubbs; +Cc: Paul Eggert, bug-gnulib, Jim Meyering, GNU grep developers Thanks Bruce for the details about the .la file. > If ./configure is run with --prefix=/usr with both libsigsegv and > /usr/lib/libc.a are present, configure gives: > > configure:10063: checking for libsigsegv > configure:10086: gcc -o conftest -g -O2 conftest.c > /usr/lib/libsigsegv.so /usr/lib/libc.a >&5 > configure:10086: $? = 0 > configure:10097: result: yes > configure:10104: checking how to link with libsigsegv > configure:10106: result: /usr/lib/libsigsegv.so /usr/lib/libc.a > > Which causes grep to segfault immediately. Whereas on an Ubuntu 16.04 system the effect is weird in a different sense: the compilation fails with an error /usr/bin/ld: dynamic STT_GNU_IFUNC symbol `strcmp' with pointer equality in `/usr/lib/libc.a(strcmp.o)' can not be used when making an executable; recompile with -fPIE and relink with -pie leading to checking for libsigsegv... no > If I move all libtool .la files (I really dislike libtool) out of the > way The .la file contains a comment # Please DO NOT delete this file! # It is necessary for linking the library. but in fact all distros I know of don't distribute .la files. Go figure. The patch below should fix it. 2020-09-19 Bruno Haible <bruno@clisp.org> havelib: Avoid linking with libc.a on GNU systems. Reported by Bruce Dubbs <bruce.dubbs@gmail.com> in <https://lists.gnu.org/archive/html/grep-devel/2020-09/msg00052.html>. * m4/lib-link.m4 (AC_LIB_LINKFLAGS_BODY): When processing the dependency_libs value of a .la file, ignore '-lc' options on GNU systems. diff --git a/m4/lib-link.m4 b/m4/lib-link.m4 index eecf70e..5ce1aa2 100644 --- a/m4/lib-link.m4 +++ b/m4/lib-link.m4 @@ -1,4 +1,4 @@ -# lib-link.m4 serial 31 +# lib-link.m4 serial 32 dnl Copyright (C) 2001-2020 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -631,7 +631,20 @@ AC_DEFUN([AC_LIB_LINKFLAGS_BODY], ;; -l*) dnl Handle this in the next round. - names_next_round="$names_next_round "`echo "X$dep" | sed -e 's/^X-l//'` + dnl But on GNU systems, ignore -lc options, because + dnl - linking with libc is the default anyway, + dnl - linking with libc.a may produce an error + dnl "/usr/bin/ld: dynamic STT_GNU_IFUNC symbol `strcmp' with pointer equality in `/usr/lib/libc.a(strcmp.o)' can not be used when making an executable; recompile with -fPIE and relink with -pie" + dnl or may produce an executable that always crashes, see + dnl <https://lists.gnu.org/archive/html/grep-devel/2020-09/msg00052.html>. + dep=`echo "X$dep" | sed -e 's/^X-l//'` + if test "X$dep" != Xc \ + || case $host_os in + linux* | gnu* | k*bsd*-gnu) false ;; + *) true ;; + esac; then + names_next_round="$names_next_round $dep" + fi ;; *.la) dnl Handle this in the next round. Throw away the .la's ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: libsigsegv on LinuxFromScratch 2020-09-19 23:47 ` Bruno Haible @ 2020-09-20 1:16 ` Bruce Dubbs 2020-09-20 23:15 ` Paul Eggert 1 sibling, 0 replies; 16+ messages in thread From: Bruce Dubbs @ 2020-09-20 1:16 UTC (permalink / raw) To: Bruno Haible; +Cc: Paul Eggert, bug-gnulib, Jim Meyering, GNU grep developers On 9/19/20 6:47 PM, Bruno Haible wrote: > Thanks Bruce for the details about the .la file. > >> If ./configure is run with --prefix=/usr with both libsigsegv and >> /usr/lib/libc.a are present, configure gives: >> >> configure:10063: checking for libsigsegv >> configure:10086: gcc -o conftest -g -O2 conftest.c >> /usr/lib/libsigsegv.so /usr/lib/libc.a >&5 >> configure:10086: $? = 0 >> configure:10097: result: yes >> configure:10104: checking how to link with libsigsegv >> configure:10106: result: /usr/lib/libsigsegv.so /usr/lib/libc.a >> >> Which causes grep to segfault immediately. > > Whereas on an Ubuntu 16.04 system the effect is weird in a different sense: > the compilation fails with an error > /usr/bin/ld: dynamic STT_GNU_IFUNC symbol `strcmp' with pointer equality in `/usr/lib/libc.a(strcmp.o)' can not be used when making an executable; recompile with -fPIE and relink with -pie > leading to > checking for libsigsegv... no > >> If I move all libtool .la files (I really dislike libtool) out of the >> way > > The .la file contains a comment > > # Please DO NOT delete this file! > # It is necessary for linking the library. > > but in fact all distros I know of don't distribute .la files. Go figure. > > The patch below should fix it. > > > 2020-09-19 Bruno Haible <bruno@clisp.org> > > havelib: Avoid linking with libc.a on GNU systems. > Reported by Bruce Dubbs <bruce.dubbs@gmail.com> in > <https://lists.gnu.org/archive/html/grep-devel/2020-09/msg00052.html>. > * m4/lib-link.m4 (AC_LIB_LINKFLAGS_BODY): When processing the > dependency_libs value of a .la file, ignore '-lc' options on GNU > systems. > > diff --git a/m4/lib-link.m4 b/m4/lib-link.m4 > index eecf70e..5ce1aa2 100644 > --- a/m4/lib-link.m4 > +++ b/m4/lib-link.m4 > @@ -1,4 +1,4 @@ > -# lib-link.m4 serial 31 > +# lib-link.m4 serial 32 > dnl Copyright (C) 2001-2020 Free Software Foundation, Inc. > dnl This file is free software; the Free Software Foundation > dnl gives unlimited permission to copy and/or distribute it, > @@ -631,7 +631,20 @@ AC_DEFUN([AC_LIB_LINKFLAGS_BODY], > ;; > -l*) > dnl Handle this in the next round. > - names_next_round="$names_next_round "`echo "X$dep" | sed -e 's/^X-l//'` > + dnl But on GNU systems, ignore -lc options, because > + dnl - linking with libc is the default anyway, > + dnl - linking with libc.a may produce an error > + dnl "/usr/bin/ld: dynamic STT_GNU_IFUNC symbol `strcmp' with pointer equality in `/usr/lib/libc.a(strcmp.o)' can not be used when making an executable; recompile with -fPIE and relink with -pie" > + dnl or may produce an executable that always crashes, see > + dnl <https://lists.gnu.org/archive/html/grep-devel/2020-09/msg00052.html>. > + dep=`echo "X$dep" | sed -e 's/^X-l//'` > + if test "X$dep" != Xc \ > + || case $host_os in > + linux* | gnu* | k*bsd*-gnu) false ;; > + *) true ;; > + esac; then > + names_next_round="$names_next_round $dep" > + fi > ;; > *.la) > dnl Handle this in the next round. Throw away the .la's > Wow. I'm glad you found where to fix the problem. Thanks. I know some systems need libtool, but Linux tools can find what they need without it. In some cases .la file are embedded in pkgconfig files and removing the .la files sometimes needs fixup. Just part of the joys of building from source. -- Bruce ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: libsigsegv on LinuxFromScratch 2020-09-19 23:47 ` Bruno Haible 2020-09-20 1:16 ` Bruce Dubbs @ 2020-09-20 23:15 ` Paul Eggert 2020-09-23 0:58 ` stack bounds Bruno Haible 1 sibling, 1 reply; 16+ messages in thread From: Paul Eggert @ 2020-09-20 23:15 UTC (permalink / raw) To: Bruno Haible; +Cc: GNU grep developers, bug-gnulib, Bruce Dubbs, Jim Meyering [-- Attachment #1: Type: text/plain, Size: 1697 bytes --] On 9/19/20 4:47 PM, Bruno Haible wrote: > havelib: Avoid linking with libc.a on GNU systems. Thanks for fixing the bug. This caused me to look at the c-stack module for the first time in a while, and I found some old-fashioned code and some unlikely bugs and fixed one misfeature when libsigsegv is not in use. I installed the attached patches to the c-stack module in Gnulib to try to fix it. These changes shouldn't affect how c-stack behaves when libsigsegv is in use. While looking into this I discovered pthread_getattr_np + pthread_attr_getstack which might have been nice for the GNU/Linux part of c-stack.c, except they're not async-signal-safe. As I understand it, libsigsegv works around the async-signal-safe problem by parsing /proc/self/maps with async-signal-safe functions, which is quite a feat and is probably beyond what c-stack should do. PS. I also found this circa-2015 Linux kernel bug related to PIE that looks like it might be of interest to the libsigsegv developers https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000253 This bug causes /proc/self/maps to report the wrong VMA (actually, overlapping VMAs) for the stack. This could be worth a comment in the libsigsegv sources. For more commentary in this area please see: https://stackoverflow.com/questions/56893353/analyzing-memory-mapping-of-a-process-with-pmap-stack/56920770 PPS. Given the longstanding security problems with stack overflow (as witness the name stackoverflow.com!) it is somewhat disturbing that there is still no reliable way in GNU/Linux to answer the simple question "Where's my stack?" or to detect and recover from stack overflow reliably. What's up with that? [-- Attachment #2: 0001-c-stack-improve-checking-if-libsigsegv.patch --] [-- Type: text/x-patch, Size: 18794 bytes --] From 8ba9126d00bfe1ab77a5c820c58c68933d4df85c Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> 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 <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 59606299b..3649c1bfe 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 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 <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 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 [-- Attachment #3: 0002-c-stack-output-diagnostic-in-single-write.patch --] [-- Type: text/x-patch, Size: 4292 bytes --] From 649a39bbd0641c8bb48ba25e5d62d03f8f36125f Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Sun, 20 Sep 2020 12:52:15 -0700 Subject: [PATCH 2/2] c-stack: output diagnostic in single 'write' * lib/c-stack.c (die): In the typical case, use just one 'write' syscall to output the diagnostic, as this lessens interleaving. (die, c_stack_action): Assume C99. * modules/c-stack (Depends-on): Add c99, mempcpy. --- ChangeLog | 6 ++++++ lib/c-stack.c | 39 ++++++++++++++++++++++++++++++--------- modules/c-stack | 2 ++ 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index 087b9232f..a43c32eb8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2020-09-20 Paul Eggert <eggert@cs.ucla.edu> + c-stack: output diagnostic in single 'write' + * lib/c-stack.c (die): In the typical case, use just one 'write' + syscall to output the diagnostic, as this lessens interleaving. + (die, c_stack_action): Assume C99. + * modules/c-stack (Depends-on): Add c99, mempcpy. + 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 diff --git a/lib/c-stack.c b/lib/c-stack.c index 3649c1bfe..742eb023e 100644 --- a/lib/c-stack.c +++ b/lib/c-stack.c @@ -118,7 +118,6 @@ static char const * volatile progname; static _GL_ASYNC_SAFE _Noreturn void die (int signo) { - char const *message; # 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 @@ -126,11 +125,34 @@ die (int signo) signo = 0; # endif segv_action (signo); - message = signo ? program_error_message : stack_overflow_message; - ignore_value (write (STDERR_FILENO, progname, strlen (progname))); - ignore_value (write (STDERR_FILENO, ": ", 2)); - ignore_value (write (STDERR_FILENO, message, strlen (message))); - ignore_value (write (STDERR_FILENO, "\n", 1)); + char const *message = signo ? program_error_message : stack_overflow_message; + + /* If the message is short, write it all at once to avoid + interleaving with other messages. Avoid writev as it is not + documented to be async-signal-safe. */ + size_t prognamelen = strlen (progname); + size_t messagelen = strlen (message); + static char const separator[] = {':', ' '}; + char buf[SIGSTKSZ / 16 + sizeof separator]; + ptrdiff_t buflen; + if (prognamelen + messagelen < sizeof buf - sizeof separator) + { + char *p = mempcpy (buf, progname, prognamelen); + p = mempcpy (p, separator, sizeof separator); + p = mempcpy (p, message, messagelen); + *p++ = '\n'; + buflen = p - buf; + } + else + { + ignore_value (write (STDERR_FILENO, progname, prognamelen)); + ignore_value (write (STDERR_FILENO, separator, sizeof separator)); + ignore_value (write (STDERR_FILENO, message, messagelen)); + buf[0] = '\n'; + buflen = 1; + } + ignore_value (write (STDERR_FILENO, buf, buflen)); + if (! signo) _exit (exit_failure); raise (signo); @@ -299,9 +321,7 @@ segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED) int c_stack_action (_GL_ASYNC_SAFE void (*action) (int)) { - int r; 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 @@ -312,7 +332,7 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int)) st.ss_sp = alternate_signal_stack.buffer; st.ss_size = sizeof alternate_signal_stack.buffer; # endif - r = sigaltstack (&st, NULL); + int r = sigaltstack (&st, NULL); if (r != 0) return r; @@ -325,6 +345,7 @@ c_stack_action (_GL_ASYNC_SAFE void (*action) (int)) page_size = sysconf (_SC_PAGESIZE); # endif + struct sigaction act; sigemptyset (&act.sa_mask); # if SIGINFO_WORKS diff --git a/modules/c-stack b/modules/c-stack index 8c898eb9e..5ddf6e6f5 100644 --- a/modules/c-stack +++ b/modules/c-stack @@ -7,6 +7,7 @@ lib/c-stack.c m4/c-stack.m4 Depends-on: +c99 errno exitfail getprogname @@ -15,6 +16,7 @@ ignore-value intprops inttypes libsigsegv +mempcpy raise sigaction stdbool -- 2.17.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* stack bounds 2020-09-20 23:15 ` Paul Eggert @ 2020-09-23 0:58 ` Bruno Haible 2020-09-23 1:28 ` Paul Eggert 0 siblings, 1 reply; 16+ messages in thread From: Bruno Haible @ 2020-09-23 0:58 UTC (permalink / raw) To: bug-gnulib; +Cc: Paul Eggert Hi Paul, > While looking into this I discovered pthread_getattr_np + pthread_attr_getstack > which might have been nice for the GNU/Linux part of c-stack.c, except they're > not async-signal-safe. As I understand it, libsigsegv works around the > async-signal-safe problem by parsing /proc/self/maps with async-signal-safe > functions, which is quite a feat and is probably beyond what c-stack should do. Yes, I agree: reading /proc/self/maps without calling malloc() is quite a bunch of code. > PS. I also found this circa-2015 Linux kernel bug related to PIE that looks like > it might be of interest to the libsigsegv developers > > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000253 > > This bug causes /proc/self/maps to report the wrong VMA (actually, overlapping > VMAs) for the stack. This could be worth a comment in the libsigsegv sources. This is hardly a danger in practice: It applies to executables with a data segment of size 128 MB or more. emacs' data segment size is 13 MB, and the one of dockerd is 21 MB. > Given the longstanding security problems with stack overflow (as witness > the name stackoverflow.com!) it is somewhat disturbing that there is still no > reliable way in GNU/Linux to answer the simple question "Where's my stack?" or > to detect and recover from stack overflow reliably. What's up with that? It sounds what you want is a function that returns the stack bounds, in such a way that every recursion step in a recursive function call the code can ask "am I close to the stack bound? do I need to stop recursing?" Such a thing cannot exist. The reason is that the OS allows or disallows further growth of the stack according to multiple criteria: - ulimit stack - ulimit memory - ulimit virtual memory - avoid collision with existing mmap()ed areas. The algorithm that makes this decision (whether more stack can be allocated or not) is undocumented and subject to change. And it changed in Linux, a couple of years ago. Also, this algorithm may treat the primary thread differently than other threads. So, really, the only thing you can do is to do a sigaltstack() for preparation, and wait for the signal when the OS decides to no longer allow stack growth. It's like when you write to a file on disk. On old operating systems like DOS you could determine the maximum file size by looking at the free space on the disk. This no longer works, because the decision whether a file can grow is done by looking at - the free space, - the space occupied by the metadata for the current file, - quota. Here too, your best bet is to let the OS do its decision and react when it reports a write failure. It would be technically possible to export the function that determines the stack bounds (libsigsegv/src/stackvma-*). But it's pointless to do so, because the stack bound is not the only factor considered by the OS. Bruno ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: stack bounds 2020-09-23 0:58 ` stack bounds Bruno Haible @ 2020-09-23 1:28 ` Paul Eggert 2020-10-10 12:08 ` Bruno Haible 0 siblings, 1 reply; 16+ messages in thread From: Paul Eggert @ 2020-09-23 1:28 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib On 9/22/20 5:58 PM, Bruno Haible wrote: > It sounds what you want is a function that returns the stack bounds, in such a > way that every recursion step in a recursive function call the code can ask > "am I close to the stack bound? do I need to stop recursing?" That shouldn't be necessary, except for functions where GCC can't prove that their activation records are small (and these exceptions should be quite rare at runtime). I am thinking of some combination of gcc -fstack-check and/or -fstack-clash-protection and/or related ideas (not that I've looked into all the details). That is, I'm expecting help from the hardware, the kernel, and from GCC. Stack-exhaustion checking should "just work". I realize it's a nontrivial ask. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: stack bounds 2020-09-23 1:28 ` Paul Eggert @ 2020-10-10 12:08 ` Bruno Haible 2020-10-10 20:10 ` Paul Eggert 0 siblings, 1 reply; 16+ messages in thread From: Bruno Haible @ 2020-10-10 12:08 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib Hi Paul, you wrote on 2020-09-22: > I am thinking of some combination of gcc -fstack-check and/or > -fstack-clash-protection and/or related ideas (not that I've looked into all the > details). That is, I'm expecting help from the hardware, the kernel, and from > GCC. Stack-exhaustion checking should "just work". I realize it's a nontrivial ask. gcc -fstack-clash-protection does exactly what most people expect; as you say, with the help from the hardware, the kernel, and GCC. - The hardware provides the MMU and the concept of page tables, which are the basis of mprotect(). - The kernel provides mprotect; organizes a guard page at the bottom of the stack; and grows the stack if an access to the guard page is made. - 'gcc -fstack-clash-protection' arranges that a stack frame that is larger than one page is accessed in single-page steps, from top to bottom, so that the kernel will grow the stack and not report a SIGSEGV. Test code below. This is how it's done on Windows. On Linux, the kernel allows the stack to grow by any amount, if it does not become closer than 1 MB to another VMA and does not violate the set limits. See linux/mm/mmap.c:expand_downwards and linux/mm/mmap.c:acct_stack_growth. Therefore on Linux, there is no need for a guard page and no need for 'gcc -fstack-clash-protection'. In both cases, it "just works". What other features around stack bounds checking are you thinking of? gcc -fstack-check is unrelated: It merely adds a canary word in each stack frame and checks that this word is still present upon return. So, it's merely a defense against buffer overflows on the stack. Bruno ========================= foo.c ===================== void bar (int *array); void foo (int n) { int array[n]; for (int i = 0; i < n; i++) array[i] = i*i; bar (array); } ===================================================== gcc -fstack-clash-protection -O2 -S foo.c ========================= foo.s ===================== .file "foo.c" .text .p2align 4 .globl foo .type foo, @function foo: .LFB0: .cfi_startproc pushq %rbp .cfi_def_cfa_offset 16 .cfi_offset 6, -16 movslq %edi, %rcx leaq 15(,%rcx,4), %rax movq %rcx, %rdx movq %rax, %rsi andq $-4096, %rax movq %rsp, %rdi movq %rsp, %rbp .cfi_def_cfa_register 6 andq $-16, %rsi subq %rax, %rdi cmpq %rdi, %rsp je .L3 .L13: subq $4096, %rsp orq $0, 4088(%rsp) cmpq %rdi, %rsp jne .L13 .L3: andl $4095, %esi subq %rsi, %rsp testq %rsi, %rsi jne .L14 .L4: movq %rsp, %rdi testl %edx, %edx jle .L5 xorl %eax, %eax .p2align 4,,10 .p2align 3 .L6: movl %eax, %edx imull %eax, %edx movl %edx, (%rdi,%rax,4) addq $1, %rax cmpq %rcx, %rax jne .L6 .L5: call bar leave .cfi_remember_state .cfi_def_cfa 7, 8 ret .p2align 4,,10 .p2align 3 .L14: .cfi_restore_state orq $0, -8(%rsp,%rsi) jmp .L4 .cfi_endproc .LFE0: .size foo, .-foo .ident "GCC: (GNU) 10.2.0" .section .note.GNU-stack,"",@progbits ===================================================== ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: stack bounds 2020-10-10 12:08 ` Bruno Haible @ 2020-10-10 20:10 ` Paul Eggert 2020-10-10 21:49 ` Bruno Haible 0 siblings, 1 reply; 16+ messages in thread From: Paul Eggert @ 2020-10-10 20:10 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib [-- Attachment #1: Type: text/plain, Size: 860 bytes --] On 10/10/20 5:08 AM, Bruno Haible wrote: > On Linux, the kernel allows the stack to grow by any amount, if it does not > become closer than 1 MB to another VMA and does not violate the set limits. > See linux/mm/mmap.c:expand_downwards and linux/mm/mmap.c:acct_stack_growth. > Therefore on Linux, there is no need for a guard page and no need for > 'gcc -fstack-clash-protection'. There's still a need, if a function declares a large local variable, as the stack pointer can jump around the 1 MB barrier and trash other storage. If I compile the attached program with 'gcc -m32 -O2 stackish.c' on Fedora 31 x86-64, the program exits with status 255 (instead of crashing with a stack overflow as it should), because the stack has overflowed and has stomped on the heap. So stack overflow checking is not "just working", at least for this particular case. [-- Attachment #2: stackish.c --] [-- Type: text/x-csrc, Size: 447 bytes --] #include <stdlib.h> #include <string.h> int growby (int bsize, int argc) { char b[bsize]; for (int i = 0; i <= argc + 256; i++) b[i] = i; return b[argc] - b[argc + 256]; } int main (int argc, char **argv) { int psize = argc + 1024 * 1024 * 1024; char *p = calloc (psize, 1); int bsize = (char *) &p - (p + psize/2); int status = growby (bsize, argc); for (int i = 0; i < psize; i++) status |= p[i]++; return status; } ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: stack bounds 2020-10-10 20:10 ` Paul Eggert @ 2020-10-10 21:49 ` Bruno Haible 2020-10-11 18:49 ` Paul Eggert 0 siblings, 1 reply; 16+ messages in thread From: Bruno Haible @ 2020-10-10 21:49 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib [-- Attachment #1: Type: text/plain, Size: 1692 bytes --] Paul Eggert wrote: > > On Linux, the kernel allows the stack to grow by any amount, if it does not > > become closer than 1 MB to another VMA and does not violate the set limits. > > See linux/mm/mmap.c:expand_downwards and linux/mm/mmap.c:acct_stack_growth. > > Therefore on Linux, there is no need for a guard page and no need for > > 'gcc -fstack-clash-protection'. > > There's still a need, if a function declares a large local variable, as the > stack pointer can jump around the 1 MB barrier and trash other storage. If I > compile the attached program with 'gcc -m32 -O2 stackish.c' on Fedora 31 x86-64, > the program exits with status 255 (instead of crashing with a stack overflow as > it should), because the stack has overflowed and has stomped on the heap. So > stack overflow checking is not "just working", at least for this particular case. Oh, I see: your program is not getting near the heap with the stack, it is getting directly *into* the heap (because it fills the bottom of array 'b' without having filled the rest of 'b' first). gcc -fstack-clash-protection -m32 -O2 stackish.c fixes this issue. So, you want 'gcc -fstack-clash-protection' [1] to become enabled by default? Some distros are doing this already: - Ubuntu 20.04 [2] (also -fstack-clash-protection is part of the default gcc flags for users), - RHEL 8 [1] (but apparently not by default for user-compiled programs), and the Firefox people are considering it [3]. Bruno [1] https://developers.redhat.com/blog/2020/05/22/stack-clash-mitigation-in-gcc-part-3/ [2] https://lists.ubuntu.com/archives/ubuntu-devel/2019-June/040741.html [3] https://bugzilla.mozilla.org/show_bug.cgi?id=1588710 [-- Attachment #2: stackish.s.ubuntu --] [-- Type: text/plain, Size: 3300 bytes --] .file "stackish.c" .text .p2align 4 .globl growby .type growby, @function growby: .LFB27: .cfi_startproc endbr64 pushq %rbp .cfi_def_cfa_offset 16 .cfi_offset 6, -16 movslq %edi, %rdi addq $15, %rdi movq %rsp, %rbp .cfi_def_cfa_register 6 subq $16, %rsp movq %fs:40, %rax movq %rax, -8(%rbp) xorl %eax, %eax movq %rsp, %rdx movq %rdi, %rax andq $-4096, %rdi subq %rdi, %rdx andq $-16, %rax cmpq %rdx, %rsp je .L3 .L14: subq $4096, %rsp orq $0, 4088(%rsp) cmpq %rdx, %rsp jne .L14 .L3: andl $4095, %eax subq %rax, %rsp testq %rax, %rax jne .L15 .L4: movl %esi, %r8d movq %rsp, %rcx addl $256, %r8d js .L5 movl %r8d, %edi xorl %eax, %eax .p2align 4,,10 .p2align 3 .L6: movq %rax, %rdx movb %al, (%rcx,%rax) addq $1, %rax cmpq %rdx, %rdi jne .L6 .L5: movslq %esi, %rsi movslq %r8d, %r8 movsbl (%rcx,%rsi), %eax movsbl (%rcx,%r8), %edx subl %edx, %eax movq -8(%rbp), %rsi xorq %fs:40, %rsi jne .L16 leave .cfi_remember_state .cfi_def_cfa 7, 8 ret .p2align 4,,10 .p2align 3 .L15: .cfi_restore_state orq $0, -8(%rsp,%rax) jmp .L4 .L16: call __stack_chk_fail@PLT .cfi_endproc .LFE27: .size growby, .-growby .section .text.startup,"ax",@progbits .p2align 4 .globl main .type main, @function main: .LFB28: .cfi_startproc endbr64 pushq %rbp .cfi_def_cfa_offset 16 .cfi_offset 6, -16 movl $1, %esi movq %rsp, %rbp .cfi_def_cfa_register 6 pushq %r12 .cfi_offset 12, -24 leal 1073741824(%rdi), %r12d pushq %rbx .cfi_offset 3, -32 movl %edi, %ebx movslq %r12d, %rdi subq $16, %rsp movq %fs:40, %rax movq %rax, -24(%rbp) xorl %eax, %eax call calloc@PLT movl %r12d, %edx movq %rsp, %rdi shrl $31, %edx movq %rax, %rcx movq %rax, -32(%rbp) leaq -32(%rbp), %rax addl %r12d, %edx sarl %edx movslq %edx, %rdx addq %rcx, %rdx movq %rsp, %rcx subq %rdx, %rax cltq addq $15, %rax movq %rax, %rdx andq $-4096, %rax subq %rax, %rcx andq $-16, %rdx movq %rcx, %rax cmpq %rax, %rsp je .L19 .L32: subq $4096, %rsp orq $0, 4088(%rsp) cmpq %rax, %rsp jne .L32 .L19: andl $4095, %edx subq %rdx, %rsp testq %rdx, %rdx jne .L33 .L20: movl %ebx, %r8d movq %rsp, %rcx addl $256, %r8d js .L21 movl %r8d, %esi xorl %eax, %eax .p2align 4,,10 .p2align 3 .L22: movq %rax, %rdx movb %al, (%rcx,%rax) addq $1, %rax cmpq %rdx, %rsi jne .L22 .L21: movslq %ebx, %rax movslq %r8d, %r8 movsbl (%rcx,%rax), %eax movsbl (%rcx,%r8), %edx movq %rdi, %rsp subl %edx, %eax testl %r12d, %r12d jle .L17 leal 1073741823(%rbx), %r8d xorl %ecx, %ecx .p2align 4,,10 .p2align 3 .L24: movq -32(%rbp), %rsi addq %rcx, %rsi movsbl (%rsi), %edx leal 1(%rdx), %edi orl %edx, %eax movq %rcx, %rdx addq $1, %rcx movb %dil, (%rsi) cmpq %rdx, %r8 jne .L24 .L17: movq -24(%rbp), %rbx xorq %fs:40, %rbx jne .L34 leaq -16(%rbp), %rsp popq %rbx popq %r12 popq %rbp .cfi_remember_state .cfi_def_cfa 7, 8 ret .L33: .cfi_restore_state orq $0, -8(%rsp,%rdx) jmp .L20 .L34: call __stack_chk_fail@PLT .cfi_endproc .LFE28: .size main, .-main .ident "GCC: (Ubuntu 9.3.0-10ubuntu2) 9.3.0" .section .note.GNU-stack,"",@progbits .section .note.gnu.property,"a" .align 8 .long 1f - 0f .long 4f - 1f .long 5 0: .string "GNU" 1: .align 8 .long 0xc0000002 .long 3f - 2f 2: .long 0x3 3: .align 8 4: ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: stack bounds 2020-10-10 21:49 ` Bruno Haible @ 2020-10-11 18:49 ` Paul Eggert 2020-10-11 22:08 ` Bruno Haible 0 siblings, 1 reply; 16+ messages in thread From: Paul Eggert @ 2020-10-11 18:49 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib On 10/10/20 2:49 PM, Bruno Haible wrote: > gcc -fstack-clash-protection -m32 -O2 stackish.c fixes this issue. Yes. However, the GCC manual says this about -fstack-clash-protection: Most targets do not fully support stack clash protection. However, on those targets '-fstack-clash-protection' will protect dynamic stack allocations. '-fstack-clash-protection' may also provide limited protection for static stack allocations if the target supports '-fstack-check=specific'. which is not as close to "it should just work" as I'd like, especially when I go read the section on -fstack-check. I suppose I need to look at the output of gcc -S -fstack-clash-protection on my platform (and understand what the OS does) to know whether stack overflow is detected reliably. That being said, it does look like a reliability win if we start using -fstack-clash-protection on platforms like Fedora x86-64 that support it and do not enable it by default. Perhaps we should have a Gnulib or Autoconf macro that does that. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: stack bounds 2020-10-11 18:49 ` Paul Eggert @ 2020-10-11 22:08 ` Bruno Haible 2020-10-11 22:56 ` Paul Eggert 0 siblings, 1 reply; 16+ messages in thread From: Bruno Haible @ 2020-10-11 22:08 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib Paul Eggert wrote: > That being said, it does look like a reliability win if we start using > -fstack-clash-protection on platforms like Fedora x86-64 that support it and do > not enable it by default. Perhaps we should have a Gnulib or Autoconf macro that > does that. Yes, such a macro would be useful. To be used at the discretion of the package maintainer (e.g. through a Gnulib module, similar to the 'largefile' module). But do you have an overview which targets are meant in the doc? "old-style stack checking is also the fallback method for ‘specific’ if no target support has been added in the compiler." "Most targets do not fully support stack clash protection." Bruno ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: stack bounds 2020-10-11 22:08 ` Bruno Haible @ 2020-10-11 22:56 ` Paul Eggert 0 siblings, 0 replies; 16+ messages in thread From: Paul Eggert @ 2020-10-11 22:56 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib On 10/11/20 3:08 PM, Bruno Haible wrote: > But do you have an overview which targets are meant in the doc? Unfortunately not. I expect it'd have to be determined from the comments in GCC, and it might also need info from various OSes and/or linkers. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-10-11 22:57 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <m2a6xnu1sv.fsf@meyering.net> [not found] ` <075a970b-f6aa-d2bb-e007-609a711085b2@gmail.com> [not found] ` <CA+8g5KGBjFfHzafN2WFH6az5bQR8f68B1J-ETVWEBXvuDUL5rA@mail.gmail.com> [not found] ` <17a0fe42-3ac6-9209-6f60-cddb5467f263@gmail.com> [not found] ` <994fd316-0420-4b94-a1de-fea7d891c4ac@gmail.com> 2020-09-18 22:16 ` new snapshot available: grep-3.4-almost.26-5419 Paul Eggert 2020-09-18 22:42 ` Bruce Dubbs 2020-09-18 23:24 ` libsigsegv on LinuxFromScratch Bruno Haible 2020-09-18 23:47 ` Bruce Dubbs 2020-09-19 8:06 ` Bruce Dubbs 2020-09-19 23:47 ` Bruno Haible 2020-09-20 1:16 ` Bruce Dubbs 2020-09-20 23:15 ` Paul Eggert 2020-09-23 0:58 ` stack bounds Bruno Haible 2020-09-23 1:28 ` Paul Eggert 2020-10-10 12:08 ` Bruno Haible 2020-10-10 20:10 ` Paul Eggert 2020-10-10 21:49 ` Bruno Haible 2020-10-11 18:49 ` Paul Eggert 2020-10-11 22:08 ` Bruno Haible 2020-10-11 22:56 ` Paul Eggert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).