linux-kernel - Re: [PATCH v2 10/36] x86/vdso: Define BUILD_VDSO while building and emit .eh

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: <20151009072140.GA31859@gmail.com>
Date:	Fri, 9 Oct 2015 09:21:40 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Andy Lutomirski <luto@...nel.org>
Cc:	x86@...nel.org, linux-kernel@...r.kernel.org,
	Brian Gerst <brgerst@...il.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v2 10/36] x86/vdso: Define BUILD_VDSO while building and
 emit .eh_frame in asm


* Andy Lutomirski <luto@...nel.org> wrote:

> For the vDSO, user code wants runtime unwind info.  Make sure that,
> if we use .cfi directives, we generate it.
> 
> Signed-off-by: Andy Lutomirski <luto@...nel.org>
> ---
>  arch/x86/entry/vdso/Makefile  |  4 ++--
>  arch/x86/include/asm/dwarf2.h | 13 ++++++++++---
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
> index 3bfb39e7b8b2..265c0ed68118 100644
> --- a/arch/x86/entry/vdso/Makefile
> +++ b/arch/x86/entry/vdso/Makefile
> @@ -67,7 +67,7 @@ $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
>  CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
>         $(filter -g%,$(KBUILD_CFLAGS)) $(call cc-option, -fno-stack-protector) \
>         -fno-omit-frame-pointer -foptimize-sibling-calls \
> -       -DDISABLE_BRANCH_PROFILING
> +       -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
>  
>  $(vobjs): KBUILD_CFLAGS += $(CFL)
>  
> @@ -131,7 +131,7 @@ targets += vdso32/vdso32.lds
>  targets += vdso32/note.o vdso32/vclock_gettime.o vdso32/system_call.o
>  targets += vdso32/vclock_gettime.o
>  
> -KBUILD_AFLAGS_32 := $(filter-out -m64,$(KBUILD_AFLAGS))
> +KBUILD_AFLAGS_32 := $(filter-out -m64,$(KBUILD_AFLAGS)) -DBUILD_VDSO
>  $(obj)/vdso32.so.dbg: KBUILD_AFLAGS = $(KBUILD_AFLAGS_32)
>  $(obj)/vdso32.so.dbg: asflags-$(CONFIG_X86_64) += -m32
>  
> diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h
> index de1cdaf4d743..09133ba032b3 100644
> --- a/arch/x86/include/asm/dwarf2.h
> +++ b/arch/x86/include/asm/dwarf2.h
> @@ -36,15 +36,22 @@
>  #endif
>  
>  #if defined(CONFIG_AS_CFI_SECTIONS) && defined(__ASSEMBLY__)
> +#ifndef BUILD_VDSO
>  	/*
>  	 * Emit CFI data in .debug_frame sections, not .eh_frame sections.
>  	 * The latter we currently just discard since we don't do DWARF
>  	 * unwinding at runtime.  So only the offline DWARF information is
> -	 * useful to anyone.  Note we should not use this directive if this
> -	 * file is used in the vDSO assembly, or if vmlinux.lds.S gets
> -	 * changed so it doesn't discard .eh_frame.
> +	 * useful to anyone.  Note we should not use this directive if
> +	 * vmlinux.lds.S gets changed so it doesn't discard .eh_frame.
>  	 */
>  	.cfi_sections .debug_frame
> +#else
> +	 /*
> +	  * For the vDSO, emit both runtime unwind information and debug
> +	  * symbols for the .dbg file.
> +	  */
> +	.cfi_sections .eh_frame, .debug_frame
> +#endif
>  #endif

So it's exactly such hacks why I dislike the old CFI code:

1)

Emitting assembler directives from .h files is just disgusting. What's wrong with 
adding it to the .c file, so that it becomes obvious to everyone what's going on? 
Or if it's possible, do it in the build command line - the vDSO is built in a 
different way anyway.

2)

Could we require that to build the vDSO you need CFI capable tooling? All other 
tooling is probably on very ancient systems which can live with only having the 
vsyscall and the regular int80 entry methods, or something like that.

So could you please clean this all up instead of reintroducing old hacks? I'll 
apply the patches and see how well they work to move things forward, but this part 
is disgusting and needs to be resolved.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/