Re: [PATCH] x86/asm/entry: fix stack return address retrieval in thunk

From	Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH] x86/asm/entry: fix stack return address retrieval in thunk
Date	2016-05-17 18:40 +0200
Message-ID	<rzQch-6VD-1@gated-at.bofh.it> (permalink)
References	(1 earlier) <rzwZY-3eP-9@gated-at.bofh.it> <rzJaT-2C9-15@gated-at.bofh.it> <rzJNv-2Rf-1@gated-at.bofh.it> <rzKqe-3kb-9@gated-at.bofh.it> <rzOtQ-5Nu-15@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Tue, May 17, 2016 at 7:43 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> index 98df1fa..dae7ca0 100644
> --- a/arch/x86/entry/thunk_64.S
> +++ b/arch/x86/entry/thunk_64.S
> @@ -15,9 +15,10 @@
>         .globl \name
>         .type \name, @function
>  \name:
> +       /* push 1 register if frame pointers are enabled */
>         FRAME_BEGIN
>
> -       /* this one pushes 9 elems, the next one would be %rIP */
> +       /* push 9 registers */

I don't hate this patch, but quite frankly, as with the other case,
I'd just make the frame pointer be unconditional in this case.

If we push nine other registers, the frame pointer setup code is *not*
going to matter.

The reason to avoid frame pointers in code generation is two-fold:

 1) for small leaf functions, it often ends up dominating

 2) it removes a register that is otherwise usable, which can be
particularly bad on 32-bit x86 due to the much more limited number of
registers (and was apparently really noticeable on the older on-order
atom cores)

and in this case neither of them is really an issue.

So I would suggest that any case that actually depends on a frame
access just make the frame pointer not just unconditional, but
_explicit_.

So not just avoiding the macro because it's conditional, but write out
the sequence to actually set up the frame, and then use

-       movq 9*8(%rsp), %rdi
+       movq 8(%rbp), %rdi   # return address

to entirely avoid all kind of "how many registers have we pushed" math.

Considering that we got this wrong in two places, it's clearly too
subtle for our little brains as-is.

               Linus

Thread

[GIT PULL] EFI fix Ingo Molnar <mingo@kernel.org> - 2016-05-16 16:50 +0200
  Re: [GIT PULL] EFI fix Linus Torvalds <torvalds@linux-foundation.org> - 2016-05-16 22:10 +0200
    Re: [GIT PULL] EFI fix Alex Thorlton <athorlton@sgi.com> - 2016-05-16 22:30 +0200
      Re: [GIT PULL] EFI fix Alex Thorlton <athorlton@sgi.com> - 2016-05-17 00:50 +0200
    [tip:x86/urgent] x86/efi: Fix 7-parameter efi_call()s tip-bot for Linus Torvalds <tipbot@zytor.com> - 2016-05-17 08:40 +0200
    Re: [GIT PULL] EFI fix Matt Fleming <matt@codeblueprint.co.uk> - 2016-05-17 11:10 +0200
      Re: [GIT PULL] EFI fix Matt Fleming <matt@codeblueprint.co.uk> - 2016-05-17 11:50 +0200
        Re: [GIT PULL] EFI fix Ingo Molnar <mingo@kernel.org> - 2016-05-17 12:30 +0200
          [PATCH] x86/asm/entry: fix stack return address retrieval in thunk Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-17 16:50 +0200
            Re: [PATCH] x86/asm/entry: fix stack return address retrieval in thunk Linus Torvalds <torvalds@linux-foundation.org> - 2016-05-17 18:40 +0200
              Re: [PATCH] x86/asm/entry: fix stack return address retrieval in  thunk Steven Rostedt <rostedt@goodmis.org> - 2016-05-17 19:00 +0200
                Re: [PATCH] x86/asm/entry: fix stack return address retrieval in thunk Linus Torvalds <torvalds@linux-foundation.org> - 2016-05-17 19:30 +0200
                Re: [PATCH] x86/asm/entry: fix stack return address retrieval in  thunk Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-17 19:30 +0200
              [PATCH v2] x86/asm/entry: fix stack return address retrieval in thunk Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-17 20:10 +0200
                Re: [PATCH v2] x86/asm/entry: fix stack return address retrieval in thunk Linus Torvalds <torvalds@linux-foundation.org> - 2016-05-17 20:40 +0200
                [tip:x86/urgent] x86/entry/64: Fix stack return address retrieval  in thunk tip-bot for Josh Poimboeuf <tipbot@zytor.com> - 2016-05-19 11:20 +0200
    Re: [GIT PULL] EFI fix Matt Fleming <matt@codeblueprint.co.uk> - 2016-05-23 14:10 +0200
      Re: [GIT PULL] EFI fix Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-23 14:40 +0200
        Re: [GIT PULL] EFI fix Ingo Molnar <mingo@kernel.org> - 2016-05-24 11:10 +0200

csiph-web