On 03/12/2018 16:39, Ard Biesheuvel wrote:
> On Mon, 3 Dec 2018 at 10:55, Ramana Radhakrishnan
> <ramana.radhakrish...@arm.com> 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.
They are permitted to appear without -fstack-protector even though it
doesn't make much sense ...
>
> 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?
It doesn't make sense to permit an offset if the stack protector guard
is a global variable.
If the default changes to sysreg which I doubt, then I would expect
-mstack-protector-guard-offset to be useable without
-mstack-protector-guard=sysreg . However changing the default is not
something I'm sure we have the appetite for yet in user land. The
decision was made in 2015 that for user land the stack protector guard
would be a hidden symbol and I expect there to be quite a lot of
protracted discussion before changing this.
regards
Ramana
>
