linux-kernel - Re: [PATCH v2 10/10] objtool: More complex static jump implementation

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]

Date:   Tue, 16 Jan 2018 21:05:31 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     David Woodhouse <dwmw2@...radead.org>,
        linux-kernel@...r.kernel.org, Dave Hansen <dave.hansen@...el.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jun Nakajima <jun.nakajima@...el.com>,
        Asit Mallick <asit.k.mallick@...el.com>
Subject: Re: [PATCH v2 10/10] objtool: More complex static jump implementation

On Tue, Jan 16, 2018 at 03:28:35PM +0100, Peter Zijlstra wrote:
> When using something like:
> 
>   -#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
>   +#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]) && \
>   +			(arch_static_assert(), true))
> 
> we get an objtool assertion fail like:
> 
> kernel/sched/fair.o: warning: objtool: hrtick_update()+0xd: static assert FAIL
> 
> where:
> 
> 0000000000001140 <hrtick_update>:
>     1140:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
>     1145:       c3                      retq
>     1146:       48 8b b7 30 09 00 00    mov    0x930(%rdi),%rsi
>     114d:       8b 87 d8 09 00 00       mov    0x9d8(%rdi),%eax
>     1153:       48 0f a3 05 00 00 00    bt     %rax,0x0(%rip)        # 115b <hrtick_update+0x1b>
>     115a:       00
>                         1157: R_X86_64_PC32     __cpu_active_mask-0x4
> 
> and:
> 
> RELOCATION RECORDS FOR [__jump_table]:
> 0000000000000150 R_X86_64_64       .text+0x0000000000001140
> 0000000000000158 R_X86_64_64       .text+0x0000000000001146
> 
> RELOCATION RECORDS FOR [.discard.jump_assert]:
> 0000000000000028 R_X86_64_64       .text+0x000000000000114d
> 
> IOW, GCC managed to place the assertion 1 instruction _after_ the
> static jump target.
> 
> So while the code generation is fine, the assertion gets placed wrong.
> We can 'fix' this by not only considering the immediate static jump
> locations but also all the unconditional code after it, terminating
> the basic block on any unconditional instruction or branch entry
> point.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>

This is pretty similar to something I've been wanting to do, which is to
track all basic blocks.  But this is fine enough for now.

One nit, can you rename "branch_target" to "jump_dest" for consistency
with the existing naming?

-- 
Josh