From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72760 invoked by alias); 12 Jul 2017 21:08:33 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 72741 invoked by uid 89); 12 Jul 2017 21:08:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=attack, distro, theyll, they'll X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Jul 2017 21:08:30 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3CEB17F6A7; Wed, 12 Jul 2017 21:08:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3CEB17F6A7 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=law@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 3CEB17F6A7 Received: from localhost.localdomain (ovpn-116-196.phx2.redhat.com [10.3.116.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id CF84D6E50F; Wed, 12 Jul 2017 21:08:28 +0000 (UTC) Subject: Re: [PATCH][RFA/RFC] Stack clash mitigation patch 07/08 To: Trevor Saunders Cc: Wilco Dijkstra , GCC Patches , nd References: <7bfc8713-e651-beec-1ae5-67cd499c78f7@redhat.com> <20170712124709.dabzomuzstsg5bfq@ball> From: Jeff Law Message-ID: <96154a33-ab91-8b37-5b0e-ee0c019810c6@redhat.com> Date: Wed, 12 Jul 2017 21:08:00 -0000 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: <20170712124709.dabzomuzstsg5bfq@ball> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00617.txt.bz2 On 07/12/2017 06:47 AM, Trevor Saunders wrote: > On Tue, Jul 11, 2017 at 08:02:26PM -0600, Jeff Law wrote: >> On 07/11/2017 06:20 PM, Wilco Dijkstra wrote: >>> Jeff Law wrote: >>>> aarch64 is the first target that does not have any implicit probes in >>>> the caller. Thus at prologue entry it must make conservative >>>> assumptions about the offset of the most recent probed address relative >>>> to the stack pointer. >>> >>> No - like I mentioned before that's not correct nor acceptable as it would imply >>> that ~70% of functions need a probe at entry. I did a quick run across SPEC and >>> found the outgoing argument size is > 1024 in just 9 functions out of 147000! >>> Those 9 were odd special cases due to auto generated code to interface between >>> C and Fortran. This is extremely unlikely to occur anywhere else. So even assuming >>> an unchecked caller, large outgoing arguments are simply not a realistic threat. >> Whether or not such frames exist in SPEC isn't the question. THere's >> nothing in the ABI or ISA that allows us to avoid those probes without >> compromising security. >> >> Mixed code scenarios are going to be a fact of life, probably forever >> unless we can convince every ISV providing software that works on top of >> RHEL/*BSD/whatever to turn on probing (based on my experiences, that >> has exactly a zero chance of occurring). > > On the other hand if probing is fast enough that it can be on by default > in gcc there should be much less of it. Even if you did change the ABI > to require probing it seems unlikely that code violating that > requirement would hit problems other than this security concern, so I'd > expect there will be some non compliant asm out there. Certainly my goal is to enable it by default one day. Even if that's ultimately done at the distro level or by a configure time switch. Certainly if/when we reach that point the amount of unprotected code will drop dramatically, but based on my experience I fully expect some folks to turn it off. They'll say something like "hey, we've audited our code and we don't have any large stack or allocas, so I'm turning this thing off to get some un-measurable performance gain." It'd be an uber-dumb thing to do, but it's going to happen. And once that happens, they're open to attack again, even if they were correct in their audit and their code only calls stack-clash protected libraries. Tweaking the ABI to mandate touching *sp in the outgoing args area & alloca space is better because we likely wouldn't have an option to avoid that access. So a well meaning, but clueless, developer couldn't turn the option off like they could stack checking. > >> In a mixed code scenario a caller may have a large alloca or large >> outgoing argument area that pushes the stack pointer into the guard page >> without actually accessing the guard page. That requires a callee which >> is compiled with stack checking to make worst case assumptions at >> function entry to protect itself as much as possible from these attacks. > > It seems to me pretty important to ask how many programs out there have > a caller that can push the stack into the guard page, but not past it. I've actually spent a lot of time thing about that precise problem. You don't need large frames to do that -- you just need controlled heap leaks and/or controlled recursion. ie, even if a function has no allocas and a small frame, it can put the stack pointer into the guard. THus in the callee we have to make worst case assumptions to be safe on targets where there is no implicit probe in the caller. > I'd expect that's not the case for most allocas, either they are safe, > or can increase the stack arbitrarily. I'd expect its more likely with > outgoing arg or large buffer allocations though. Unfortunately not. A small alloca is not inherently safe unless you also touch the allocated space. That's precisely why the generic code touches residuals after the loop that handles PROBE_INTERVAL chunks (consider a small alloca in a loop). That's also why I suggested a small backwards compatible tweak to the aarch64 ABI. Specifically there should be a mandated touch of *sp in any function where the outgoing args size is greater than some well defined value or if the function allocates any dynamic stack space. That mandated touch of *sp at the ABI level would be enough to allow for a much less conservative assumed state at function start and would allow the vast majority of functions to need no probing at all. I think the largest > buffer Qualys found was less than 400k? So 1 256k guard page should > protect 95% of functions, and 1m or 2m seems like enough to protect > against all non malicious programs. I'm not sure, but this is a 64 bit > arch, so it seems like we should have the adress space for large guard > pages like that. I'm all for larger guards, particularly on 64 bit architectures. We use 64k pages on aarch64 for RHEL which implicitly gives us a minimum guard of 64k. That would be a huge factor in any analysis I would do if the aarch64 maintainers choose not to fully protect their architecture and I was forced to make a recommendation for Red Hat and its customers. I hope I don't have to sit down and do the analysis on this and make any kind of recommendation. The fact that Qualys found nothing larger than X (for any X) in the code they scanned isn't relevant. There could well be code out there they did not look at that uses > X or code that is yet to be written that uses > X. > > There's also the trade off that increasing the amount of code that > probes reduces the set of code that can either move the stack into or > past the guard page. Correct. > >> THe aarch64 maintainers can certain nix what I've done or modify it in >> ways that suit them. That is their choice as port maintainers. >> However, Red Hat will have to evaluate the cost of reducing security for >> our customer base against the performance improvement of such changes. >> As I've said before, I do not know where that decision would fall. >> >> >>> >>> Therefore even when using a tiny 4K probe size we can safely adjust SP by 3KB >>> before needing an explicit probe - now only 0.6% of functions need a probe. >>> If we choose a proper minimum probe distance, say 64KB, explicit probes are >>> basically non-existent (just 35 functions, or ~0.02% of all functions are > 64KB). >>> Clearly inserting probes can be the default as the impact on code quality is negligible. >> Again, there's nothing that says 3k is safe. You're picking an >> arbitrary point that is safe in a codebase you've looked at. But I'm >> looking at this from a "what guarantees do I have from an ABI or ISA >> standpoint". The former may be more performant, but it's inherently >> less secure than the latter. > > On the other hand making -fstack-check=clash the default seems to me > like a very significant security improvement. Agreed and it's where I'd like this to go. Jeff