Bug 21672 - sys-libs/glibc on ia64 crashes on thread exit: signal SIGSEGV, Segmentation fault: pthread_create.c:432: __madvise (pd->stackblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED);
Summary: sys-libs/glibc on ia64 crashes on thread exit: signal SIGSEGV, Segmentation f...
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: 2.24
: P2 normal
Target Milestone: 2.27
Assignee: Adhemerval Zanella
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-25 21:52 UTC by Sergei Trofimovich
Modified: 2017-12-13 11:00 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
0001-fix-pthread_create-crash-in-ia64.patch (1.26 KB, patch)
2017-06-25 22:02 UTC, Sergei Trofimovich
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergei Trofimovich 2017-06-25 21:52:33 UTC
First found in gentoo in https://bugs.gentoo.org/622694

The tets file:

$ cat bug.c 
    // how to crash: gcc -O0 -ggdb3 -o r bug.c -pthread && ./r

    #include <pthread.h>

    static void * f (void * p)
    {
        return NULL;
    }

    int main (int argc, const char ** argv)
    {
        pthread_t t;
        pthread_create (&t, NULL, &f, NULL);

        pthread_join (t, NULL);
        return 0;
    }

How to crash:
$ gcc -O0 -ggdb3 -o r bug.c -pthread && ./r
Segmentation fault (core dumped)

$  gdb r core
...
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x2000000000077da0 in start_thread (arg=0x0) at pthread_create.c:432
432         __madvise (pd->stackblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED);
[Current thread is 1 (Thread 0x2000000000b6b1f0 (LWP 20912))]

