[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 30 Jun 2017 09:35:12 +0200
From: Arnd Bergmann <arnd@...db.de>
To: Kees Cook <keescook@...omium.org>
Cc: Kernel Hardening <kernel-hardening@...ts.openwall.com>, Laura Abbott <labbott@...hat.com>,
"the arch/x86 maintainers" <x86@...nel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
Russell King - ARM Linux <linux@...linux.org.uk>, Nicolas Pitre <nicolas.pitre@...aro.org>,
Will Deacon <will.deacon@....com>
Subject: Re: [PATCH v2 04/20] gcc-plugins: Add the randstruct plugin
On Fri, Jun 30, 2017 at 12:53 AM, Kees Cook <keescook@...omium.org> wrote:
> On Thu, Jun 29, 2017 at 3:08 PM, Arnd Bergmann <arnd@...db.de> wrote:
>> On Fri, May 26, 2017 at 10:17 PM, Kees Cook <keescook@...omium.org> wrote:
>> I noticed new build errors that bisected back to this patch, which has
>> now showed up
>> in linux-next again:
>
> (FWIW this is randstruct not initify, and has been in -next for a
> couple weeks now.)
I first saw it last week and only now got around to looking any deeper,
as I had assumed that one of my own patches caused it.
>> /git/arm-soc/arch/arm/kernel/entry-armv.S: Assembler messages:
>> /git/arm-soc/arch/arm/kernel/entry-armv.S:800: Error: bad immediate
>> value for offset (4644)
>> /git/arm-soc/scripts/Makefile.build:403: recipe for target
>> 'arch/arm/kernel/entry-armv.o' failed
>> make[3]: *** [arch/arm/kernel/entry-armv.o] Error 1
>> /git/arm-soc/arch/arm/kernel/entry-armv.S: Assembler messages:
>> /git/arm-soc/arch/arm/kernel/entry-armv.S:800: Error: bad immediate
>> value for offset (5584)
>
> arch/arm/kernel/entry-armv.S: ldr r7, [r7, #TSK_STACK_CANARY]
> arch/arm/kernel/asm-offsets.c: DEFINE(TSK_STACK_CANARY,
> offsetof(struct task_struct, stack_canary));
>
> This would imply that stack_canary got randomized to an offset within
> struct task_struct beyond the "ldr" immediate range (4096). Yay for
> giant structs.
>
> I'm surprised this didn't bisect to "task_struct: Allow randomized layout".
The bisection was a bit tricky, it's very possible that this should have
been the one to report.
>> /git/arm-soc/scripts/Makefile.build:403: recipe for target
>> 'arch/arm/kernel/entry-armv.o' failed
>> make[3]: *** [arch/arm/kernel/entry-armv.o] Error 1
>> /git/arm-soc/arch/arm/mm/tlb-v4.S: Assembler messages:
>> /git/arm-soc/arch/arm/mm/tlb-v4.S:35: Error: bad immediate value for
>> offset (4928)
>
> Similar:
>
> act_mm r3 @ get current->active_mm
> ...
> .macro act_mm, rd
> ldr \rd, [\rd, #TSK_ACTIVE_MM]
> ...
> kernel/asm-offsets.c: DEFINE(TSK_ACTIVE_MM,
> offsetof(struct task_struct, active_mm));
>
>> /git/arm-soc/scripts/Makefile.build:403: recipe for target
>> 'arch/arm/mm/tlb-v4.o' failed
>> make[3]: *** [arch/arm/mm/tlb-v4.o] Error 1
>> /git/arm-soc/arch/arm/mm/tlb-v4wbi.S: Assembler messages:
>> /git/arm-soc/arch/arm/mm/tlb-v4wbi.S:34: Error: bad immediate value
>> for offset (4928)
>> /git/arm-soc/scripts/Makefile.build:403: recipe for target
>> 'arch/arm/mm/tlb-v4wbi.o' failed
>
> Same as above.
>
>> So far, that's the only thing that goes wrong for me though, and this
>> is probably
>> easy to fix.
>
> Thanks for letting me know! These haven't shown up in my tests since I
> haven't gotten "unlucky" in randomizing the task_struct, it seems.
I've only hit it a couple of times a few thousand builds.
> I see a few possible solutions:
>
> - ignore it and try your build again with a fresh tree and a new
> randomization seed ;)
> - remove "depends on !COMPILE_TEST" from
> GCC_PLUGIN_RANDSTRUCT_PERFORMANCE, which will leave most stuff near
> their original locations
> - add a new annotation __randomize_cacheline which performs the same
> logic as above, but only for the marked structure
> - build new logic to keep certain fields (with some special marking)
> within a given range of their original position
> - rewrite the ARM code to handle larger immediates
>
> The first obviously won't fly. The second just bypasses the problem
> forcing it to be exposed by other people later. The third is likely
> easiest to do now, but reduces the effectiveness of randomization for
> architectures that don't have sensitive immediate values. The fourth
> sounds not generally useful. The fifth may be unacceptable to arm
> maintainers due to performance impacts.
I was thinking of the fifth solution, but don't know exactly how to
do it. If performance is a concern, I guess we could have separate
implementations for randstruct and traditional builds.
I've added a few more people to Cc that may know exactly how to
do it right.
> Can you verify that reverting "task_struct: Allow randomized layout"
> fixes a bugged build for you?
Confirmed.
Arnd
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
