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:   Wed, 22 Nov 2017 07:19:18 -0800
From:   Andy Lutomirski <luto@...nel.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Ingo Molnar <mingo@...nel.org>, Andy Lutomirski <luto@...nel.org>,
        X86 ML <x86@...nel.org>, Borislav Petkov <bpetkov@...e.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Brian Gerst <brgerst@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: WARNING: can't dereference registers at ffffc90004dfff60 for ip
 error_entry+0x7d/0xd0 (Re: [PATCH v2 00/18] Entry stack switching)

On Wed, Nov 22, 2017 at 5:55 AM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> On Wed, Nov 22, 2017 at 08:39:07AM +0100, Ingo Molnar wrote:
>>
>> * Andy Lutomirski <luto@...nel.org> wrote:
>>
>> > This sets up stack switching, including for SYSCALL.  I think it's
>> > in decent shape.
>> >
>> > Known issues:
>> >  - I think we're going to want a way to turn the stack switching on and
>> >    off either at boot time or at runtime.  It should be fairly straightforward
>> >    to make it work.
>> >
>> >  - I think the ORC unwinder isn't so good at dealing with stack overflows.
>> >    It bails too early (I think), resulting in lots of ? entries.  This
>> >    isn't a regression with this series -- it's just something that could
>> >    be improved.
>>
>> Note that with the attached config on an Intel testbox I get the following new ORC
>> unwinder warning during bootup:
>>
>> [   12.200554] calling  ghash_pclmulqdqni_mod_init+0x0/0x54 @ 1
>> [   12.209536] WARNING: can't dereference registers at ffffc90004dfff60 for ip error_entry+0x7d/0xd0
>> [   12.231388] initcall ghash_pclmulqdqni_mod_init+0x0/0x54 returned 0 after 23480 usecs
>
> After the stack switch in error_entry(), the pt_regs are at a different
> offset than before, so they aren't where ORC expects them to be.
>
> I think the below patch should fix it, by popping the return address off
> the stack instead of just copying it.  This way we can avoid adding
> another ORC annotation because pt_regs will be at the same offset both
> before and after the sync_regs() call.
>
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 2fdd2127e8e9..8dad83724469 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -1291,8 +1291,8 @@ ENTRY(error_entry)
>
>  .Lerror_entry_from_usermode_after_swapgs:
>         /* Put us onto the real thread stack. */
> -       leaq    8(%rsp), %rdi                   /* pt_regs pointer */
> -       movq    (%rsp), %r12
> +       popq    %r12                            /* function return address */
> +       leaq    (%rsp), %rdi                    /* pt_regs pointer */
>         call    sync_regs
>         movq    %rax, %rsp                      /* switch stack */
>         ENCODE_FRAME_POINTER

Ah, I see -- SAVE_C_REGS contains an UNWIND_HINT_REGS hint.  I had
thought I needed to fix it differently.

Ingo, if you want to apply this, I attached it.  I'll fold it in next
time I resend the whole  series.

View attachment "0001-Fixup-for-x86-asm-64-Use-a-percpu-trampoline-stack-f.patch" of type "text/x-patch" (1120 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