linux-kernel - Re: [PATCH V3 6/9] objtool: Report inconsistent stack changes in alternative

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: <806684aa-dcba-644e-540a-c88055b0d936@oracle.com>
Date:   Thu, 16 Apr 2020 16:43:18 +0200
From:   Alexandre Chartre <alexandre.chartre@...cle.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, jpoimboe@...hat.com,
        jthierry@...hat.com, tglx@...utronix.de
Subject: Re: [PATCH V3 6/9] objtool: Report inconsistent stack changes in
 alternative


On 4/16/20 4:18 PM, Peter Zijlstra wrote:
> On Tue, Apr 14, 2020 at 12:36:15PM +0200, Alexandre Chartre wrote:
>> To allow a valid stack unwinding, an alternative should have code
>> where the same stack changes happens at the same places as in the
>> original code. Add a check in objtool to validate that stack changes
>> in alternative are effectively consitent with the original code.
> 
> This thing is completely buggered, it warns all over the place, even for
> obviously correct alternatives like:
> 
> 0000000000000310 <return_to_handler>:
>   310:   48 83 ec 18             sub    $0x18,%rsp
>   314:   48 89 04 24             mov    %rax,(%rsp)
>   318:   48 89 54 24 08          mov    %rdx,0x8(%rsp)
>   31d:   48 89 ef                mov    %rbp,%rdi
>   320:   e8 00 00 00 00          callq  325 <return_to_handler+0x15>
>                          321: R_X86_64_PLT32     ftrace_return_to_handler-0x4
>   325:   48 89 c7                mov    %rax,%rdi
>   328:   48 8b 54 24 08          mov    0x8(%rsp),%rdx
>   32d:   48 8b 04 24             mov    (%rsp),%rax
>   331:   48 83 c4 18             add    $0x18,%rsp
>   335:   ff e7                   jmpq   *%rdi
>   337:   90                      nop
>   338:   90                      nop
>   339:   90                      nop
> 
> 
> Where 335 has two alternatives:
> 
>     0:   e9 00 00 00 00          jmpq   5 <.altinstr_replacement+0x5>
>                          1: R_X86_64_PLT32       __x86_retpoline_rdi-0x4
> 
> and
> 
>     5:   0f ae e8                lfence
>     8:   ff e7                   jmpq   *%rdi
> 
> 
> And it then comes back with:
> 
>    defconfig-build/arch/x86/kernel/ftrace_64.o: warning: objtool: .entry.text+0x335: error in alternative
>    defconfig-build/arch/x86/kernel/ftrace_64.o: warning: objtool: .altinstr_replacement+0x5: in alternative 2
>    defconfig-build/arch/x86/kernel/ftrace_64.o: warning: objtool: .altinstr_replacement+0x8: misaligned alternative state change
> 
> which is just utter crap, JMP has no (CFI) state change.

I think that's because the original nop instructions are not reached, so
they probably have an undefined stack state. Hence there's a different
stack state between 335 (jmpq *%rdi) and 337 (nop).

With the alternative, we have:

335:         lfence
338:         jmpq *%rdi

So now instruction 338 is reached with the stack state we had at 335
which is different from the original stack state at 338 (undefined)
with the unreached instruction.

Don't we have a state change recorded at 337 because the instruction
is seen as not reached?


alex.