Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755279AbbHNOHS (ORCPT ); Fri, 14 Aug 2015 10:07:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37861 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754179AbbHNOHQ (ORCPT ); Fri, 14 Aug 2015 10:07:16 -0400 Date: Fri, 14 Aug 2015 09:07:12 -0500 From: Josh Poimboeuf To: Matt Fleming Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Michal Marek , Peter Zijlstra , Andy Lutomirski , Borislav Petkov , Linus Torvalds , Andi Kleen , Pedro Alves , Namhyung Kim , Bernd Petrovitsch , Chris J Arges , live-patching@vger.kernel.org, Matt Fleming Subject: Re: [PATCH v10 19/20] x86/asm/efi: Create a stack frame in efi_call() Message-ID: <20150814140712.GB341@treble.redhat.com> References: <89cbfadfe41061aeeacb56a4b384adf82f272e97.1439521412.git.jpoimboe@redhat.com> <20150814091159.GA2865@codeblueprint.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150814091159.GA2865@codeblueprint.co.uk> User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3166 Lines: 87 On Fri, Aug 14, 2015 at 10:11:59AM +0100, Matt Fleming wrote: > On Thu, 13 Aug, at 10:10:40PM, Josh Poimboeuf wrote: > > efi_call() is a callable non-leaf function which doesn't honor > > CONFIG_FRAME_POINTER, which can result in bad stack traces. > > > > Create a stack frame for it when CONFIG_FRAME_POINTER is enabled. > > > > Signed-off-by: Josh Poimboeuf > > Cc: Matt Fleming > > --- > > arch/x86/platform/efi/efi_stub_64.S | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/arch/x86/platform/efi/efi_stub_64.S b/arch/x86/platform/efi/efi_stub_64.S > > index 86d0f9e..0df2dcc 100644 > > --- a/arch/x86/platform/efi/efi_stub_64.S > > +++ b/arch/x86/platform/efi/efi_stub_64.S > > @@ -11,6 +11,7 @@ > > #include > > #include > > #include > > +#include > > > > #define SAVE_XMM \ > > mov %rsp, %rax; \ > > @@ -74,6 +75,7 @@ > > .endm > > > > ENTRY(efi_call) > > + FRAME_BEGIN > > SAVE_XMM > > mov (%rsp), %rax > > mov 8(%rax), %rax > > @@ -88,6 +90,7 @@ ENTRY(efi_call) > > RESTORE_PGT > > addq $48, %rsp > > RESTORE_XMM > > + FRAME_END > > ret > > ENDPROC(efi_call) > > You mention that stackvalidate will recursively validate the frame > pointers in all code paths. Since we're calling into firmware code from > efi_call(), we don't need to do anything special here right? > > I'm guessing stackvalidate would just stop since it has no way of > knowing the target address of the %call instruction, but I just wanted > to check (especially since the firmware ABI is different). It recursively follows all code paths *inside* each function, including jumps. It doesn't try to follow calls across functions, which can't be done reliably because of function pointers. Instead it just verifies that each function follows the calling conventions. The result of creating a stack frame is that the *caller* of the function shows up in the stack trace. So this patch ensures that efi_call()'s caller would show up if, for example, the stack was dumped from an interrupt which occurred in efi_call() between FRAME_BEGIN and FRAME_END. As to whether it helps in case the stack is dumped from firmware code (or from an interrupt to firmware), it really depends on what the firmware does: - If it follows frame pointer convention, great (but I'm guessing this is unlikely...) - If it doesn't follow frame pointer convention, but still leaves rbp alone, then efi_call() would be skipped in the stack trace but efi_call()'s caller and the rest of the stack would still show up. - If it trashes rbp, then we're out of luck and there's no stack trace. But regardless of what firmware does, this patch still helps in the case where the stack is dumped starting from efi_call(). > Reviewed-by: Matt Fleming Thanks! -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/