Re: WARNING: can't dereference registers at ffffc90004dfff60 for ip error_entry+0x7d/0xd0 (Re: [PATCH v2 00/18] Entry stack switching)

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>, X86 ML <x86@kernel.org>,
	Borislav Petkov <bpetkov@suse.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Brian Gerst <brgerst@gmail.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Linus Torvalds <torvalds@linux-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)
Date: Wed, 22 Nov 2017 07:55:50 -0600	[thread overview]
Message-ID: <20171122135550.ckgtmzjnwydevkln@treble> (raw)
In-Reply-To: <20171122073907.mg6yayycfyvf4x4k@gmail.com>

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
