From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76699 invoked by alias); 10 Jan 2019 10:53:47 -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 76685 invoked by uid 89); 10 Jan 2019 10:53:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=luxury, sk:UNSPEC_, sk:unspec_, Hx-languages-length:3844 X-HELO: mail-vs1-f52.google.com Received: from mail-vs1-f52.google.com (HELO mail-vs1-f52.google.com) (209.85.217.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 10 Jan 2019 10:53:45 +0000 Received: by mail-vs1-f52.google.com with SMTP id v205so6726956vsc.3 for ; Thu, 10 Jan 2019 02:53:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9KGC3TdhhSkPKhn1UuiCyKgMV2FzwxSGTxNnqmX8PKE=; b=K8pKUISZwBeT3Kxip2nfWZ0JUFRXFccrIMAoKU8v+nfhA/DYGIGyCtNK4VdqifkbSo 5gM/IA2JY6kUB/CLHdEJ8z0BBZthpaZvHLKWSjnyZi7susEhYN6o0jnbNEg3H14ojgQb GtUzG+/FXOkq7RFleNCaHI1QZPntHYCFi6CRBoPJTux0WeKEaIDN/7zQqv/gOmAYVUwD fSDudaJRKjySQLkxSiOjANaDgkCoVMZNGRVxKyDzAxS5DzkSsLPG2jQND8k9fekqAIsV +R44IjHKK0i4vh+O1azEwEHqlp5h9sxt/o7vkR+Tv5x2TRASzO5m+1c8EaACvvWZphr1 26yQ== MIME-Version: 1.0 References: <7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com> In-Reply-To: <7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com> From: Ramana Radhakrishnan Date: Thu, 10 Jan 2019 10:53:00 -0000 Message-ID: Subject: Re: [RFC][AArch64] Add support for system register based stack protector canary access To: Ramana Radhakrishnan Cc: James Greenhalgh , Richard Earnshaw , Marcus Shawcroft , "gcc-patches@gcc.gnu.org" , Ard Biesheuvel , Will Deacon , Mark Rutland , nd Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00527.txt.bz2 On Mon, Dec 3, 2018 at 9:55 AM 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 ? > > 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. Any further thoughts or is it just Jakub's comments that I need to address on this patch ? It looks like the kernel folks have queued this for the next kernel release and given this is helping the kernel with a security feature, can we move this forward ? Ramana