From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1755059AbcEQKU4 (ORCPT <rfc822;w@1wt.eu>);
	Tue, 17 May 2016 06:20:56 -0400
Received: from mail-wm0-f51.google.com ([74.125.82.51]:38043 "EHLO
	mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751169AbcEQKUz (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 17 May 2016 06:20:55 -0400
Date: Tue, 17 May 2016 12:20:49 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Alex Thorlton <athorlton@sgi.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>, "H. Peter Anvin" <hpa@zytor.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Borislav Petkov <bp@alien8.de>, Josh Poimboeuf <jpoimboe@redhat.com>,
        Andy Lutomirski <luto@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Denys Vlasenko <dvlasenk@redhat.com>
Subject: Re: [GIT PULL] EFI fix
Message-ID: <20160517102049.GB26924@gmail.com>
References: <20160516144648.GA22999@gmail.com>
 <CA+55aFzBS2v=WnEH83cUDg7XkOremFqJ30BJwF40dCYjReBkUQ@mail.gmail.com>
 <20160517090434.GA21993@codeblueprint.co.uk>
 <20160517094655.GB21993@codeblueprint.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160517094655.GB21993@codeblueprint.co.uk>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> On Tue, 17 May, at 10:04:34AM, Matt Fleming wrote:
> > 
> > Now I'm wondering whether other users of FRAME_BEGIN/FRAME_END make
> > this same mistake. Coccinelle might be able to detect it perhaps.
> 
> A quick bit of sed turned up the code in arch/x86/entry/entry_64.S,
> which looks to suffer from the same bug,

That would be arch/x86/entry/thunk_64.S, but yeah, good find!

>         /* rdi: arg1 ... normal C conventions. rax is saved/restored. */
>         .macro THUNK name, func, put_ret_addr_in_rdi=0
>         .globl \name
>         .type \name, @function
> \name:
>         FRAME_BEGIN
> 
>         /* this one pushes 9 elems, the next one would be %rIP */
>         pushq %rdi
>         pushq %rsi
>         pushq %rdx
>         pushq %rcx
>         pushq %rax
>         pushq %r8
>         pushq %r9
>         pushq %r10
>         pushq %r11
> 
>         .if \put_ret_addr_in_rdi
>         /* 9*8(%rsp) is return addr on stack */
>         movq 9*8(%rsp), %rdi
>         .endif
> 
> With CONFIG_FRAME_POINTER=y 9*8(%rsp) is actually the value of %rbp on
> entry, not the return address.

This seems to affect:

#ifdef CONFIG_TRACE_IRQFLAGS
        THUNK trace_hardirqs_on_thunk,trace_hardirqs_on_caller,1
        THUNK trace_hardirqs_off_thunk,trace_hardirqs_off_caller,1
#endif

and with TRACE_IRQFLAGS we already in most times force frame pointers, such as 
when LOCKDEP is enabled:

  config LOCKDEP
        bool
        depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
  ...
        select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !SCORE

Also, unlike the efi_call() case, this thunk_64.S bug affects the quality of 
debug/tracing information, not runtime correctness per se.

still all that is by accident, not by design - this bug should be fixed too.

Thanks,

	Ingo