From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 519 invoked by alias); 10 Jan 2019 15:49:39 -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 502 invoked by uid 89); 10 Jan 2019 15:49:39 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 10 Jan 2019 15:49:37 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8BF281596; Thu, 10 Jan 2019 07:49:35 -0800 (PST) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 109E43F6CF; Thu, 10 Jan 2019 07:49:33 -0800 (PST) Date: Thu, 10 Jan 2019 15:49:00 -0000 From: James Greenhalgh To: Ramana Radhakrishnan Cc: Richard Earnshaw , Marcus Shawcroft , "gcc-patches@gcc.gnu.org" , Ard Biesheuvel , Will Deacon , Mark Rutland , nd Subject: Re: [RFC][AArch64] Add support for system register based stack protector canary access Message-ID: <20190110154927.GA37554@arm.com> References: <7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00557.txt.bz2 On Mon, Dec 03, 2018 at 03:55:36AM -0600, Ramana Radhakrishnan wrote: > For quite sometime the kernel guys, (more specifically Ard) have been > talking about using a system register (sp_el0) and an offset from that > for a canary based access. This patchset adds support for a new set of > command line options similar to how powerpc has done this. > > I don't intend to change the defaults in userland, we've discussed this > for user-land in the past and as far as glibc and userland is concerned > we stick to the options as currently existing. The system register > option is really for the kernel to use along with an offset as they > control their ABI and this is a decision for them to make. > > I did consider sticking this all under a mcmodel=kernel-small option but > thought that would be a bit too aggressive. There is very little error > checking I can do in terms of the system register being used and really > the assembler would barf quite quickly in case things go wrong. I've > managed to rebuild Ard's kernel tree with an additional patch that > I will send to him. I haven't managed to boot this kernel. > > There was an additional question asked about the performance > characteristics of this but it's a security feature and the kernel > doesn't have the luxury of a hidden symbol. Further since the kernel > uses sp_el0 for access everywhere and if they choose to use the same > register I don't think the performance characteristics would be too bad, > but that's a decision for the kernel folks to make when taking in the > feature into the kernel. > > I still need to add some tests and documentation in invoke.texi but > this is at the stage where it would be nice for some other folks > to look at this. > > The difference in code generated is as below. > > extern void bar (char *); > int foo (void) > { > char a[100]; > bar (&a); > } > > $GCC -O2 -fstack-protector-strong vs > -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard=sysreg > -mstack-protector-guard-offset=1024 -fstack-protector-strong > > > --- tst.s 2018-12-03 09:46:21.174167443 +0000 > +++ tst.s.1 2018-12-03 09:46:03.546257203 +0000 > @@ -15,15 +15,14 @@ > mov x29, sp > str x19, [sp, 16] > .cfi_offset 19, -128 > - adrp x19, __stack_chk_guard > - add x19, x19, :lo12:__stack_chk_guard > - ldr x0, [x19] > - str x0, [sp, 136] > - mov x0,0 > + mrs x19, sp_el0 > add x0, sp, 32 > + ldr x1, [x19, 1024] > + str x1, [sp, 136] > + mov x1,0 > bl bar > ldr x0, [sp, 136] > - ldr x1, [x19] > + ldr x1, [x19, 1024] > eor x1, x0, x1 > cbnz x1, .L5 > > > > > I will be afk tomorrow and day after but this is to elicit some comments > and for Ard to try this out with his kernel patches. > > Thoughts ? I didn't see ananswer on list to Ard's questions about the command-line logic. Remember to also fix up the error message concerns Florian raised. That said, if Jakub is happy with this in Stage 4, I am too. My biggest concern is the -mstack-protector-guard-reg interface, which is unchecked user input and so opens up nasty ways to force the compiler towards out of bounds accesses (e.g. -mstack-protector-guard-reg="What memory is at %10") Thanks, James > > regards > Ramana > > gcc/ChangeLog: > > 2018-11-23 Ramana Radhakrishnan > > * config/aarch64/aarch64-opts.h (enum stack_protector_guard): New > * config/aarch64/aarch64.c (aarch64_override_options_internal): > Handle > and put in error checks for stack protector guard options. > (aarch64_stack_protect_guard): New. > (TARGET_STACK_PROTECT_GUARD): Define. > * config/aarch64/aarch64.md (UNSPEC_SSP_SYSREG): New. > (reg_stack_protect_address): New. > (stack_protect_set): Adjust for SSP_GLOBAL. > (stack_protect_test): Likewise. > * config/aarch64/aarch64.opt (-mstack-protector-guard-reg): New. > (-mstack-protector-guard): Likewise. > (-mstack-protector-guard-offset): Likewise. > * doc/invoke.texi: Document new AArch64 options.