lkml.org 
[lkml]   [2017]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: 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@redhat.com> wrote:
> On Wed, Nov 22, 2017 at 08:39:07AM +0100, Ingo Molnar wrote:
>>
>> * Andy Lutomirski <luto@kernel.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.
From e4b840c5a2eee3610d870703d484263226df80cc Mon Sep 17 00:00:00 2001
Message-Id: <e4b840c5a2eee3610d870703d484263226df80cc.1511363929.git.luto@kernel.org>
From: Andy Lutomirski <luto@kernel.org>
Date: Wed, 22 Nov 2017 07:15:51 -0800
Subject: [PATCH] Fixup for "x86/asm/64: Use a percpu trampoline stack for IDT
entries"

One of my sync_regs calls was confusing objtool. I'll fold this in
to the series next time I refresh it.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/entry/entry_64.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 2fdd2127e8e9..f1cef194dfba 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 /* save return addr in %12 */
+ movq %rsp, %rdi /* arg0 = pt_regs pointer */
call sync_regs
movq %rax, %rsp /* switch stack */
ENCODE_FRAME_POINTER
--
2.13.6
\
 
 \ /
  Last update: 2017-11-22 16:19    [W:0.090 / U:2.980 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site