linux-kernel - Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk

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: <20210111205814.m6bbvekdhqs7pnhr@google.com>
Date:   Mon, 11 Jan 2021 12:58:14 -0800
From:   Fangrui Song <maskray@...gle.com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Arnd Bergmann <arnd@...db.de>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Nathan Chancellor <natechancellor@...il.com>,
        linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com
Subject: Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk


On 2021-01-11, Nick Desaulniers wrote:
>Arnd found a randconfig that produces the warning:
>
>arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn at
>offset 0x3e
>
>when building with LLVM_IAS=1 (use Clang's integrated assembler). Josh
>notes:
>
>  With the LLVM assembler stripping the .text section symbol, objtool
>  has no way to reference this code when it generates ORC unwinder
>  entries, because this code is outside of any ELF function.
>
>Fangrui notes that this optimization is helpful for reducing images size
>when compiling with -ffunction-sections and -fdata-sections. I have
>observerd on the order of tens of thousands of symbols for the kernel
>images built with those flags. A patch has been authored against GNU
>binutils to match this behavior, with a new flag
>--generate-unused-section-symbols=[yes|no].

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d1bcae833b32f1408485ce69f844dcd7ded093a8
has been committed. The patch should be included in binutils 2.37.
The maintainers are welcome to the idea, but fixing all the arch-specific tests is tricky.

H.J. fixed the x86 tests and enabled this for x86. When binutils 2.37
come out, some other architectures may follow as well.

>We can omit the .L prefix on a label to emit an entry into the symbol
>table for the label, with STB_LOCAL binding.  This enables objtool to
>generate proper unwind info here with LLVM_IAS=1.

Josh, I think objtool orc generate needs to synthesize STT_SECTION
symbols even if they do not exist in object files.

rg 'SYM_CODE.*\.L' reveals a few other .S files which may have similar problems.

>Cc: Fangrui Song <maskray@...gle.com>
>Link: https://github.com/ClangBuiltLinux/linux/issues/1209
>Link: https://reviews.llvm.org/D93783
>Link: https://sourceware.org/binutils/docs/as/Symbol-Names.html
>Link: https://sourceware.org/pipermail/binutils/2020-December/114671.html
>Reported-by: Arnd Bergmann <arnd@...db.de>
>Suggested-by: Josh Poimboeuf <jpoimboe@...hat.com>
>Signed-off-by: Nick Desaulniers <ndesaulniers@...gle.com>
>---
>Changes v2 -> v3:
>* rework to use STB_LOCAL rather than STB_GLOBAL by dropping .L prefix,
>  as per Josh.
>* rename oneline to drop STB_GLOBAL in commit message.
>* add link to GAS docs on .L prefix.
>* drop Josh's ack since patch changed.
>
>Changes v1 -> v2:
>* Pick up Josh's Ack.
>* Add commit message info about -ffunction-sections/-fdata-sections, and
>  link to binutils patch.
>
>
> arch/x86/entry/thunk_64.S | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
>index ccd32877a3c4..c9a9fbf1655f 100644
>--- a/arch/x86/entry/thunk_64.S
>+++ b/arch/x86/entry/thunk_64.S
>@@ -31,7 +31,7 @@ SYM_FUNC_START_NOALIGN(\name)
> 	.endif
>
> 	call \func
>-	jmp  .L_restore
>+	jmp  __thunk_restore
> SYM_FUNC_END(\name)
> 	_ASM_NOKPROBE(\name)
> 	.endm
>@@ -44,7 +44,7 @@ SYM_FUNC_END(\name)
> #endif
>
> #ifdef CONFIG_PREEMPTION
>-SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
>+SYM_CODE_START_LOCAL_NOALIGN(__thunk_restore)
> 	popq %r11
> 	popq %r10
> 	popq %r9
>@@ -56,6 +56,6 @@ SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
> 	popq %rdi
> 	popq %rbp
> 	ret
>-	_ASM_NOKPROBE(.L_restore)
>-SYM_CODE_END(.L_restore)
>+	_ASM_NOKPROBE(__thunk_restore)
>+SYM_CODE_END(__thunk_restore)
> #endif
>-- 
>2.30.0.284.gd98b1dd5eaa7-goog
>