From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x736.google.com (mail-qk1-x736.google.com [IPv6:2607:f8b0:4864:20::736]) by sourceware.org (Postfix) with ESMTPS id 2A258388C00F for ; Fri, 12 Mar 2021 17:34:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2A258388C00F Received: by mail-qk1-x736.google.com with SMTP id m186so8433518qke.12 for ; Fri, 12 Mar 2021 09:34:05 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:references:from:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=u1LrOcN2VLkTBnMth8RVLC3Ztdrl41nx/3+O1DHUziM=; b=e6co82BwL+7aaaoVkf3TvsDqSIR6VlDIae5UvAUpTQ7eo0B701fMU/w90EQFzeayP3 hj/0/KfoNsmrGHUH9LA33+SdpZy40kkIquqUySHSM+siLB2t0wf6mYdog5d0QCdSTFnt yyPvi+AvUj0TnrJwdA4eMHKlqIxowyXltUm8tvys97DPrrxOoaxDC/Fznpo5S+CZZGrB 8JxyY8XrNfSBIZaUf3kZsWaB3siIaIIi6XklZ3t8AAq6328aIPxAHET9/tp5+SQFx7ru T9EjbYqnStnOLIZZ6H7OTErHnptmH+sqtr51vNik8fMmW0D3wgA1EptTiEV3oUN3j3Bx PbpA== X-Gm-Message-State: AOAM532eYwHytuAfRjPV1k7ajUjtiP0qO6hwSlo5i4MvspitR3iAkqzw FABTFabyNZhPUKR9M5TbZgtYYg== X-Google-Smtp-Source: ABdhPJwECp8OvtnrY/BKhJMZYyB+SHnE8KJB9iy3DPlEeCNPaj43s8e87Yx8sbOew/B7yebmmWFeQA== X-Received: by 2002:a37:a54f:: with SMTP id o76mr13780411qke.95.1615570444712; Fri, 12 Mar 2021 09:34:04 -0800 (PST) Received: from [192.168.1.4] ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id j15sm4324569qtr.34.2021.03.12.09.34.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 12 Mar 2021 09:34:04 -0800 (PST) To: Siddhesh Poyarekar , libc-alpha@sourceware.org, Florian Weimer , Sergei Trofimovich References: <20210310101400.3904724-1-siddhesh@sourceware.org> <20210310101400.3904724-2-siddhesh@sourceware.org> <00d1c526-cff0-8b00-1009-a488695d1f86@linaro.org> <595c9ca3-a0d8-5958-2544-500812d2ecc3@sourceware.org> From: Adhemerval Zanella Subject: Re: [PATCH 1/3] Add inhibit_stack_protector to ifuncmain9 [BZ #25680] Message-ID: <93d85ab8-071f-50b0-f56c-cc10e6b6e206@linaro.org> Date: Fri, 12 Mar 2021 14:34:01 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <595c9ca3-a0d8-5958-2544-500812d2ecc3@sourceware.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Mar 2021 17:34:06 -0000 On 12/03/2021 05:52, Siddhesh Poyarekar wrote: > On 3/10/21 6:20 PM, Adhemerval Zanella via Libc-alpha wrote: >> >> >> On 10/03/2021 07:13, Siddhesh Poyarekar via Libc-alpha wrote: >>> From: David Hughes >>> >>> Enabling --enable-stack-protector=all causes the following tests to fail: >>> >>>      FAIL: elf/ifuncmain9picstatic >>>      FAIL: elf/ifuncmain9static >>> >>> Nick Alcock (who committed the stack protector code) marked the IFUNC >>> resolvers with inhibit_stack_protector when he done the original work and >>> suggested doing so again @ BZ #25680. This patch adds >>> inhibit_stack_protector to ifuncmain9. >>> >>> After patch is applied, --enable-stack-protector=all does not fail the >>> above tests. >> >> The BZ#25680 report makes me wonder if would be better to just disable >> --enable-stack-protector=all on architecture with IFUNC for now. >> This fix the issue on glibc testsuite, but it might still trigger >> by users if this same trick is not used (as noted by Sergei). > > The general problem is that it is at best pointless to add stack protector prologue/epilogue to ifunc resolver functions in static binaries.  On x86_64 it is visible because it results in a crash due to TLS segment register %fs not being set up but elsewhere, the stack chk guard is zero, which makes the dereference safe, but useless.  On ppc64le, the TLS setup happens first, thus avoiding the crash and behaving like the other non-TLS stack_chk_guard architectures. > > One way to make it not crash on x86_64 could be to delay ifunc resolution (which would then mean making sure that TLS doesn't use ifunc'd functions, e.g. memcpy) and bring it on par with the rest of the architectures. Is it something preventing us to make x86 follows other architectures in this regard? > > I'm also considering proposing that gcc skips over ifunc resolvers for stack protection, at least in the static case.  The dynamic case happens to work somehow, I am yet to conclude that it's by design. Not sure if adding exceptions for specific cases is the best approach, the users would expect that such mitigations to work regardless of compilation/linking model (although some do require extra linker/runtime support). My take is if we could fix on glibc we should aim for it. > >> Florian stated on comment #2 that “all” is very unlikely to add >> additional protection and this basically adds *another* undocumented >> ifunc restriction. And it seems likely that distributions like Gentoo >> will just use 'strong' instead of 'all'. > > AFAICT Gentoo decided to use 'strong' instead of 'all' because of Florian's comment. > >> So, what would be the implications of limiting stack protection to >> 'strong' instead of 'all' for ABIs with ifunc? Is this issue only >> redistricted to some ABIs (the reported indicates that only x86_64 >> is affected)? > > It's not the glibc build that's affected though, at least not to an extent that we ought to care.  What's broken is our support with binaries that were built with stack-protector-all *and* ifuncs.  This patch implies that we don't support it; all we need to do is document this somewhere and perhaps fix gcc to never emit the prologue/epilogue for ifunc resolvers. Yeah, I understood that and my concern is if users do start to use ifunc more profusely they will need to keep adding the inhibit_stack_protector on resolvers to work around this issue. Fixing the ifunc resolution order also might simplify the code and avoid adding the inhibit_stack_protector on multiples files.