(gdb) list
427     #ifdef _STACK_GROWS_DOWN
428       char *sp = CURRENT_STACK_FRAME;
429       size_t freesize = (sp - (char *) pd->stackblock) & ~pagesize_m1;
430       assert (freesize < pd->stackblock_size);
431       if (freesize > PTHREAD_STACK_MIN)
432         __madvise (pd->stackblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED);
433     #else
434       /* Page aligned start of memory to free (higher than or equal
435          to current sp plus the minimum stack size).  */
436       void *freeblock = (void*)((size_t)(CURRENT_STACK_FRAME

#0  0x2000000000077da0 in start_thread (arg=0x0) at pthread_create.c:432
        pd = 0x0
        now = <optimized out>
        unwind_buf = <error reading variable unwind_buf (Cannot access memory at address 0xfffffffffffffd90)>
        not_first_call = <optimized out>
        pagesize_m1 = <optimized out>
        sp = 0x2000000000b6a870 ""
        freesize = <optimized out>
        __PRETTY_FUNCTION__ = "start_thread"
#1  0x0000000000000000 in ?? ()
Comment 1 Sergei Trofimovich 2017-06-25 22:02:44 UTC
Created attachment 10221 [details]
0001-fix-pthread_create-crash-in-ia64.patch

The SIGSEGV is caused by the code responsible for stack cleanup
when thread exits. madvise(MADV_DONTNEED) is called on a part of stack
activelu being used at exit.

It happens because on ia64 stack grows from both sides of stack block:
 - normal "sp" stack (stack for local variables) grows down
 - register stack "bsp" grows up from the opposite end of stack block

madvise(MADV_DONTNEED) effectively does memset(0) register stack
which causes SIGSEGV at address 0x8 afterwards when a pointer frop
stack is being dereferenced.
Comment 2 Sergei Trofimovich 2017-06-25 22:09:23 UTC
Sent the patch to libc-alpha as https://sourceware.org/ml/libc-alpha/2017-06/msg01265.html
Comment 3 Sergei Trofimovich 2017-06-27 09:57:41 UTC
Described in more details breakage mechanics: http://trofi.github.io/posts/202-stack-growth-direction-how-hard-can-it-be.html
Comment 4 Sourceware Commits 2017-08-29 16:32:15 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  01b87c656f670863ce437421b8e9278200965d38 (commit)
      from  16f138a49ad1815e113d2b5b7a87f26999ade811 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=01b87c656f670863ce437421b8e9278200965d38

commit 01b87c656f670863ce437421b8e9278200965d38
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Mon Aug 28 11:24:35 2017 -0300

    ia64: Fix thread stack allocation permission set (BZ #21672)
    
    This patch fixes ia64 failures on thread exit by madvise the required
    area taking in consideration its disjoing stacks
    (NEED_SEPARATE_REGISTER_STACK).  Also the snippet that setup the
    madvise call to advertise kernel the area won't be used anymore in
    near future is reallocated in allocatestack.c (for consistency to
    put all stack management function in one place).
    
    Checked on x86_64-linux-gnu and i686-linux-gnu for sanity (since
    it is not expected code changes for architecture that do not
    define NEED_SEPARATE_REGISTER_STACK) and also got a report that
    it fixes ia64-linux-gnu failures from Sergei Trofimovich
    <slyfox@gentoo.org>.
    
    	[BZ #21672]
    	* nptl/allocatestack.c [_STACK_GROWS_DOWN] (setup_stack_prot):
    	Set to use !NEED_SEPARATE_REGISTER_STACK as well.
    	(advise_stack_range): New function.
    	* nptl/pthread_create.c (START_THREAD_DEFN): Move logic to mark
    	stack non required to advise_stack_range at allocatestack.c

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog             |    9 +++++++++
 nptl/allocatestack.c  |   29 ++++++++++++++++++++++++++++-
 nptl/pthread_create.c |   27 ++-------------------------
 3 files changed, 39 insertions(+), 26 deletions(-)
Comment 5 Adhemerval Zanella 2017-08-29 16:32:48 UTC
Fixed by 01b87c656f670863ce437421b8e9278200965d38.
Comment 6 Sourceware Commits 2017-12-13 11:00:06 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, release/2.26/master has been updated
       via  828efe784258ed0c53e1594463c084045bdc8acb (commit)
       via  3a0b7d09dbbf2493b54aacdbf462987892f8a6cd (commit)
       via  d8b79b0eb1f8ab69333e47f432c8174dde508d2b (commit)
      from  c48e2e558e27f5d38346828c46f9c325c21e52f5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=828efe784258ed0c53e1594463c084045bdc8acb

commit 828efe784258ed0c53e1594463c084045bdc8acb
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Wed Dec 13 11:42:27 2017 +0100

    Update IA64 libm-test-ulps
    
    Ran on Itanium Processor 9020, GCC 7.2.1.
    
    	* sysdeps/ia64/fpu/libm-test-ulps: Update.
    
    Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=3a0b7d09dbbf2493b54aacdbf462987892f8a6cd

commit 3a0b7d09dbbf2493b54aacdbf462987892f8a6cd
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Mon Aug 28 11:24:35 2017 -0300

    ia64: Fix thread stack allocation permission set (BZ #21672)
    
    This patch fixes ia64 failures on thread exit by madvise the required
    area taking in consideration its disjoing stacks
    (NEED_SEPARATE_REGISTER_STACK).  Also the snippet that setup the
    madvise call to advertise kernel the area won't be used anymore in
    near future is reallocated in allocatestack.c (for consistency to
    put all stack management function in one place).
    
    Checked on x86_64-linux-gnu and i686-linux-gnu for sanity (since
    it is not expected code changes for architecture that do not
    define NEED_SEPARATE_REGISTER_STACK) and also got a report that
    it fixes ia64-linux-gnu failures from Sergei Trofimovich
    <slyfox@gentoo.org>.
    
    	[BZ #21672]
    	* nptl/allocatestack.c [_STACK_GROWS_DOWN] (setup_stack_prot):
    	Set to use !NEED_SEPARATE_REGISTER_STACK as well.
    	(advise_stack_range): New function.
    	* nptl/pthread_create.c (START_THREAD_DEFN): Move logic to mark
    	stack non required to advise_stack_range at allocatestack.c
    
    (cherry pick from commit 01b87c656f670863ce437421b8e9278200965d38)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d8b79b0eb1f8ab69333e47f432c8174dde508d2b

commit d8b79b0eb1f8ab69333e47f432c8174dde508d2b
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Tue Dec 12 21:16:53 2017 +0100

    posix: Fix mmap for m68k and ia64 (BZ#21908)
    
    Default semantic for mmap2 syscall is to take the offset in 4096-byte
    units.  However m68k and ia64 mmap2 implementation take in the
    configured pageunit units and for both architecture it can be
    different values.
    
    This patch fixes the m68k runtime discover of mmap2 offset unit
    and adds the ia64 definition to find it at runtime.
    
    Checked the basic tst-mmap and tst-mmap-offset on m68k (the system
    is configured with 4k, so current code is already passing on this
    system) and a sanity check on x86_64-linux-gnu (which should not be
    affected by this change).  Sergei also states that ia64 loader now
    work correctly with this change.
    
    	Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    	Sergei Trofimovich  <slyfox@inbox.ru>
    
    	* sysdeps/unix/sysv/linux/m68k/mmap_internal.h (MMAP2_PAGE_SHIFT):
    	Rename to MMAP2_PAGE_UNIT.
    	* sysdeps/unix/sysv/linux/mmap.c: Include mmap_internal iff
    	__OFF_T_MATCHES_OFF64_T is not defined.
    	* sysdeps/unix/sysv/linux/mmap_internal.h (page_unit): Declare as
    	uint64_t.
    	(MMAP2_PAGE_UNIT) [MMAP2_PAGE_UNIT == -1]: Redefine to page_unit.
    	(page_unit) [MMAP2_PAGE_UNIT != -1]: Remove definition.
    
    (cherry picked from commit 1f14d0c3ddce47f7021bbc0862fdb207891345dc)

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                          |   27 +
 nptl/allocatestack.c                               |   29 +-
 nptl/pthread_create.c                              |   27 +-
 sysdeps/ia64/fpu/libm-test-ulps                    | 1940 +++++++++++++++++++-
 .../unix/sysv/linux/{m68k => ia64}/mmap_internal.h |   14 +-
 sysdeps/unix/sysv/linux/m68k/mmap_internal.h       |    2 +-
 sysdeps/unix/sysv/linux/mmap.c                     |    2 +-
 sysdeps/unix/sysv/linux/mmap_internal.h            |    6 +-
 8 files changed, 1983 insertions(+), 64 deletions(-)
 copy sysdeps/unix/sysv/linux/{m68k => ia64}/mmap_internal.h (70%)