From patchwork Fri Jul 7 14:36:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 785533 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3x3xxr4cvcz9t2P for ; Sat, 8 Jul 2017 00:36:32 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="CD97fSsQ"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=MNaorDzR/jhmll28 unHgi7SnYXNFVwQcAjB6Yg2xMuHP0t6vcUMAdbi0w5A1PnpKLNzQXvVH7WtZ30W5 h+zCcwnHQLDD2SWqSuyziQMLBM9KpDhikLXK7lwK9MSbS0dLi8z+f3Tgukjivqyc QrlhBc2Dyoxa3yVKiLaquHJNnnc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=8QAUEw02a7ik4zfHpjazss ZPFMg=; b=CD97fSsQtQ2E2BWGUTZFDYanvwcEAo6Lddc8vdRA8629cb0OpH3h/X iVXCizD1xEclTRnZeSl/ydl1cl2XTcdNLk/jQirn2VDnK69c5m6U85Rmvhfgx+Wi gBl9VWIHgr8fhUGzHoiHu6nuKFNDGIuaMsLMGpDgB+CESzxURVoME= Received: (qmail 47650 invoked by alias); 7 Jul 2017 14:36:25 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 46345 invoked by uid 89); 7 Jul 2017 14:36:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=ski, bsp X-HELO: mail-qt0-f173.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=wyHVOF4HzC22wkoL2JsY7TkCP0NMeoJma8qSj7EATac=; b=lNwSqQg+wvHp93avw3m7h3lKiEqD4F4I8dJJ0r2QUGPgENrlmw2UhzKIBJ42oijIMC gtYvdH2VyCcXZrHilXQ9svDYHZ64VtT4/ZqIem30khfzvdfqUZ8PtsDFnCxDpZlaZI1h VDPSiGyDjUTaGHKtaSU8nO92z7pGLTULHjcZkIm+DWwESv+Bgw08Xdn0cr1xPXpNRNXe ZycOfu7tFlFwXIJQDSlEsg5f6sNtYxIUcYEgb3fwRq+kM3RsSvQS4PQLi34BuvMBw4In 2l2GT0E3rAyGObEA/cJgcsCQ5T7drNhzoOK3uTE09TsWTdVrRE20/Non+fvceDAVJuRO pO2g== X-Gm-Message-State: AKS2vOwSa0Bhls+0EC/SiXCW/Ofh0IDoEz/etruCut6MpxYrKcy2YeAP 2MYj9WVUvrfkMJuBHiAi1A== X-Received: by 10.200.57.162 with SMTP id v31mr71654142qte.57.1499438180352; Fri, 07 Jul 2017 07:36:20 -0700 (PDT) Subject: Re: [PATCH][BZ 21672] fix pthread_create crash in ia64 To: libc-alpha@sourceware.org References: <20170625220715.19422-1-slyfox@gentoo.org> From: Adhemerval Zanella Message-ID: Date: Fri, 7 Jul 2017 11:36:17 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170625220715.19422-1-slyfox@gentoo.org> On 25/06/2017 19:07, Sergei Trofimovich wrote: > Minimal reproducer: > > #include > > 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 I am trying to validate this change using the same emulator you referenced in the bug reports (ski) using first master then 2.25 to check if it is a recent change (since Mike Frysinger reported that on actual hardware that nptl tests are actually working [1]). However, even your simple testcase seems to fail either or without the proposed fix. So I am not sure if it is something wrong with the kernel I build (a 4.12 from linux-stable), the base environment I set (I basically use the sysroot create from build-many-glibc.py plus a busybox to have some workable tools). In any case, I would like to know why exactly this started to happen since on 2.25 mostly nptl cases shows no issue. I am more inclined to take this is something related to my changes for BZ#18988 [2] and I think we need to take these separate stack in consideration on 'setup_stack_prot' which is not what we currently doing. If I understood correctly, for both IA64 and HPPA (the only arch with _STACK_GROWN_UP) the mmap area for thread stack will have two disjoin areas and on ia64 it will be the two stack areas divided by the guard page. If it is the case I think we need to use the same logic of _STACK_GROWS_UP on 'setup_stack_prot'. Could you check if the patch below helps? [1] https://sourceware.org/glibc/wiki/Release/2.25 [2] https://sourceware.org/git/?p=glibc.git;a=commit;h=0edbf1230131dfeb03d843d2859e2104456fad80 > Signed-off-by: Sergei Trofimovich > --- > 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). */ > diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index ec7d42e..d07b410 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -356,7 +356,7 @@ setup_stack_prot (char *mem, size_t size, char *guard, size_t guardsize, const int prot) { char *guardend = guard + guardsize; -#if _STACK_GROWS_DOWN +#if _STACK_GROWS_DOWN && !defined(NEED_SEPARATE_REGISTER_STACK) /* As defined at guard_position, for architectures with downward stack the guard page is always at start of the allocated area. */ if (__mprotect (guardend, size - guardsize, prot) != 0)