From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23705 invoked by alias); 3 Dec 2018 16:40:13 -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 23692 invoked by uid 89); 3 Dec 2018 16:40:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.4 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SEM_URI,SEM_URIRED,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:3891, irrelevant, Hx-spam-relays-external:209.85.166.65, H*RU:209.85.166.65 X-HELO: mail-io1-f65.google.com Received: from mail-io1-f65.google.com (HELO mail-io1-f65.google.com) (209.85.166.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Dec 2018 16:40:08 +0000 Received: by mail-io1-f65.google.com with SMTP id n9so11010841ioh.7 for ; Mon, 03 Dec 2018 08:40:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=JpF7MGnJ5hg8r0BVWu9mc8fD8z+8bXysHJ5CljWOXZY=; b=TGZEv1y/05TwqxiPJejTP83CE7jboZJ+3kkQ3n10gyzQHP02PlQnm82zU9kEtfGvEM w8CsCBcX2ErpFUfIv58ZsCYHV8Dk2QpHiswDh4WHrxKVxGwNgC62XV5GSEUR8cG4eVFW gHpojA95Mo4FY5kP4h3Qbk0H+4kTTYKMRCB6A= MIME-Version: 1.0 References: <7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com> In-Reply-To: <7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com> From: Ard Biesheuvel Date: Mon, 03 Dec 2018 16:40:00 -0000 Message-ID: Subject: Re: [RFC][AArch64] Add support for system register based stack protector canary access To: Ramana Radhakrishnan Cc: James.Greenhalgh@arm.com, Richard Earnshaw , Marcus Shawcroft , gcc-patches@gcc.gnu.org, Will Deacon , Mark Rutland , nd Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-12/txt/msg00103.txt.bz2 On Mon, 3 Dec 2018 at 10:55, 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. > Thanks Ramana. I managed to build and run a complete kernel (including modules) on a bare metal system, and everything works as expected. The only thing I'd like to confirm with you is the logic wrt the command line arguments, more specifically, if/when all 3 arguments have to appear, and whether they are permitted to appear if -fstack-protector is not set. This is relevant given that we invoke the compiler in 3 different ways: - at the configure stage, we invoke the compiler with some/all of these options to decide whether the feature is supported, but the actual offset is not known, but also irrelevant - we invoke the compiler to build the header file that actually gives us the offset to pass to later invocations - finally, all kernel objects are built with all 3 arguments passed on the command line It looks like your code permits -mstack-protector-guard-reg at any time, but only permits -mstack-protector-guard-offset if -mstack-protector-guard is set to sysreg (and thus set explicitly, since the default is global). Is that intentional? Can we expect this to remain like that?