linux-kernel - Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170426014217.hshjlweqsimaumuy@treble>
Date:   Tue, 25 Apr 2017 20:42:17 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Jiri Slaby <jslaby@...e.cz>
Cc:     mingo@...hat.com, tglx@...utronix.de, hpa@...or.com,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>, xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

On Fri, Apr 21, 2017 at 04:12:40PM +0200, Jiri Slaby wrote:
> Somewhere END was used to end a function. It is not intended to be used
> for functions, because it does not mark the actual symbols as functions.
> Use ENDPROC in such cases which does the right job.
> 
> Signed-off-by: Jiri Slaby <jslaby@...e.cz>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: <x86@...nel.org>
> Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> Cc: Juergen Gross <jgross@...e.com>
> Reviewed-by: Juergen Gross <jgross@...e.com> [xen parts]
> Cc: <xen-devel@...ts.xenproject.org>
> ---
>  arch/x86/entry/entry_32.S        | 58 ++++++++++++++++++++--------------------
>  arch/x86/entry/entry_64.S        | 40 +++++++++++++--------------
>  arch/x86/entry/entry_64_compat.S |  4 +--
>  arch/x86/kernel/ftrace_32.S      |  8 +++---
>  arch/x86/kernel/ftrace_64.S      | 10 +++----
>  arch/x86/xen/xen-pvh.S           |  2 +-
>  6 files changed, 61 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index 50bc26949e9e..a546b84aec01 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -249,7 +249,7 @@ ENTRY(__switch_to_asm)
>  	popl	%ebp
>  
>  	jmp	__switch_to
> -END(__switch_to_asm)
> +ENDPROC(__switch_to_asm)
>  
>  /*
>   * A newly forked process directly context switches into this address.
> @@ -289,7 +289,7 @@ ENTRY(ret_from_fork)
>  	 */
>  	movl	$0, PT_EAX(%esp)
>  	jmp	2b
> -END(ret_from_fork)
> +ENDPROC(ret_from_fork)
>  
>  /*
>   * Return to user mode is not as complex as all this looks,
> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
>  	movl	%esp, %eax
>  	call	prepare_exit_to_usermode
>  	jmp	restore_all
> -END(ret_from_exception)
> +ENDPROC(ret_from_exception)

What exactly is the motivation of this patch?  It would be good to
describe that in the commit message.

Is the point to allow objtool to generate CFI for it?  If so, I don't
really see how that would work.  Today, objtool considers ENDPROC to
annotate a *callable* function which conforms to the C calling ABI and
can be called by another function.  The stack is in a known state at
function entry, and so the CFI (or frame pointer info) can be reliably
determined.

But entry code is different.  In most cases, the global symbols aren't
actually called, and they don't follow any conventions.  The code is
spaghetti-esque, with HW handlers and jumps everywhere.  The state of
the stack at symbol entry varies per "function".  That's why objtool
ignores these files.

For special cases (like entry code), I was thinking we'd need manual CFI
annotations, like we had before.  Or maybe there's another way, like
some new macros which tell objtool about the HW entry points and the
state of the registers there.

But I'm having trouble seeing how marking these code snippets with
ENTRY/ENDPROC would help objtool make any sense of the code and where
things are on the stack.

-- 
Josh