linux-kernel - Re: [PATCH] x86/build: Specify stack alignment for clang

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: <20170610070359.7pci2anwe3pvvcea@gmail.com>
Date:   Sat, 10 Jun 2017 09:03:59 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Matthias Kaehlcke <mka@...omium.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        "H . J . Lu" <hjl.tools@...il.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, Michael Davidson <md@...gle.com>,
        Greg Hackmann <ghackmann@...gle.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Stephen Hines <srhines@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Arnd Bergmann <arnd@...db.de>,
        Bernhard.Rosenkranzer@...aro.org,
        Peter Foley <pefoley2@...oley.com>,
        Behan Webster <behanw@...verseincode.com>,
        Douglas Anderson <dianders@...omium.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH] x86/build: Specify stack alignment for clang


* Matthias Kaehlcke <mka@...omium.org> wrote:

> For gcc stack alignment is configured with -mpreferred-stack-boundary=N,
> clang has the option -mstack-alignment=N for that purpose. Use the same
> alignment as for gcc.
> 
> If the alignment is not specified clang assumes an alignment of 16 bytes,
> as required by the standard ABI. However as mentioned in d9b0cde91c60
> ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if supported") the
> standard kernel entry on x86-64 leaves the stack on an 8-byte
> boundary, as a consequence clang will keep the stack misaligned.
> 
> Signed-off-by: Matthias Kaehlcke <mka@...omium.org>
> ---
>  arch/x86/Makefile | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 5851411e60fb..a32badbe87ad 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -27,7 +27,8 @@ REALMODE_CFLAGS	:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \
>  		   -mno-mmx -mno-sse \
>  		   $(call cc-option, -ffreestanding) \
>  		   $(call cc-option, -fno-stack-protector) \
> -		   $(call cc-option, -mpreferred-stack-boundary=2)
> +		   $(call cc-option, -mpreferred-stack-boundary=2) \
> +		   $(call cc-option, -mstack-alignment=2)
>  export REALMODE_CFLAGS
>  
>  # BITS is used as extension for files which are available in a 32 bit
> @@ -64,8 +65,9 @@ ifeq ($(CONFIG_X86_32),y)
>          # with nonstandard options
>          KBUILD_CFLAGS += -fno-pic
>  
> -        # prevent gcc from keeping the stack 16 byte aligned
> +        # prevent the compiler from keeping the stack 16 byte aligned
>          KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=2)
> +        KBUILD_CFLAGS += $(call cc-option,-mstack-alignment=2)
>  
>          # Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc use
>          # a lot more stack due to the lack of sharing of stacklots:
> @@ -97,8 +99,9 @@ else
>          KBUILD_CFLAGS += $(call cc-option,-mno-80387)
>          KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>  
> -	# Use -mpreferred-stack-boundary=3 if supported.
> +	# Align the stack to 8 bytes if supported.
>  	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
> +	KBUILD_CFLAGS += $(call cc-option,-mstack-alignment=3)
>  
>  	# Use -mskip-rax-setup if supported.
>  	KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup)

That's really ugly and the duplicated options are repeated. A cleaner solution 
would be introduce a variable that stores the option, and use that later on. Also 
describe the variable, pointing out that -mpreferred-stack-boundary is a GCC 
option, while -mstack-alignment is a Clang one.

BTW., GCC also has -mstack-align (note the different spelling), which does 
something else, so Clang's incompatibility is super confusing things...

Thanks,

	Ingo