Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753818AbdF2THF (ORCPT ); Thu, 29 Jun 2017 15:07:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50040 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753766AbdF2TGG (ORCPT ); Thu, 29 Jun 2017 15:06:06 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E7972145FA7 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jpoimboe@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E7972145FA7 Date: Thu, 29 Jun 2017 14:05:59 -0500 From: Josh Poimboeuf To: Andy Lutomirski Cc: X86 ML , "linux-kernel@vger.kernel.org" , live-patching@vger.kernel.org, Linus Torvalds , Jiri Slaby , Ingo Molnar , "H. Peter Anvin" , Peter Zijlstra , Mike Galbraith Subject: Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Message-ID: <20170629190559.ttw52ahwtsjynayx@treble> References: <20170629175333.bicpvbwo4d5pdbak@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 29 Jun 2017 19:06:01 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2045 Lines: 44 On Thu, Jun 29, 2017 at 11:50:18AM -0700, Andy Lutomirski wrote: > On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf wrote: > > There's a bug here that will need a small change to the entry code. > > > > Mike Galbraith reported: > > > > WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb > > > > After some looking I found that it's caused by the following code > > snippet in the 'interrupt' macro in entry_64.S: > > > > /* > > * Save previous stack pointer, optionally switch to interrupt stack. > > * irq_count is used to check if a CPU is already on an interrupt stack > > * or not. While this is essentially redundant with preempt_count it is > > * a little cheaper to use a separate counter in the PDA (short of > > * moving irq_enter into assembly, which would be too much work) > > */ > > movq %rsp, %rdi > > incl PER_CPU_VAR(irq_count) > > cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp > > UNWIND_HINT_REGS base=rdi > > pushq %rdi > > UNWIND_HINT_REGS indirect=1 > > > > The problem is that it's changing the stack pointer *before* writing the > > previous stack pointer (push %rdi). So when unwinding from an NMI which > > hit between the rsp write and the rdi push, the unwinder tries to access > > the regs on the previous stack (by reading rdi), but the previous stack > > pointer isn't there yet, so the access is considered out of bounds. > > Ugh, that code. Does this problem go away with this patch applied: > > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1 > > If so, want to update the patch for new kernels (shouldn't conflict > with anything except your unwind hints)? I don't think that patch will fix it, because it still updates rsp *before* writing the old rsp on the new stack. So there's still a window where the "previous stack" pointer is missing. -- Josh