[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrU-aOe9k=NKG3h_SXfB2Zwr_Sxj6moLhEMhLqgM2V2qTg@mail.gmail.com>
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