From: Josh Poimboeuf Add CFI annotations to the VDSO implementation of getrandom() so it will have valid DWARF unwinding metadata. Fixes: 33385150ac45 ("x86: vdso: Wire up getrandom() vDSO implementation") Signed-off-by: Josh Poimboeuf Signed-off-by: Steven Rostedt (Google) Acked-by: H. Peter Anvin (Intel) Signed-off-by: Jens Remus --- arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S b/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S index bcba5639b8ee..cc82da9216fb 100644 --- a/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S +++ b/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S @@ -4,7 +4,7 @@ */ #include -#include +#include .section .rodata, "a" .align 16 @@ -22,7 +22,7 @@ CONSTANTS: .octa 0x6b20657479622d323320646e61707865 * rcx: number of 64-byte blocks to write to output */ SYM_FUNC_START(__arch_chacha20_blocks_nostack) - + CFI_STARTPROC .set output, %rdi .set key, %rsi .set counter, %rdx @@ -175,4 +175,5 @@ SYM_FUNC_START(__arch_chacha20_blocks_nostack) pxor temp,temp ret + CFI_ENDPROC SYM_FUNC_END(__arch_chacha20_blocks_nostack) -- 2.51.0 From: Josh Poimboeuf It was decided years ago that .cfi_* annotations aren't maintainable in the kernel. They were replaced by objtool unwind hints. For the kernel proper, ensure the CFI_* macros don't do anything. On the other hand the VDSO library *does* use them, so user space can unwind through it. Make sure these macros only work for VDSO. They aren't actually being used outside of VDSO anyway, so there's no functional change. [ Jens Remus: Define CFI_SIGNAL_FRAME for !BUILD_VDSO. ] Signed-off-by: Josh Poimboeuf Signed-off-by: Steven Rostedt (Google) Acked-by: H. Peter Anvin (Intel) Signed-off-by: Jens Remus --- Notes (jremus): Changes in v8: - Define CFI_SIGNAL_FRAME for !BUILD_VDSO (new on tip:x86/entry). arch/x86/include/asm/dwarf2.h | 52 ++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h index 09c9684d3ad6..13e2e64ef265 100644 --- a/arch/x86/include/asm/dwarf2.h +++ b/arch/x86/include/asm/dwarf2.h @@ -6,6 +6,15 @@ #warning "asm/dwarf2.h should be only included in pure assembly files" #endif +#ifdef BUILD_VDSO + + /* + * For the vDSO, emit both runtime unwind information and debug + * symbols for the .dbg file. + */ + + .cfi_sections .eh_frame, .debug_frame + #define CFI_STARTPROC .cfi_startproc #define CFI_ENDPROC .cfi_endproc #define CFI_DEF_CFA .cfi_def_cfa @@ -22,21 +31,32 @@ #define CFI_ESCAPE .cfi_escape #define CFI_SIGNAL_FRAME .cfi_signal_frame -#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 we - * ever decide to enable DWARF unwinding at runtime. - */ - .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 +#else /* !BUILD_VDSO */ + +/* + * On x86, these macros aren't used outside VDSO. As well they shouldn't be: + * they're fragile and very difficult to maintain. + */ + +.macro nocfi args:vararg +.endm + +#define CFI_STARTPROC nocfi +#define CFI_ENDPROC nocfi +#define CFI_DEF_CFA nocfi +#define CFI_DEF_CFA_REGISTER nocfi +#define CFI_DEF_CFA_OFFSET nocfi +#define CFI_ADJUST_CFA_OFFSET nocfi +#define CFI_OFFSET nocfi +#define CFI_REL_OFFSET nocfi +#define CFI_REGISTER nocfi +#define CFI_RESTORE nocfi +#define CFI_REMEMBER_STATE nocfi +#define CFI_RESTORE_STATE nocfi +#define CFI_UNDEFINED nocfi +#define CFI_ESCAPE nocfi +#define CFI_SIGNAL_FRAME nocfi + +#endif /* !BUILD_VDSO */ #endif /* _ASM_X86_DWARF2_H */ -- 2.51.0 From: Josh Poimboeuf Add CFI_STARTPROC and CFI_ENDPROC annotations to the SYM_FUNC_* macros so the VDSO asm functions don't need to add them manually. Note this only affects VDSO, the CFI_* macros are empty for the kernel proper. Signed-off-by: Josh Poimboeuf Signed-off-by: Steven Rostedt (Google) Acked-by: H. Peter Anvin (Intel) Signed-off-by: Jens Remus --- Notes (jremus): Changes in v8: - Reword commit subject to Steven's v6 one: https://lore.kernel.org/all/20250425024022.652143069@goodmis.org/ - Drop my changelog from the commit message, as the patch is now identical to Steven's v6. arch/x86/entry/vdso/common/vdso-layout.lds.S | 2 +- .../x86/entry/vdso/vdso64/vgetrandom-chacha.S | 2 -- arch/x86/entry/vdso/vdso64/vsgx.S | 4 --- arch/x86/include/asm/linkage.h | 33 +++++++++++++++---- arch/x86/include/asm/vdso.h | 1 - 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/arch/x86/entry/vdso/common/vdso-layout.lds.S b/arch/x86/entry/vdso/common/vdso-layout.lds.S index a1e30be3e83d..856b8b9d278c 100644 --- a/arch/x86/entry/vdso/common/vdso-layout.lds.S +++ b/arch/x86/entry/vdso/common/vdso-layout.lds.S @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0 */ -#include +#include #include #include diff --git a/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S b/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S index cc82da9216fb..a33212594731 100644 --- a/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S +++ b/arch/x86/entry/vdso/vdso64/vgetrandom-chacha.S @@ -22,7 +22,6 @@ CONSTANTS: .octa 0x6b20657479622d323320646e61707865 * rcx: number of 64-byte blocks to write to output */ SYM_FUNC_START(__arch_chacha20_blocks_nostack) - CFI_STARTPROC .set output, %rdi .set key, %rsi .set counter, %rdx @@ -175,5 +174,4 @@ SYM_FUNC_START(__arch_chacha20_blocks_nostack) pxor temp,temp ret - CFI_ENDPROC SYM_FUNC_END(__arch_chacha20_blocks_nostack) diff --git a/arch/x86/entry/vdso/vdso64/vsgx.S b/arch/x86/entry/vdso/vdso64/vsgx.S index 37a3d4c02366..c0342238c976 100644 --- a/arch/x86/entry/vdso/vdso64/vsgx.S +++ b/arch/x86/entry/vdso/vdso64/vsgx.S @@ -24,8 +24,6 @@ .section .text, "ax" SYM_FUNC_START(__vdso_sgx_enter_enclave) - /* Prolog */ - .cfi_startproc push %rbp .cfi_adjust_cfa_offset 8 .cfi_rel_offset %rbp, 0 @@ -143,8 +141,6 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave) jle .Lout jmp .Lenter_enclave - .cfi_endproc - _ASM_VDSO_EXTABLE_HANDLE(.Lenclu_eenter_eresume, .Lhandle_exception) SYM_FUNC_END(__vdso_sgx_enter_enclave) diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h index 9d38ae744a2e..9d7f90c57451 100644 --- a/arch/x86/include/asm/linkage.h +++ b/arch/x86/include/asm/linkage.h @@ -40,6 +40,10 @@ #ifdef __ASSEMBLER__ +#ifndef LINKER_SCRIPT +#include +#endif + #if defined(CONFIG_MITIGATION_RETHUNK) && !defined(__DISABLE_EXPORTS) && !defined(BUILD_VDSO) #define RET jmp __x86_return_thunk #else /* CONFIG_MITIGATION_RETPOLINE */ @@ -112,34 +116,51 @@ # define SYM_FUNC_ALIAS_MEMFUNC SYM_FUNC_ALIAS #endif +#define __SYM_FUNC_START \ + CFI_STARTPROC ASM_NL + +#define __SYM_FUNC_END \ + CFI_ENDPROC ASM_NL + /* SYM_TYPED_FUNC_START -- use for indirectly called globals, w/ CFI type */ #define SYM_TYPED_FUNC_START(name) \ SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_F_ALIGN) \ + __SYM_FUNC_START \ ENDBR /* SYM_FUNC_START -- use for global functions */ #define SYM_FUNC_START(name) \ - SYM_START(name, SYM_L_GLOBAL, SYM_F_ALIGN) + SYM_START(name, SYM_L_GLOBAL, SYM_F_ALIGN) \ + __SYM_FUNC_START /* SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment */ #define SYM_FUNC_START_NOALIGN(name) \ - SYM_START(name, SYM_L_GLOBAL, SYM_A_NONE) + SYM_START(name, SYM_L_GLOBAL, SYM_A_NONE) \ + __SYM_FUNC_START /* SYM_FUNC_START_LOCAL -- use for local functions */ #define SYM_FUNC_START_LOCAL(name) \ - SYM_START(name, SYM_L_LOCAL, SYM_F_ALIGN) + SYM_START(name, SYM_L_LOCAL, SYM_F_ALIGN) \ + __SYM_FUNC_START /* SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o alignment */ #define SYM_FUNC_START_LOCAL_NOALIGN(name) \ - SYM_START(name, SYM_L_LOCAL, SYM_A_NONE) + SYM_START(name, SYM_L_LOCAL, SYM_A_NONE) \ + __SYM_FUNC_START /* SYM_FUNC_START_WEAK -- use for weak functions */ #define SYM_FUNC_START_WEAK(name) \ - SYM_START(name, SYM_L_WEAK, SYM_F_ALIGN) + SYM_START(name, SYM_L_WEAK, SYM_F_ALIGN) \ + __SYM_FUNC_START /* SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment */ #define SYM_FUNC_START_WEAK_NOALIGN(name) \ - SYM_START(name, SYM_L_WEAK, SYM_A_NONE) + SYM_START(name, SYM_L_WEAK, SYM_A_NONE) \ + __SYM_FUNC_START + +#define SYM_FUNC_END(name) \ + __SYM_FUNC_END \ + SYM_END(name, SYM_T_FUNC) /* * Expose 'sym' to the startup code in arch/x86/boot/startup/, by emitting an diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h index e8afbe9faa5b..498ac423741c 100644 --- a/arch/x86/include/asm/vdso.h +++ b/arch/x86/include/asm/vdso.h @@ -2,7 +2,6 @@ #ifndef _ASM_X86_VDSO_H #define _ASM_X86_VDSO_H -#include #include #include -- 2.51.0 From: Josh Poimboeuf Use SYM_FUNC_{START,END} instead of all the boilerplate. No functional change. Signed-off-by: Josh Poimboeuf Signed-off-by: Steven Rostedt (Google) Acked-by: H. Peter Anvin (Intel) Signed-off-by: Jens Remus --- arch/x86/entry/vdso/vdso32/system_call.S | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/arch/x86/entry/vdso/vdso32/system_call.S b/arch/x86/entry/vdso/vdso32/system_call.S index 9157cf9c5749..a90f4f7de396 100644 --- a/arch/x86/entry/vdso/vdso32/system_call.S +++ b/arch/x86/entry/vdso/vdso32/system_call.S @@ -9,11 +9,7 @@ #include .text - .globl __kernel_vsyscall - .type __kernel_vsyscall,@function - ALIGN -__kernel_vsyscall: - CFI_STARTPROC +SYM_FUNC_START(__kernel_vsyscall) /* * If using int $0x80, there is no reason to muck about with the @@ -85,7 +81,5 @@ SYM_INNER_LABEL(int80_landing_pad, SYM_L_GLOBAL) CFI_RESTORE ecx CFI_ADJUST_CFA_OFFSET -4 RET - CFI_ENDPROC - - .size __kernel_vsyscall,.-__kernel_vsyscall +SYM_FUNC_END(__kernel_vsyscall) .previous -- 2.51.0 From: Josh Poimboeuf Use the CFI macros instead of the raw .cfi_* directives to be consistent with the rest of the VDSO asm. It's also easier on the eyes. No functional changes. Signed-off-by: Josh Poimboeuf Signed-off-by: Steven Rostedt (Google) Acked-by: H. Peter Anvin (Intel) Signed-off-by: Jens Remus --- Notes (jremus): Changes in v8: - Remove SYM_F_ALIGN. (Josh) https://lore.kernel.org/all/amh7fzsjx4p5nowy3b3j6blkty5ojqf7jawtlslexwzdbxrhc3@zersgacmcnai/ arch/x86/entry/vdso/vdso64/vsgx.S | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/x86/entry/vdso/vdso64/vsgx.S b/arch/x86/entry/vdso/vdso64/vsgx.S index c0342238c976..76efbeb1e287 100644 --- a/arch/x86/entry/vdso/vdso64/vsgx.S +++ b/arch/x86/entry/vdso/vdso64/vsgx.S @@ -25,12 +25,12 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave) push %rbp - .cfi_adjust_cfa_offset 8 - .cfi_rel_offset %rbp, 0 + CFI_ADJUST_CFA_OFFSET 8 + CFI_REL_OFFSET %rbp, 0 mov %rsp, %rbp - .cfi_def_cfa_register %rbp + CFI_DEF_CFA_REGISTER %rbp push %rbx - .cfi_rel_offset %rbx, -8 + CFI_REL_OFFSET %rbx, -8 mov %ecx, %eax .Lenter_enclave: @@ -77,13 +77,11 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave) .Lout: pop %rbx leave - .cfi_def_cfa %rsp, 8 + CFI_DEF_CFA %rsp, 8 RET - /* The out-of-line code runs with the pre-leave stack frame. */ - .cfi_def_cfa %rbp, 16 - .Linvalid_input: + CFI_DEF_CFA %rbp, 16 mov $(-EINVAL), %eax jmp .Lout -- 2.51.0 From: Josh Poimboeuf Enable sframe generation in the VDSO library so kernel and user space can unwind through it. SFrame isn't supported for x32 or x86-32. Discard .sframe sections for those VDSOs. [ Jens Remus: Add support for SFrame V3. Prevent GNU_SFRAME program table entry to empty .sframe section. ] Signed-off-by: Josh Poimboeuf Signed-off-by: Steven Rostedt (Google) Signed-off-by: Jens Remus --- Notes (jremus): Changes in v8: - Discard .sframe for x32 and x86-32 VDSOs. (Josh/Indu) Note that the use of KEEP_SFRAME enables to define it for x86-64 VDSO only. Unlike CONFIG_AS_SFRAME, which may also be defined for x32 and x86-32 VDSO. In x32 VDSO it would result in superfluous .sframe (copied from the x86-64 build - could be removed in X32 build step). In x86-32 VDSO it would cause a bogus GNU_SFRAME program table entry. - Reword commit message (append Josh's private fix). - Drop .cfi_sections .sframe in dwarf2.h in favor of the explicitly specified more specific assembler option --gsframe-3. With this it is not necessary to undefine CONFIG_AS_SFRAME in fake_32bit_build.h. arch/Kconfig | 7 +++++++ arch/x86/entry/vdso/common/vdso-layout.lds.S | 15 +++++++++++++++ arch/x86/entry/vdso/vdso64/Makefile | 1 + arch/x86/entry/vdso/vdso64/vdso64.lds.S | 4 ++++ 4 files changed, 27 insertions(+) diff --git a/arch/Kconfig b/arch/Kconfig index 31220f512b16..8170e492a44c 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -479,6 +479,13 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH It uses the same command line parameters, and sysctl interface, as the generic hardlockup detectors. +config AS_SFRAME + bool + +config AS_SFRAME3 + def_bool $(as-instr,.cfi_startproc\n.cfi_endproc,-Wa$(comma)--gsframe-3) + select AS_SFRAME + config UNWIND_USER bool diff --git a/arch/x86/entry/vdso/common/vdso-layout.lds.S b/arch/x86/entry/vdso/common/vdso-layout.lds.S index 856b8b9d278c..47e6a6a0560a 100644 --- a/arch/x86/entry/vdso/common/vdso-layout.lds.S +++ b/arch/x86/entry/vdso/common/vdso-layout.lds.S @@ -60,6 +60,13 @@ SECTIONS *(.eh_frame.*) } :text +#ifdef KEEP_SFRAME + .sframe : { + KEEP (*(.sframe)) + *(.sframe.*) + } :text :sframe +#endif + /* * Text is well-separated from actual data: there's plenty of * stuff that isn't used at runtime in between. @@ -80,6 +87,10 @@ SECTIONS *(.discard) *(.discard.*) *(__bug_table) +#ifndef KEEP_SFRAME + *(.sframe) + *(.sframe.*) +#endif } } @@ -89,6 +100,7 @@ SECTIONS #define PT_GNU_EH_FRAME 0x6474e550 #define PT_GNU_STACK 0x6474e551 #define PT_GNU_PROPERTY 0x6474e553 +#define PT_GNU_SFRAME 0x6474e554 /* * We must supply the ELF program headers explicitly to get just one @@ -104,6 +116,9 @@ PHDRS dynamic PT_DYNAMIC PF_R; note PT_NOTE PF_R; eh_frame_hdr PT_GNU_EH_FRAME PF_R; +#ifdef KEEP_SFRAME + sframe PT_GNU_SFRAME PF_R; +#endif gnu_stack PT_GNU_STACK PF_RW; gnu_property PT_GNU_PROPERTY PF_R; } diff --git a/arch/x86/entry/vdso/vdso64/Makefile b/arch/x86/entry/vdso/vdso64/Makefile index bfffaf1aeecc..459f8026531e 100644 --- a/arch/x86/entry/vdso/vdso64/Makefile +++ b/arch/x86/entry/vdso/vdso64/Makefile @@ -14,6 +14,7 @@ vobjs-$(CONFIG_X86_SGX) += vsgx.o # Compilation flags flags-y := -DBUILD_VDSO64 -m64 -mcmodel=small +flags-$(CONFIG_AS_SFRAME3) += -Wa,--gsframe-3 # The location of this include matters! include $(src)/../common/Makefile.include diff --git a/arch/x86/entry/vdso/vdso64/vdso64.lds.S b/arch/x86/entry/vdso/vdso64/vdso64.lds.S index 5ce3f2b6373a..13e30d3b4827 100644 --- a/arch/x86/entry/vdso/vdso64/vdso64.lds.S +++ b/arch/x86/entry/vdso/vdso64/vdso64.lds.S @@ -9,6 +9,10 @@ #define BUILD_VDSO64 +#ifdef CONFIG_AS_SFRAME +# define KEEP_SFRAME +#endif + #include "common/vdso-layout.lds.S" /* -- 2.51.0