linux-kernel - Re: [PATCH v2 04/20] gcc-plugins: Add the randstruct plugin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jJHRC83YSDKVfm4_41k0wgqfG=wpqPQss-ce77SRNkwRA@mail.gmail.com>
Date:   Thu, 29 Jun 2017 15:53:02 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Arnd Bergmann <arnd@...db.de>
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>
Subject: Re: [PATCH v2 04/20] gcc-plugins: Add the randstruct plugin

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.)

> /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".

> /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 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.

Can you verify that reverting "task_struct: Allow randomized layout"
fixes a bugged build for you?

-Kees

-- 
Kees Cook
Pixel Security