Re: [PATCH][BZ 21672] fix pthread_create crash in ia64

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 02/08/2017 17:53, Sergei Trofimovich wrote:
> On Wed, 5 Jul 2017 22:47:17 +0100
> Sergei Trofimovich <slyfox@gentoo.org> wrote:
> 
>> On Sun, 25 Jun 2017 23:07:15 +0100
>> Sergei Trofimovich <slyfox@gentoo.org> wrote:
>>
>>> Minimal reproducer:
>>>
>>>     #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;
>>>     }
>>>
>>>     $ gcc -O0 -ggdb3 -o r bug.c -pthread && ./r
>>>
>>>     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);
>>>
>>> Here crash happens right after attempt to free unused part of
>>> thread's stack.
>>>
>>> On most architectures stack grows only down or grows only up.
>>> And there glibc decides which of unused ends of stack blocks can be freed.
>>>
>>> ia64 maintans two stacks. Both of them grow from the opposite directions:
>>>  - normal "sp" stack (stack for local variables) grows down
>>>  - register stack "bsp" grows up from the opposite end of stack block
>>>
>>> In this failure case we have prematurely freed "rsp" stack.
>>>
>>> The change leaves a few pages from both sides of stack block.
>>>
>>> Bug: https://sourceware.org/PR21672
>>> Bug: https://bugs.gentoo.org/622694
>>> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
>>> ---
>>>  nptl/pthread_create.c | 18 ++++++++++++++++--
>>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
>>> index 7a970ffc5b..6e3f6db5b1 100644
>>> --- a/nptl/pthread_create.c
>>> +++ b/nptl/pthread_create.c
>>> @@ -555,10 +555,24 @@ START_THREAD_DEFN
>>>    size_t pagesize_m1 = __getpagesize () - 1;
>>>  #ifdef _STACK_GROWS_DOWN
>>>    char *sp = CURRENT_STACK_FRAME;
>>> -  size_t freesize = (sp - (char *) pd->stackblock) & ~pagesize_m1;
>>> +  char *freeblock = (char *) pd->stackblock;
>>> +  size_t freesize = (sp - freeblock) & ~pagesize_m1;
>>>    assert (freesize < pd->stackblock_size);
>>> +# ifdef __ia64__
>>>    if (freesize > PTHREAD_STACK_MIN)
>>> -    __madvise (pd->stackblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED);
>>> +    {
>>> +      /* On ia64 stack grows both ways!
>>> +         - normal "sp" stack (stack for local variables) grows down
>>> +         - register stack "bsp" grows up from the opposite end of stack block
>>> +
>>> +         Thus we leave PTHREAD_STACK_MIN bytes from stack block top
>>> +         and leave same PTHREAD_STACK_MIN at stack block bottom.  */
>>> +      freeblock += PTHREAD_STACK_MIN;
>>> +      freesize -= PTHREAD_STACK_MIN;
>>> +    }
>>> +# endif
>>> +  if (freesize > PTHREAD_STACK_MIN)
>>> +    __madvise (freeblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED);
>>>  #else
>>>    /* Page aligned start of memory to free (higher than or equal
>>>       to current sp plus the minimum stack size).  */
>>> -- 
>>> 2.13.1
>>>   
>>
>> Ping :)
> 
> Ping^2
> 

What about https://sourceware.org/ml/libc-alpha/2017-07/msg00302.html ?