* libgcc patch committed: Avoid hooks in split-stack code @ 2020-04-03 21:58 Ian Lance Taylor 2020-04-04 17:10 ` Eric Botcazou 2020-04-07 10:19 ` Richard Biener 0 siblings, 2 replies; 10+ messages in thread From: Ian Lance Taylor @ 2020-04-03 21:58 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 630 bytes --] The split-stack code invokes mmap and munmap with a limited amount of stack space. That is fine when the functions just make a system call, but it's not fine when programs use LD_PRELOAD or linker tricks to add hooks to mmap/munmap. Those hooks may use too much stack space, leading to an obscure program crash. Avoid that at least on GNU/Linux by calling __mmap/__munmap instead. Bootstrapped and ran Go and split-stack tests on x86_64-pc-linux-gnu. Committed to mainline. Ian 2020-04-03 Ian Lance Taylor <iant@golang.org> * generic-morestack.c: On GNU/Linux use __mmap/__munmap rather than mmap/munmap, to avoid hooks. [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 1013 bytes --] diff --git a/libgcc/generic-morestack.c b/libgcc/generic-morestack.c index c26dba1ae4a..bb9f67a7366 100644 --- a/libgcc/generic-morestack.c +++ b/libgcc/generic-morestack.c @@ -53,6 +53,23 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #include "generic-morestack.h" +/* Some systems use LD_PRELOAD or similar tricks to add hooks to + mmap/munmap. That breaks this code, because when we call mmap + there is enough stack space for the system call but there is not, + in general, enough stack space to run a hook. At least when using + glibc on GNU/Linux we can avoid the problem by calling __mmap and + __munmap. */ + +#ifdef __gnu_linux__ + +extern void *__mmap (void *, size_t, int, int, int, off_t); +extern int __munmap (void *, size_t); + +#define mmap __mmap +#define munmap __munmap + +#endif /* defined(__gnu_linux__) */ + typedef unsigned uintptr_type __attribute__ ((mode (pointer))); /* This file contains subroutines that are used by code compiled with ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-03 21:58 libgcc patch committed: Avoid hooks in split-stack code Ian Lance Taylor @ 2020-04-04 17:10 ` Eric Botcazou 2020-04-04 20:23 ` Ian Lance Taylor 2020-04-07 10:19 ` Richard Biener 1 sibling, 1 reply; 10+ messages in thread From: Eric Botcazou @ 2020-04-04 17:10 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gcc-patches > 2020-04-03 Ian Lance Taylor <iant@golang.org> > > * generic-morestack.c: On GNU/Linux use __mmap/__munmap rather > than mmap/munmap, to avoid hooks. This breaks the build of gotools for me with undefined references to __mmap and __munmap from libgcc/generic-morestack.c (it is with glibc 2.22). -- Eric Botcazou ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-04 17:10 ` Eric Botcazou @ 2020-04-04 20:23 ` Ian Lance Taylor 2020-04-04 20:28 ` Eric Botcazou 0 siblings, 1 reply; 10+ messages in thread From: Ian Lance Taylor @ 2020-04-04 20:23 UTC (permalink / raw) To: Eric Botcazou; +Cc: gcc-patches On Sat, Apr 4, 2020 at 10:10 AM Eric Botcazou <botcazou@adacore.com> wrote: > > > 2020-04-03 Ian Lance Taylor <iant@golang.org> > > > > * generic-morestack.c: On GNU/Linux use __mmap/__munmap rather > > than mmap/munmap, to avoid hooks. > > This breaks the build of gotools for me with undefined references to __mmap > and __munmap from libgcc/generic-morestack.c (it is with glibc 2.22). Hmmm, sorry about that. Which target? x86_64? It does seem that glibc consolidated mmap implementations in 2.26, but even in 2.22 I see definitions for __mmap. Ian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-04 20:23 ` Ian Lance Taylor @ 2020-04-04 20:28 ` Eric Botcazou 2020-04-04 20:35 ` H.J. Lu 0 siblings, 1 reply; 10+ messages in thread From: Eric Botcazou @ 2020-04-04 20:28 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gcc-patches > Hmmm, sorry about that. Which target? x86_64? It does seem that > glibc consolidated mmap implementations in 2.26, but even in 2.22 I > see definitions for __mmap. GNU C Library (GNU libc) stable release version 2.22 (git bbab82c25da9), by Roland McGrath et al. Copyright (C) 2015 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Configured for x86_64-suse-linux. Compiled by GNU CC version 4.8.5. Available extensions: crypt add-on version 2.1 by Michael Glad and others GNU Libidn by Simon Josefsson Native POSIX Threads Library by Ulrich Drepper et al BIND-8.2.3-T5B libc ABIs: UNIQUE IFUNC For bug reporting instructions, please see: <http://bugs.opensuse.org>. eric@polaris:~/build/gcc/native> readelf -s /lib64/libc.so.6 | grep mmap 317: 00000000000e8ac0 36 FUNC WEAK DEFAULT 13 mmap64@@GLIBC_2.2.5 657: 00000000000e8ac0 36 FUNC WEAK DEFAULT 13 mmap@@GLIBC_2.2.5 332: 0000000000070f00 320 FUNC LOCAL DEFAULT 13 _IO_wfile_underflow_mmap 364: 0000000000075310 453 FUNC LOCAL DEFAULT 13 mmap_remap_check 365: 00000000000754f0 90 FUNC LOCAL DEFAULT 13 _IO_file_sync_mmap 366: 0000000000075550 306 FUNC LOCAL DEFAULT 13 decide_maybe_mmap 368: 00000000000757a0 238 FUNC LOCAL DEFAULT 13 _IO_file_xsgetn_mmap 1727: 000000000039e5c0 168 OBJECT LOCAL DEFAULT 28 _IO_file_jumps_mmap 2012: 00000000000e8ac0 36 FUNC LOCAL DEFAULT 13 __GI_mmap 2065: 0000000000075a80 224 FUNC LOCAL DEFAULT 13 _IO_file_seekoff_mmap 2449: 00000000000752c0 74 FUNC LOCAL DEFAULT 13 _IO_file_close_mmap 2476: 00000000000e8ac0 36 FUNC LOCAL DEFAULT 13 __GI_mmap64 2520: 000000000006b040 63 FUNC LOCAL DEFAULT 13 __fopen_maybe_mmap 2605: 00000000000e8ac0 36 FUNC LOCAL DEFAULT 13 __GI___mmap64 2742: 00000000000e8ac0 36 FUNC LOCAL DEFAULT 13 __GI___mmap 2993: 00000000000e8ac0 36 FUNC LOCAL DEFAULT 13 __mmap64 3161: 00000000000e8ac0 36 FUNC LOCAL DEFAULT 13 __mmap 3304: 0000000000074d10 93 FUNC LOCAL DEFAULT 13 _IO_file_setbuf_mmap 3351: 0000000000075b60 81 FUNC LOCAL DEFAULT 13 _IO_file_underflow_mmap 3365: 000000000039e500 168 OBJECT LOCAL DEFAULT 28 _IO_file_jumps_maybe_mmap 3483: 000000000039e140 168 OBJECT LOCAL DEFAULT 28 _IO_wfile_jumps_mmap 5919: 00000000000e8ac0 36 FUNC WEAK DEFAULT 13 mmap64 6128: 00000000000e8ac0 36 FUNC WEAK DEFAULT 13 mmap -- Eric Botcazou ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-04 20:28 ` Eric Botcazou @ 2020-04-04 20:35 ` H.J. Lu 2020-04-04 20:48 ` Ian Lance Taylor 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2020-04-04 20:35 UTC (permalink / raw) To: Eric Botcazou; +Cc: Ian Lance Taylor, gcc-patches On Sat, Apr 4, 2020 at 1:28 PM Eric Botcazou <botcazou@adacore.com> wrote: > > > Hmmm, sorry about that. Which target? x86_64? It does seem that > > glibc consolidated mmap implementations in 2.26, but even in 2.22 I > > see definitions for __mmap. > commit fa872e1b6210e81e60d6029429f0a083b8eab26e Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> Date: Fri Dec 2 16:32:28 2016 -0200 Clean pthread functions namespaces for C11 threads ... * misc/Versions (libc): Export __mmap, __munmap, __mprotect, __sched_get_priority_min, and __sched_get_priority_max under GLIBC_PRIVATE. was checked into glibc 2.26. -- H.J. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-04 20:35 ` H.J. Lu @ 2020-04-04 20:48 ` Ian Lance Taylor 2020-04-04 21:16 ` Eric Botcazou 0 siblings, 1 reply; 10+ messages in thread From: Ian Lance Taylor @ 2020-04-04 20:48 UTC (permalink / raw) To: H.J. Lu; +Cc: Eric Botcazou, gcc-patches [-- Attachment #1: Type: text/plain, Size: 805 bytes --] On Sat, Apr 4, 2020 at 1:35 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Sat, Apr 4, 2020 at 1:28 PM Eric Botcazou <botcazou@adacore.com> wrote: > > > > > Hmmm, sorry about that. Which target? x86_64? It does seem that > > > glibc consolidated mmap implementations in 2.26, but even in 2.22 I > > > see definitions for __mmap. > > > > commit fa872e1b6210e81e60d6029429f0a083b8eab26e > Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> > Date: Fri Dec 2 16:32:28 2016 -0200 > > Clean pthread functions namespaces for C11 threads > ... > * misc/Versions (libc): Export __mmap, __munmap, __mprotect, > __sched_get_priority_min, and __sched_get_priority_max under > GLIBC_PRIVATE. > > was checked into glibc 2.26. Thanks. I committed this patch. Ian [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 567 bytes --] diff --git a/libgcc/generic-morestack.c b/libgcc/generic-morestack.c index bb9f67a7366..fa2062e2bb3 100644 --- a/libgcc/generic-morestack.c +++ b/libgcc/generic-morestack.c @@ -60,7 +60,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see glibc on GNU/Linux we can avoid the problem by calling __mmap and __munmap. */ -#ifdef __gnu_linux__ +#if defined(__gnu_linux__) && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 26)) extern void *__mmap (void *, size_t, int, int, int, off_t); extern int __munmap (void *, size_t); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-04 20:48 ` Ian Lance Taylor @ 2020-04-04 21:16 ` Eric Botcazou 0 siblings, 0 replies; 10+ messages in thread From: Eric Botcazou @ 2020-04-04 21:16 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gcc-patches, H.J. Lu > I committed this patch. Works for me, thanks for the quick fix! -- Eric Botcazou ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-03 21:58 libgcc patch committed: Avoid hooks in split-stack code Ian Lance Taylor 2020-04-04 17:10 ` Eric Botcazou @ 2020-04-07 10:19 ` Richard Biener 2020-04-07 10:33 ` Jakub Jelinek 1 sibling, 1 reply; 10+ messages in thread From: Richard Biener @ 2020-04-07 10:19 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gcc-patches On Fri, Apr 3, 2020 at 11:59 PM Ian Lance Taylor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > The split-stack code invokes mmap and munmap with a limited amount of > stack space. That is fine when the functions just make a system call, > but it's not fine when programs use LD_PRELOAD or linker tricks to add > hooks to mmap/munmap. Those hooks may use too much stack space, > leading to an obscure program crash. > > Avoid that at least on GNU/Linux by calling __mmap/__munmap instead. > > Bootstrapped and ran Go and split-stack tests on x86_64-pc-linux-gnu. > Committed to mainline. This causes references to GLIBC_PRIVATE exported symbols which is highly undesirable for system integrators (no ABI stability is guaranteed for those). I opened the regression PR94513 for this. Richard. > Ian > > 2020-04-03 Ian Lance Taylor <iant@golang.org> > > * generic-morestack.c: On GNU/Linux use __mmap/__munmap rather > than mmap/munmap, to avoid hooks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-07 10:19 ` Richard Biener @ 2020-04-07 10:33 ` Jakub Jelinek 2020-04-07 18:31 ` Ian Lance Taylor 0 siblings, 1 reply; 10+ messages in thread From: Jakub Jelinek @ 2020-04-07 10:33 UTC (permalink / raw) To: Richard Biener; +Cc: Ian Lance Taylor, gcc-patches On Tue, Apr 07, 2020 at 12:19:45PM +0200, Richard Biener via Gcc-patches wrote: > On Fri, Apr 3, 2020 at 11:59 PM Ian Lance Taylor via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > The split-stack code invokes mmap and munmap with a limited amount of > > stack space. That is fine when the functions just make a system call, > > but it's not fine when programs use LD_PRELOAD or linker tricks to add > > hooks to mmap/munmap. Those hooks may use too much stack space, > > leading to an obscure program crash. > > > > Avoid that at least on GNU/Linux by calling __mmap/__munmap instead. > > > > Bootstrapped and ran Go and split-stack tests on x86_64-pc-linux-gnu. > > Committed to mainline. > > This causes references to GLIBC_PRIVATE exported symbols which is > highly undesirable for system integrators (no ABI stability is guaranteed > for those). I opened the regression PR94513 for this. Yeah, GLIBC_PRIVATE symbols may only be used by glibc itself, not by anything else. I'd suggest to use syscall function instead (though, that can be interposed too). Jakub ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: libgcc patch committed: Avoid hooks in split-stack code 2020-04-07 10:33 ` Jakub Jelinek @ 2020-04-07 18:31 ` Ian Lance Taylor 0 siblings, 0 replies; 10+ messages in thread From: Ian Lance Taylor @ 2020-04-07 18:31 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1550 bytes --] On Tue, Apr 7, 2020 at 3:33 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Apr 07, 2020 at 12:19:45PM +0200, Richard Biener via Gcc-patches wrote: > > On Fri, Apr 3, 2020 at 11:59 PM Ian Lance Taylor via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > The split-stack code invokes mmap and munmap with a limited amount of > > > stack space. That is fine when the functions just make a system call, > > > but it's not fine when programs use LD_PRELOAD or linker tricks to add > > > hooks to mmap/munmap. Those hooks may use too much stack space, > > > leading to an obscure program crash. > > > > > > Avoid that at least on GNU/Linux by calling __mmap/__munmap instead. > > > > > > Bootstrapped and ran Go and split-stack tests on x86_64-pc-linux-gnu. > > > Committed to mainline. > > > > This causes references to GLIBC_PRIVATE exported symbols which is > > highly undesirable for system integrators (no ABI stability is guaranteed > > for those). I opened the regression PR94513 for this. > > Yeah, GLIBC_PRIVATE symbols may only be used by glibc itself, not by > anything else. > I'd suggest to use syscall function instead (though, that can be interposed > too). Sorry about that. OK, let's try syscall. I committed this patch. Bootstrapped and tested on x86_64-pc-linux-gnu, did a small amount of testing on s390x (but I don't have full access to an s390x system). Ian 2020-04-07 Ian Lance Taylor <iant@golang.org> PR libgcc/94513 * generic-morestack.c: Give up trying to use __mmap/__munmap, use syscall instead. [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 2179 bytes --] diff --git a/libgcc/generic-morestack.c b/libgcc/generic-morestack.c index fa2062e2bb3..35764a848f6 100644 --- a/libgcc/generic-morestack.c +++ b/libgcc/generic-morestack.c @@ -56,17 +56,56 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see /* Some systems use LD_PRELOAD or similar tricks to add hooks to mmap/munmap. That breaks this code, because when we call mmap there is enough stack space for the system call but there is not, - in general, enough stack space to run a hook. At least when using - glibc on GNU/Linux we can avoid the problem by calling __mmap and - __munmap. */ + in general, enough stack space to run a hook. Try to avoid the + problem by calling syscall directly. We only do this on GNU/Linux + for now, but it should be easy to add support for more systems with + testing. */ -#if defined(__gnu_linux__) && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 26)) +#if defined(__gnu_linux__) -extern void *__mmap (void *, size_t, int, int, int, off_t); -extern int __munmap (void *, size_t); +#include <sys/syscall.h> -#define mmap __mmap -#define munmap __munmap +#if defined(SYS_mmap) || defined(SYS_mmap2) + +#ifdef SYS_mmap2 +#define MORESTACK_MMAP SYS_mmap2 +#define MORESTACK_ADJUST_OFFSET(x) ((x) / 4096ULL) +#else +#define MORESTACK_MMAP SYS_mmap +#define MORESTACK_ADJUST_OFFSET(x) (x) +#endif + +static void * +morestack_mmap (void *addr, size_t length, int prot, int flags, int fd, + off_t offset) +{ + offset = MORESTACK_ADJUST_OFFSET (offset); + +#ifdef __s390__ + long args[6] = { (long) addr, (long) length, (long) prot, (long) flags, + (long) fd, (long) offset }; + return (void *) syscall (MORESTACK_MMAP, args); +#else + return (void *) syscall (MORESTACK_MMAP, addr, length, prot, flags, fd, + offset); +#endif +} + +#define mmap morestack_mmap + +#endif /* defined(SYS_MMAP) || defined(SYS_mmap2) */ + +#if defined(SYS_munmap) + +static int +morestack_munmap (void * addr, size_t length) +{ + return (int) syscall (SYS_munmap, addr, length); +} + +#define munmap morestack_munmap + +#endif /* defined(SYS_munmap) */ #endif /* defined(__gnu_linux__) */ ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-07 18:31 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-03 21:58 libgcc patch committed: Avoid hooks in split-stack code Ian Lance Taylor 2020-04-04 17:10 ` Eric Botcazou 2020-04-04 20:23 ` Ian Lance Taylor 2020-04-04 20:28 ` Eric Botcazou 2020-04-04 20:35 ` H.J. Lu 2020-04-04 20:48 ` Ian Lance Taylor 2020-04-04 21:16 ` Eric Botcazou 2020-04-07 10:19 ` Richard Biener 2020-04-07 10:33 ` Jakub Jelinek 2020-04-07 18:31 ` Ian Lance Taylor
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).