Parts of this were discussed in the context of Andrew's CET-SS work.
Further parts simply fit the underlying picture. And a few patches
towards the end get attached here simply because of their dependency.
Patch 7 is new.
All patches except for the new ones in principle have acks / R-b-s
which would allow them to go in. However, there still the controversy
on the naming of the newly introduced header in patch 1 (which
subsequent patches then add to). There hasn't been a name suggestion
which would - imo - truly represent an improvement.
It's also still not really clear to me what - if any - changes to
make to patch 2. As said there I'd be willing to drop some of the
changes made, but not all. Prior discussion hasn't led to a clear
understanding on my part of what's wanted to be kept / dropped. It
could have looked like the entire patch was wanted to go away, but I
don't think I can agree with this. (I could see about moving this to
the end of the series, to unblock what's currently the remainder.)
1: replace __ASM_{CL,ST}AC
2: reduce CET-SS related #ifdef-ary
3: drop ASM_{CL,ST}AC
4: fold indirect_thunk_asm.h into asm-defns.h
5: guard against straight-line speculation past RET
6: limit amount of INT3 in IND_THUNK_*
7: make guarding against straight-line speculation optional
Jan
Introduce proper assembler macros instead, enabled only when the
assembler itself doesn't support the insns. To avoid duplicating the
macros for assembly and C files, have them processed into asm-macros.h.
This in turn requires adding a multiple inclusion guard when generating
that header.
No change to generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -243,7 +243,10 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
echo '#if 0' >$@.new
echo '.if 0' >>$@.new
echo '#endif' >>$@.new
+ echo '#ifndef __ASM_MACROS_H__' >>$@.new
+ echo '#define __ASM_MACROS_H__' >>$@.new
echo 'asm ( ".include \"$@\"" );' >>$@.new
+ echo '#endif /* __ASM_MACROS_H__ */' >>$@.new
echo '#if 0' >>$@.new
echo '.endif' >>$@.new
cat $< >>$@.new
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -20,6 +20,7 @@ $(call as-option-add,CFLAGS,CC,"rdrand %
$(call as-option-add,CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE)
$(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
$(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
+$(call as-option-add,CFLAGS,CC,"clac",-DHAVE_AS_CLAC_STAC)
$(call as-option-add,CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB)
$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1",-DHAVE_AS_QUOTED_SYM)
$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID)
--- a/xen/arch/x86/asm-macros.c
+++ b/xen/arch/x86/asm-macros.c
@@ -1 +1,2 @@
+#include <asm/asm-defns.h>
#include <asm/alternative-asm.h>
--- /dev/null
+++ b/xen/include/asm-x86/asm-defns.h
@@ -0,0 +1,9 @@
+#ifndef HAVE_AS_CLAC_STAC
+.macro clac
+ .byte 0x0f, 0x01, 0xca
+.endm
+
+.macro stac
+ .byte 0x0f, 0x01, 0xcb
+.endm
+#endif
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -13,10 +13,12 @@
#include <asm/alternative.h>
#ifdef __ASSEMBLY__
+#include <asm/asm-defns.h>
#ifndef CONFIG_INDIRECT_THUNK
.equ CONFIG_INDIRECT_THUNK, 0
#endif
#else
+#include <asm/asm-macros.h>
asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
__stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) );
#endif
@@ -200,34 +202,27 @@ register unsigned long current_stack_poi
#endif
-/* "Raw" instruction opcodes */
-#define __ASM_CLAC ".byte 0x0f,0x01,0xca"
-#define __ASM_STAC ".byte 0x0f,0x01,0xcb"
-
#ifdef __ASSEMBLY__
.macro ASM_STAC
- ALTERNATIVE "", __ASM_STAC, X86_FEATURE_XEN_SMAP
+ ALTERNATIVE "", stac, X86_FEATURE_XEN_SMAP
.endm
.macro ASM_CLAC
- ALTERNATIVE "", __ASM_CLAC, X86_FEATURE_XEN_SMAP
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
.endm
#else
static always_inline void clac(void)
{
/* Note: a barrier is implicit in alternative() */
- alternative("", __ASM_CLAC, X86_FEATURE_XEN_SMAP);
+ alternative("", "clac", X86_FEATURE_XEN_SMAP);
}
static always_inline void stac(void)
{
/* Note: a barrier is implicit in alternative() */
- alternative("", __ASM_STAC, X86_FEATURE_XEN_SMAP);
+ alternative("", "stac", X86_FEATURE_XEN_SMAP);
}
#endif
-#undef __ASM_STAC
-#undef __ASM_CLAC
-
#ifdef __ASSEMBLY__
.macro SAVE_ALL op, compat=0
.ifeqs "\op", "CLAC"
Commit b586a81b7a90 ("x86/CET: Fix build following c/s 43b98e7190") had
to introduce a number of #ifdef-s to make the build work with older tool
chains. Introduce an assembler macro covering for tool chains not
knowing of CET-SS, allowing some conditionals where just SETSSBSY is the
problem to be dropped again.
No change to generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
Now that I've done this I'm no longer sure which direction is better to
follow: On one hand this introduces dead code (even if just NOPs) into
CET-SS-disabled builds. Otoh this is a step towards breaking the tool
chain version dependency of the feature.
I've also dropped conditionals around bigger chunks of code; while I
think that's preferable, I'm open to undo those parts.
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -31,7 +31,6 @@ ENTRY(__high_start)
jz .L_bsp
/* APs. Set up shadow stacks before entering C. */
-#ifdef CONFIG_XEN_SHSTK
testl $cpufeat_mask(X86_FEATURE_XEN_SHSTK), \
CPUINFO_FEATURE_OFFSET(X86_FEATURE_XEN_SHSTK) + boot_cpu_data(%rip)
je .L_ap_shstk_done
@@ -55,7 +54,6 @@ ENTRY(__high_start)
mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx
mov %rcx, %cr4
setssbsy
-#endif
.L_ap_shstk_done:
call start_secondary
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -668,7 +668,7 @@ static void __init noreturn reinit_bsp_s
stack_base[0] = stack;
memguard_guard_stack(stack);
- if ( IS_ENABLED(CONFIG_XEN_SHSTK) && cpu_has_xen_shstk )
+ if ( cpu_has_xen_shstk )
{
wrmsrl(MSR_PL0_SSP,
(unsigned long)stack + (PRIMARY_SHSTK_SLOT + 1) * PAGE_SIZE - 8);
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -197,9 +197,7 @@ ENTRY(cr4_pv32_restore)
/* See lstar_enter for entry register state. */
ENTRY(cstar_enter)
-#ifdef CONFIG_XEN_SHSTK
ALTERNATIVE "", "setssbsy", X86_FEATURE_XEN_SHSTK
-#endif
/* sti could live here when we don't switch page tables below. */
CR4_PV32_RESTORE
movq 8(%rsp),%rax /* Restore %rax. */
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -236,9 +236,7 @@ iret_exit_to_guest:
* %ss must be saved into the space left by the trampoline.
*/
ENTRY(lstar_enter)
-#ifdef CONFIG_XEN_SHSTK
ALTERNATIVE "", "setssbsy", X86_FEATURE_XEN_SHSTK
-#endif
/* sti could live here when we don't switch page tables below. */
movq 8(%rsp),%rax /* Restore %rax. */
movq $FLAT_KERNEL_SS,8(%rsp)
@@ -272,9 +270,7 @@ ENTRY(lstar_enter)
jmp test_all_events
ENTRY(sysenter_entry)
-#ifdef CONFIG_XEN_SHSTK
ALTERNATIVE "", "setssbsy", X86_FEATURE_XEN_SHSTK
-#endif
/* sti could live here when we don't switch page tables below. */
pushq $FLAT_USER_SS
pushq $0
--- a/xen/include/asm-x86/asm-defns.h
+++ b/xen/include/asm-x86/asm-defns.h
@@ -7,3 +7,9 @@
.byte 0x0f, 0x01, 0xcb
.endm
#endif
+
+#ifndef CONFIG_HAS_AS_CET_SS
+.macro setssbsy
+ .byte 0xf3, 0x0f, 0x01, 0xe8
+.endm
+#endif
Use ALTERNATIVE directly, such that at the use sites it is visible that
alternative code patching is in use. Similarly avoid hiding the fact in
SAVE_ALL.
No change to generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2: Further adjust comment in asm_domain_crash_synchronous().
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2165,9 +2165,8 @@ void activate_debugregs(const struct vcp
void asm_domain_crash_synchronous(unsigned long addr)
{
/*
- * We need clear AC bit here because in entry.S AC is set
- * by ASM_STAC to temporarily allow accesses to user pages
- * which is prevented by SMAP by default.
+ * We need to clear the AC bit here because the exception fixup logic
+ * may leave user accesses enabled.
*
* For some code paths, where this function is called, clac()
* is not needed, but adding clac() here instead of each place
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -12,7 +12,7 @@
#include <irq_vectors.h>
ENTRY(entry_int82)
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
pushq $0
movl $HYPERCALL_VECTOR, 4(%rsp)
SAVE_ALL compat=1 /* DPL1 gate, restricted to 32bit PV guests only. */
@@ -284,7 +284,7 @@ ENTRY(compat_int80_direct_trap)
compat_create_bounce_frame:
ASSERT_INTERRUPTS_ENABLED
mov %fs,%edi
- ASM_STAC
+ ALTERNATIVE "", stac, X86_FEATURE_XEN_SMAP
testb $2,UREGS_cs+8(%rsp)
jz 1f
/* Push new frame at registered guest-OS stack base. */
@@ -331,7 +331,7 @@ compat_create_bounce_frame:
movl TRAPBOUNCE_error_code(%rdx),%eax
.Lft8: movl %eax,%fs:(%rsi) # ERROR CODE
1:
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
/* Rewrite our stack frame and return to guest-OS mode. */
/* IA32 Ref. Vol. 3: TF, VM, RF and NT flags are cleared on trap. */
andl $~(X86_EFLAGS_VM|X86_EFLAGS_RF|\
@@ -377,7 +377,7 @@ compat_crash_page_fault_4:
addl $4,%esi
compat_crash_page_fault:
.Lft14: mov %edi,%fs
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
movl %esi,%edi
call show_page_walk
jmp dom_crash_sync_extable
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -276,7 +276,7 @@ ENTRY(sysenter_entry)
pushq $0
pushfq
GLOBAL(sysenter_eflags_saved)
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
pushq $3 /* ring 3 null cs */
pushq $0 /* null rip */
pushq $0
@@ -329,7 +329,7 @@ UNLIKELY_END(sysenter_gpf)
jmp .Lbounce_exception
ENTRY(int80_direct_trap)
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
pushq $0
movl $0x80, 4(%rsp)
SAVE_ALL
@@ -448,7 +448,7 @@ __UNLIKELY_END(create_bounce_frame_bad_s
subq $7*8,%rsi
movq UREGS_ss+8(%rsp),%rax
- ASM_STAC
+ ALTERNATIVE "", stac, X86_FEATURE_XEN_SMAP
movq VCPU_domain(%rbx),%rdi
STORE_GUEST_STACK(rax,6) # SS
movq UREGS_rsp+8(%rsp),%rax
@@ -486,7 +486,7 @@ __UNLIKELY_END(create_bounce_frame_bad_s
STORE_GUEST_STACK(rax,1) # R11
movq UREGS_rcx+8(%rsp),%rax
STORE_GUEST_STACK(rax,0) # RCX
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
#undef STORE_GUEST_STACK
@@ -528,11 +528,11 @@ domain_crash_page_fault_2x8:
domain_crash_page_fault_1x8:
addq $8,%rsi
domain_crash_page_fault_0x8:
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
movq %rsi,%rdi
call show_page_walk
ENTRY(dom_crash_sync_extable)
- ASM_CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
# Get out of the guest-save area of the stack.
GET_STACK_END(ax)
leaq STACK_CPUINFO_FIELD(guest_cpu_user_regs)(%rax),%rsp
@@ -590,7 +590,8 @@ UNLIKELY_END(exit_cr3)
iretq
ENTRY(common_interrupt)
- SAVE_ALL CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
+ SAVE_ALL
GET_STACK_END(14)
@@ -622,7 +623,8 @@ ENTRY(page_fault)
movl $TRAP_page_fault,4(%rsp)
/* No special register assumptions. */
GLOBAL(handle_exception)
- SAVE_ALL CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
+ SAVE_ALL
GET_STACK_END(14)
@@ -827,7 +829,8 @@ ENTRY(entry_CP)
ENTRY(double_fault)
movl $TRAP_double_fault,4(%rsp)
/* Set AC to reduce chance of further SMAP faults */
- SAVE_ALL STAC
+ ALTERNATIVE "", stac, X86_FEATURE_XEN_SMAP
+ SAVE_ALL
GET_STACK_END(14)
@@ -860,7 +863,8 @@ ENTRY(nmi)
pushq $0
movl $TRAP_nmi,4(%rsp)
handle_ist_exception:
- SAVE_ALL CLAC
+ ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
+ SAVE_ALL
GET_STACK_END(14)
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -200,16 +200,6 @@ register unsigned long current_stack_poi
UNLIKELY_END_SECTION "\n" \
".Llikely." #tag ".%=:"
-#endif
-
-#ifdef __ASSEMBLY__
-.macro ASM_STAC
- ALTERNATIVE "", stac, X86_FEATURE_XEN_SMAP
-.endm
-.macro ASM_CLAC
- ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
-.endm
-#else
static always_inline void clac(void)
{
/* Note: a barrier is implicit in alternative() */
@@ -224,18 +214,7 @@ static always_inline void stac(void)
#endif
#ifdef __ASSEMBLY__
-.macro SAVE_ALL op, compat=0
-.ifeqs "\op", "CLAC"
- ASM_CLAC
-.else
-.ifeqs "\op", "STAC"
- ASM_STAC
-.else
-.ifnb \op
- .err
-.endif
-.endif
-.endif
+.macro SAVE_ALL compat=0
addq $-(UREGS_error_code-UREGS_r15), %rsp
cld
movq %rdi,UREGS_rdi(%rsp)
There's little point in having two separate headers both getting
included by asm_defns.h. This in particular reduces the number of
instances of guarding asm(".include ...") suitably in such dual use
headers.
No change to generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -139,7 +139,7 @@ ifeq ($(TARGET_ARCH),x86)
t1 = $(call as-insn,$(CC),".L0: .L1: .skip (.L1 - .L0)",,-no-integrated-as)
# Check whether clang asm()-s support .include.
-t2 = $(call as-insn,$(CC) -I$(BASEDIR)/include,".include \"asm-x86/indirect_thunk_asm.h\"",,-no-integrated-as)
+t2 = $(call as-insn,$(CC) -I$(BASEDIR)/include,".include \"asm-x86/asm-defns.h\"",,-no-integrated-as)
# Check whether clang keeps .macro-s between asm()-s:
# https://bugs.llvm.org/show_bug.cgi?id=36110
--- a/xen/include/asm-x86/asm-defns.h
+++ b/xen/include/asm-x86/asm-defns.h
@@ -13,3 +13,40 @@
.byte 0xf3, 0x0f, 0x01, 0xe8
.endm
#endif
+
+.macro INDIRECT_BRANCH insn:req arg:req
+/*
+ * Create an indirect branch. insn is one of call/jmp, arg is a single
+ * register.
+ *
+ * With no compiler support, this degrades into a plain indirect call/jmp.
+ * With compiler support, dispatch to the correct __x86_indirect_thunk_*
+ */
+ .if CONFIG_INDIRECT_THUNK == 1
+
+ $done = 0
+ .irp reg, ax, cx, dx, bx, bp, si, di, 8, 9, 10, 11, 12, 13, 14, 15
+ .ifeqs "\arg", "%r\reg"
+ \insn __x86_indirect_thunk_r\reg
+ $done = 1
+ .exitm
+ .endif
+ .endr
+
+ .if $done != 1
+ .error "Bad register arg \arg"
+ .endif
+
+ .else
+ \insn *\arg
+ .endif
+.endm
+
+/* Convenience wrappers. */
+.macro INDIRECT_CALL arg:req
+ INDIRECT_BRANCH call \arg
+.endm
+
+.macro INDIRECT_JMP arg:req
+ INDIRECT_BRANCH jmp \arg
+.endm
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -22,7 +22,6 @@
asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
__stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) );
#endif
-#include <asm/indirect_thunk_asm.h>
#ifndef __ASSEMBLY__
void ret_from_intr(void);
--- a/xen/include/asm-x86/indirect_thunk_asm.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Trickery to allow this header to be included at the C level, to permit
- * proper dependency tracking in .*.o.d files, while still having it contain
- * assembler only macros.
- */
-#ifndef __ASSEMBLY__
-# if 0
- .if 0
-# endif
-asm ( "\t.include \"asm/indirect_thunk_asm.h\"" );
-# if 0
- .endif
-# endif
-#else
-
-.macro INDIRECT_BRANCH insn:req arg:req
-/*
- * Create an indirect branch. insn is one of call/jmp, arg is a single
- * register.
- *
- * With no compiler support, this degrades into a plain indirect call/jmp.
- * With compiler support, dispatch to the correct __x86_indirect_thunk_*
- */
- .if CONFIG_INDIRECT_THUNK == 1
-
- $done = 0
- .irp reg, ax, cx, dx, bx, bp, si, di, 8, 9, 10, 11, 12, 13, 14, 15
- .ifeqs "\arg", "%r\reg"
- \insn __x86_indirect_thunk_r\reg
- $done = 1
- .exitm
- .endif
- .endr
-
- .if $done != 1
- .error "Bad register arg \arg"
- .endif
-
- .else
- \insn *\arg
- .endif
-.endm
-
-/* Convenience wrappers. */
-.macro INDIRECT_CALL arg:req
- INDIRECT_BRANCH call \arg
-.endm
-
-.macro INDIRECT_JMP arg:req
- INDIRECT_BRANCH jmp \arg
-.endm
-
-#endif
Under certain conditions CPUs can speculate into the instruction stream
past a RET instruction. Guard against this just like 3b7dab93f240
("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
did - by inserting an "INT $3" insn. It's merely the mechanics of how to
achieve this that differ: A set of macros gets introduced to post-
process RET insns issued by the compiler (or living in assembly files).
Unfortunately for clang this requires further features their built-in
assembler doesn't support: We need to be able to override insn mnemonics
produced by the compiler (which may be impossible, if internally
assembly mnemonics never get generated), and we want to use \(text)
escaping / quoting in the auxiliary macro.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
---
TBD: Would be nice to avoid the additions in .init.text, but a query to
the binutils folks regarding the ability to identify the section
stuff is in (by Peter Zijlstra over a year ago:
https://sourceware.org/pipermail/binutils/2019-July/107528.html)
has been left without helpful replies.
---
v3: Use .byte 0xc[23] instead of the nested macros.
v2: Fix build with newer clang. Use int3 mnemonic. Also override retq.
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -145,7 +145,15 @@ t2 = $(call as-insn,$(CC) -I$(BASEDIR)/i
# https://bugs.llvm.org/show_bug.cgi?id=36110
t3 = $(call as-insn,$(CC),".macro FOO;.endm"$(close); asm volatile $(open)".macro FOO;.endm",-no-integrated-as)
-CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
+# Check whether \(text) escaping in macro bodies is supported.
+t4 = $(call as-insn,$(CC),".macro m ret:req; \\(ret) $$\\ret; .endm; m 8",,-no-integrated-as)
+
+# Check whether macros can override insn mnemonics in inline assembly.
+t5 = $(call as-insn,$(CC),".macro ret; .error; .endm; .macro retq; .error; .endm",-no-integrated-as)
+
+acc1 := $(call or,$(t1),$(t2),$(t3),$(t4))
+
+CLANG_FLAGS += $(call or,$(acc1),$(t5))
endif
CLANG_FLAGS += -Werror=unknown-warning-option
--- a/xen/include/asm-x86/asm-defns.h
+++ b/xen/include/asm-x86/asm-defns.h
@@ -50,3 +50,19 @@
.macro INDIRECT_JMP arg:req
INDIRECT_BRANCH jmp \arg
.endm
+
+/*
+ * To guard against speculation past RET, insert a breakpoint insn
+ * immediately after them.
+ */
+.macro ret operand:vararg
+ retq \operand
+.endm
+.macro retq operand:vararg
+ .ifb \operand
+ .byte 0xc3
+ .else
+ .byte 0xc2
+ .word \operand
+ .endif
+.endm
On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
> Under certain conditions CPUs can speculate into the instruction stream
> past a RET instruction. Guard against this just like 3b7dab93f240
> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
> achieve this that differ: A set of macros gets introduced to post-
> process RET insns issued by the compiler (or living in assembly files).
>
> Unfortunately for clang this requires further features their built-in
> assembler doesn't support: We need to be able to override insn mnemonics
> produced by the compiler (which may be impossible, if internally
> assembly mnemonics never get generated), and we want to use \(text)
> escaping / quoting in the auxiliary macro.
Could this have an option to enable/disable at build time?
FreeBSD will drop GNU as quite soon from base, and albeit it can be
installed as a package I would like to be able to build Xen using a
toolchain based on LLVM exclusively.
Thanks, Roger.
On 10.11.2020 10:31, Roger Pau Monné wrote:
> On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
>> Under certain conditions CPUs can speculate into the instruction stream
>> past a RET instruction. Guard against this just like 3b7dab93f240
>> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
>> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
>> achieve this that differ: A set of macros gets introduced to post-
>> process RET insns issued by the compiler (or living in assembly files).
>>
>> Unfortunately for clang this requires further features their built-in
>> assembler doesn't support: We need to be able to override insn mnemonics
>> produced by the compiler (which may be impossible, if internally
>> assembly mnemonics never get generated), and we want to use \(text)
>> escaping / quoting in the auxiliary macro.
>
> Could this have an option to enable/disable at build time?
Well, a subsequent patch adds a config option for this, which in
the worst case could be turned off. I'm afraid though I'm not
clear about the question, because ...
> FreeBSD will drop GNU as quite soon from base, and albeit it can be
> installed as a package I would like to be able to build Xen using a
> toolchain based on LLVM exclusively.
... it's not clear to me what the implications here are: Are you
saying -no-integrated-as is not going to function anymore, unless
people explicitly install gas? If that's not what you meant to
indicate, then I don't see how building would become impossible.
Depending on the situation as a whole, we might then be in need
of a 2nd config option...
And btw, good that you pointed me back at this: The v3 change
wasn't quite complete, since we now don't need to check for
proper \(text) handling anymore in our logic to establish
CLANG_FLAGS. I've dropped that part for v4.
Jan
On Tue, Nov 10, 2020 at 11:06:46AM +0100, Jan Beulich wrote:
> On 10.11.2020 10:31, Roger Pau Monné wrote:
> > On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
> >> Under certain conditions CPUs can speculate into the instruction stream
> >> past a RET instruction. Guard against this just like 3b7dab93f240
> >> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
> >> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
> >> achieve this that differ: A set of macros gets introduced to post-
> >> process RET insns issued by the compiler (or living in assembly files).
> >>
> >> Unfortunately for clang this requires further features their built-in
> >> assembler doesn't support: We need to be able to override insn mnemonics
> >> produced by the compiler (which may be impossible, if internally
> >> assembly mnemonics never get generated), and we want to use \(text)
> >> escaping / quoting in the auxiliary macro.
> >
> > Could this have an option to enable/disable at build time?
>
> Well, a subsequent patch adds a config option for this, which in
> the worst case could be turned off. I'm afraid though I'm not
> clear about the question, because ...
>
> > FreeBSD will drop GNU as quite soon from base, and albeit it can be
> > installed as a package I would like to be able to build Xen using a
> > toolchain based on LLVM exclusively.
>
> ... it's not clear to me what the implications here are: Are you
> saying -no-integrated-as is not going to function anymore, unless
> people explicitly install gas? If that's not what you meant to
> indicate, then I don't see how building would become impossible.
I'm still inquiring about this, but I would say that when gas is
removed from FreeBSD then the 'as' command would be mapped to llvm-as,
and thus -no-integrated-as would hit the same issues as the integrated
as. So far in Xen we have assumed that -no-integrated-as would
fallback to an as capable of doing what the integrated clang as
doesn't support, but that might not be the case.
Ideally we would have to re-run the tests with -no-integrated-as, in
order to assert that the external as is really capable of what the
internal one is not.
Roger.
On 10.11.2020 12:16, Roger Pau Monné wrote:
> On Tue, Nov 10, 2020 at 11:06:46AM +0100, Jan Beulich wrote:
>> On 10.11.2020 10:31, Roger Pau Monné wrote:
>>> On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
>>>> Under certain conditions CPUs can speculate into the instruction stream
>>>> past a RET instruction. Guard against this just like 3b7dab93f240
>>>> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
>>>> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
>>>> achieve this that differ: A set of macros gets introduced to post-
>>>> process RET insns issued by the compiler (or living in assembly files).
>>>>
>>>> Unfortunately for clang this requires further features their built-in
>>>> assembler doesn't support: We need to be able to override insn mnemonics
>>>> produced by the compiler (which may be impossible, if internally
>>>> assembly mnemonics never get generated), and we want to use \(text)
>>>> escaping / quoting in the auxiliary macro.
>>>
>>> Could this have an option to enable/disable at build time?
>>
>> Well, a subsequent patch adds a config option for this, which in
>> the worst case could be turned off. I'm afraid though I'm not
>> clear about the question, because ...
>>
>>> FreeBSD will drop GNU as quite soon from base, and albeit it can be
>>> installed as a package I would like to be able to build Xen using a
>>> toolchain based on LLVM exclusively.
>>
>> ... it's not clear to me what the implications here are: Are you
>> saying -no-integrated-as is not going to function anymore, unless
>> people explicitly install gas? If that's not what you meant to
>> indicate, then I don't see how building would become impossible.
>
> I'm still inquiring about this, but I would say that when gas is
> removed from FreeBSD then the 'as' command would be mapped to llvm-as,
> and thus -no-integrated-as would hit the same issues as the integrated
> as. So far in Xen we have assumed that -no-integrated-as would
> fallback to an as capable of doing what the integrated clang as
> doesn't support, but that might not be the case.
At which point Xen couldn't be built anyway, I expect. If llvm-as
isn't sufficiently gas-compatible, we've lost (right now at least).
> Ideally we would have to re-run the tests with -no-integrated-as, in
> order to assert that the external as is really capable of what the
> internal one is not.
And if it doesn't, what would we do other than failing the build
(which it would also if we didn't do the 2nd round of checks)?
Jan
On Tue, Nov 10, 2020 at 02:19:40PM +0100, Jan Beulich wrote:
> On 10.11.2020 12:16, Roger Pau Monné wrote:
> > On Tue, Nov 10, 2020 at 11:06:46AM +0100, Jan Beulich wrote:
> >> On 10.11.2020 10:31, Roger Pau Monné wrote:
> >>> On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
> >>>> Under certain conditions CPUs can speculate into the instruction stream
> >>>> past a RET instruction. Guard against this just like 3b7dab93f240
> >>>> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
> >>>> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
> >>>> achieve this that differ: A set of macros gets introduced to post-
> >>>> process RET insns issued by the compiler (or living in assembly files).
> >>>>
> >>>> Unfortunately for clang this requires further features their built-in
> >>>> assembler doesn't support: We need to be able to override insn mnemonics
> >>>> produced by the compiler (which may be impossible, if internally
> >>>> assembly mnemonics never get generated), and we want to use \(text)
> >>>> escaping / quoting in the auxiliary macro.
> >>>
> >>> Could this have an option to enable/disable at build time?
> >>
> >> Well, a subsequent patch adds a config option for this, which in
> >> the worst case could be turned off. I'm afraid though I'm not
> >> clear about the question, because ...
> >>
> >>> FreeBSD will drop GNU as quite soon from base, and albeit it can be
> >>> installed as a package I would like to be able to build Xen using a
> >>> toolchain based on LLVM exclusively.
> >>
> >> ... it's not clear to me what the implications here are: Are you
> >> saying -no-integrated-as is not going to function anymore, unless
> >> people explicitly install gas? If that's not what you meant to
> >> indicate, then I don't see how building would become impossible.
> >
> > I'm still inquiring about this, but I would say that when gas is
> > removed from FreeBSD then the 'as' command would be mapped to llvm-as,
> > and thus -no-integrated-as would hit the same issues as the integrated
> > as. So far in Xen we have assumed that -no-integrated-as would
> > fallback to an as capable of doing what the integrated clang as
> > doesn't support, but that might not be the case.
>
> At which point Xen couldn't be built anyway, I expect. If llvm-as
> isn't sufficiently gas-compatible, we've lost (right now at least).
>
> > Ideally we would have to re-run the tests with -no-integrated-as, in
> > order to assert that the external as is really capable of what the
> > internal one is not.
>
> And if it doesn't, what would we do other than failing the build
> (which it would also if we didn't do the 2nd round of checks)?
I would always prefer a clear message (ie: your toolstack is not
capable of building Xen) rather than a weird build time failure.
Also we could maybe disable certain options by default if the
toolstack doesn't have the required support to build them?
Has anyone reported this shortcoming to upstream llvm, so they are
aware and can work on this or maybe provide an alternative way to
achieve the same result?
Thanks, Roger.
On 10.11.2020 15:08, Roger Pau Monné wrote:
> On Tue, Nov 10, 2020 at 02:19:40PM +0100, Jan Beulich wrote:
>> On 10.11.2020 12:16, Roger Pau Monné wrote:
>>> On Tue, Nov 10, 2020 at 11:06:46AM +0100, Jan Beulich wrote:
>>>> On 10.11.2020 10:31, Roger Pau Monné wrote:
>>>>> On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
>>>>>> Under certain conditions CPUs can speculate into the instruction stream
>>>>>> past a RET instruction. Guard against this just like 3b7dab93f240
>>>>>> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
>>>>>> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
>>>>>> achieve this that differ: A set of macros gets introduced to post-
>>>>>> process RET insns issued by the compiler (or living in assembly files).
>>>>>>
>>>>>> Unfortunately for clang this requires further features their built-in
>>>>>> assembler doesn't support: We need to be able to override insn mnemonics
>>>>>> produced by the compiler (which may be impossible, if internally
>>>>>> assembly mnemonics never get generated), and we want to use \(text)
>>>>>> escaping / quoting in the auxiliary macro.
>>>>>
>>>>> Could this have an option to enable/disable at build time?
>>>>
>>>> Well, a subsequent patch adds a config option for this, which in
>>>> the worst case could be turned off. I'm afraid though I'm not
>>>> clear about the question, because ...
>>>>
>>>>> FreeBSD will drop GNU as quite soon from base, and albeit it can be
>>>>> installed as a package I would like to be able to build Xen using a
>>>>> toolchain based on LLVM exclusively.
>>>>
>>>> ... it's not clear to me what the implications here are: Are you
>>>> saying -no-integrated-as is not going to function anymore, unless
>>>> people explicitly install gas? If that's not what you meant to
>>>> indicate, then I don't see how building would become impossible.
>>>
>>> I'm still inquiring about this, but I would say that when gas is
>>> removed from FreeBSD then the 'as' command would be mapped to llvm-as,
>>> and thus -no-integrated-as would hit the same issues as the integrated
>>> as. So far in Xen we have assumed that -no-integrated-as would
>>> fallback to an as capable of doing what the integrated clang as
>>> doesn't support, but that might not be the case.
>>
>> At which point Xen couldn't be built anyway, I expect. If llvm-as
>> isn't sufficiently gas-compatible, we've lost (right now at least).
>>
>>> Ideally we would have to re-run the tests with -no-integrated-as, in
>>> order to assert that the external as is really capable of what the
>>> internal one is not.
>>
>> And if it doesn't, what would we do other than failing the build
>> (which it would also if we didn't do the 2nd round of checks)?
>
> I would always prefer a clear message (ie: your toolstack is not
> capable of building Xen) rather than a weird build time failure.
Fair point in general.
> Also we could maybe disable certain options by default if the
> toolstack doesn't have the required support to build them?
We could, but I'm afraid this will go down the route of embedding
tool chain capabilities in xen/.config, which I continue to not
consider a good idea (and the thread got stalled, as expected).
In fact (also to Andrew and Anthony), recently I've become aware
of another shortcoming of this model: Our kernel packages contain
.config files for the various architectures and specific per-
architecture flavors. It used to be easy to update them on any
system, until the tool chain capability checks got introduced.
Now, in order to update them, one has to use the precise versions
of the various tool chain parts that will be used on the build
hosts, or else an error may result (for unexpected changes to
the file), or one may unknowingly turn off options that are
expected to be enabled.
Put more generally - if I handed someone a specific .config, I'd
expect their resulting binary to contain what I did set up. Or
for them to report back that they can't build the thing. But it
should not be the case that the .config got silently changed and
certain functionality disabled just because they use a different
tool chain.
> Has anyone reported this shortcoming to upstream llvm, so they are
> aware and can work on this or maybe provide an alternative way to
> achieve the same result?
I didn't and I'm unaware of anyone else possibly having done so.
That said, I consider it sort of obvious though that the goal of
replacing the GNU tool chain implies being fully compatible (and
presumably better in certain areas).
Jan
On Tue, Nov 10, 2020 at 03:32:43PM +0100, Jan Beulich wrote:
> On 10.11.2020 15:08, Roger Pau Monné wrote:
> > On Tue, Nov 10, 2020 at 02:19:40PM +0100, Jan Beulich wrote:
> >> On 10.11.2020 12:16, Roger Pau Monné wrote:
> >>> On Tue, Nov 10, 2020 at 11:06:46AM +0100, Jan Beulich wrote:
> >>>> On 10.11.2020 10:31, Roger Pau Monné wrote:
> >>>>> On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
> >>>>>> Under certain conditions CPUs can speculate into the instruction stream
> >>>>>> past a RET instruction. Guard against this just like 3b7dab93f240
> >>>>>> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
> >>>>>> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
> >>>>>> achieve this that differ: A set of macros gets introduced to post-
> >>>>>> process RET insns issued by the compiler (or living in assembly files).
> >>>>>>
> >>>>>> Unfortunately for clang this requires further features their built-in
> >>>>>> assembler doesn't support: We need to be able to override insn mnemonics
> >>>>>> produced by the compiler (which may be impossible, if internally
> >>>>>> assembly mnemonics never get generated), and we want to use \(text)
> >>>>>> escaping / quoting in the auxiliary macro.
> >>>>>
> >>>>> Could this have an option to enable/disable at build time?
> >>>>
> >>>> Well, a subsequent patch adds a config option for this, which in
> >>>> the worst case could be turned off. I'm afraid though I'm not
> >>>> clear about the question, because ...
> >>>>
> >>>>> FreeBSD will drop GNU as quite soon from base, and albeit it can be
> >>>>> installed as a package I would like to be able to build Xen using a
> >>>>> toolchain based on LLVM exclusively.
> >>>>
> >>>> ... it's not clear to me what the implications here are: Are you
> >>>> saying -no-integrated-as is not going to function anymore, unless
> >>>> people explicitly install gas? If that's not what you meant to
> >>>> indicate, then I don't see how building would become impossible.
> >>>
> >>> I'm still inquiring about this, but I would say that when gas is
> >>> removed from FreeBSD then the 'as' command would be mapped to llvm-as,
> >>> and thus -no-integrated-as would hit the same issues as the integrated
> >>> as. So far in Xen we have assumed that -no-integrated-as would
> >>> fallback to an as capable of doing what the integrated clang as
> >>> doesn't support, but that might not be the case.
> >>
> >> At which point Xen couldn't be built anyway, I expect. If llvm-as
> >> isn't sufficiently gas-compatible, we've lost (right now at least).
> >>
> >>> Ideally we would have to re-run the tests with -no-integrated-as, in
> >>> order to assert that the external as is really capable of what the
> >>> internal one is not.
> >>
> >> And if it doesn't, what would we do other than failing the build
> >> (which it would also if we didn't do the 2nd round of checks)?
> >
> > I would always prefer a clear message (ie: your toolstack is not
> > capable of building Xen) rather than a weird build time failure.
>
> Fair point in general.
>
> > Also we could maybe disable certain options by default if the
> > toolstack doesn't have the required support to build them?
>
> We could, but I'm afraid this will go down the route of embedding
> tool chain capabilities in xen/.config, which I continue to not
> consider a good idea (and the thread got stalled, as expected).
>
> In fact (also to Andrew and Anthony), recently I've become aware
> of another shortcoming of this model: Our kernel packages contain
> .config files for the various architectures and specific per-
> architecture flavors. It used to be easy to update them on any
> system, until the tool chain capability checks got introduced.
> Now, in order to update them, one has to use the precise versions
> of the various tool chain parts that will be used on the build
> hosts, or else an error may result (for unexpected changes to
> the file), or one may unknowingly turn off options that are
> expected to be enabled.
I think the options should only be set based on toolchain capabilities
when there's no .config. If there's an existing .config we should just
check whether the toolchain is capable of building the selected set of
options, or else report an error.
I guess this would apply to defconfig selecting options based on
toolchain capabilties.
> Put more generally - if I handed someone a specific .config, I'd
> expect their resulting binary to contain what I did set up. Or
> for them to report back that they can't build the thing. But it
> should not be the case that the .config got silently changed and
> certain functionality disabled just because they use a different
> tool chain.
Yes, I agree with this.
> > Has anyone reported this shortcoming to upstream llvm, so they are
> > aware and can work on this or maybe provide an alternative way to
> > achieve the same result?
>
> I didn't and I'm unaware of anyone else possibly having done so.
> That said, I consider it sort of obvious though that the goal of
> replacing the GNU tool chain implies being fully compatible (and
> presumably better in certain areas).
Well, I think we have to keep in mind that the usage of the compiler
and the linker by Xen is far more advanced than what most applications
do, and we are likely to hit corner cases. I bet the LLVM people
weren't even aware of such usage.
Thanks, Roger.
On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
> Under certain conditions CPUs can speculate into the instruction stream
> past a RET instruction. Guard against this just like 3b7dab93f240
> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
> achieve this that differ: A set of macros gets introduced to post-
> process RET insns issued by the compiler (or living in assembly files).
>
> Unfortunately for clang this requires further features their built-in
> assembler doesn't support: We need to be able to override insn mnemonics
> produced by the compiler (which may be impossible, if internally
> assembly mnemonics never get generated)
FTR I've reported this to LLVM upstream:
https://bugs.llvm.org/show_bug.cgi?id=48159
Roger.
On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
> Under certain conditions CPUs can speculate into the instruction stream
> past a RET instruction. Guard against this just like 3b7dab93f240
> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
> achieve this that differ: A set of macros gets introduced to post-
> process RET insns issued by the compiler (or living in assembly files).
>
> Unfortunately for clang this requires further features their built-in
> assembler doesn't support: We need to be able to override insn mnemonics
> produced by the compiler (which may be impossible, if internally
> assembly mnemonics never get generated), and we want to use \(text)
> escaping / quoting in the auxiliary macro.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> TBD: Would be nice to avoid the additions in .init.text, but a query to
> the binutils folks regarding the ability to identify the section
> stuff is in (by Peter Zijlstra over a year ago:
> https://sourceware.org/pipermail/binutils/2019-July/107528.html)
> has been left without helpful replies.
> ---
> v3: Use .byte 0xc[23] instead of the nested macros.
> v2: Fix build with newer clang. Use int3 mnemonic. Also override retq.
>
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -145,7 +145,15 @@ t2 = $(call as-insn,$(CC) -I$(BASEDIR)/i
> # https://bugs.llvm.org/show_bug.cgi?id=36110
> t3 = $(call as-insn,$(CC),".macro FOO;.endm"$(close); asm volatile $(open)".macro FOO;.endm",-no-integrated-as)
>
> -CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
> +# Check whether \(text) escaping in macro bodies is supported.
> +t4 = $(call as-insn,$(CC),".macro m ret:req; \\(ret) $$\\ret; .endm; m 8",,-no-integrated-as)
> +
> +# Check whether macros can override insn mnemonics in inline assembly.
> +t5 = $(call as-insn,$(CC),".macro ret; .error; .endm; .macro retq; .error; .endm",-no-integrated-as)
I was going over this to post a bug report to LLVM, but it seems like
gcc also doesn't overwrite ret when using the above snippet:
https://godbolt.org/z/oqsPTv
Roger.
On 11.11.2020 12:15, Roger Pau Monné wrote:
> On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
>> Under certain conditions CPUs can speculate into the instruction stream
>> past a RET instruction. Guard against this just like 3b7dab93f240
>> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
>> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
>> achieve this that differ: A set of macros gets introduced to post-
>> process RET insns issued by the compiler (or living in assembly files).
>>
>> Unfortunately for clang this requires further features their built-in
>> assembler doesn't support: We need to be able to override insn mnemonics
>> produced by the compiler (which may be impossible, if internally
>> assembly mnemonics never get generated), and we want to use \(text)
>> escaping / quoting in the auxiliary macro.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> TBD: Would be nice to avoid the additions in .init.text, but a query to
>> the binutils folks regarding the ability to identify the section
>> stuff is in (by Peter Zijlstra over a year ago:
>> https://sourceware.org/pipermail/binutils/2019-July/107528.html)
>> has been left without helpful replies.
>> ---
>> v3: Use .byte 0xc[23] instead of the nested macros.
>> v2: Fix build with newer clang. Use int3 mnemonic. Also override retq.
>>
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -145,7 +145,15 @@ t2 = $(call as-insn,$(CC) -I$(BASEDIR)/i
>> # https://bugs.llvm.org/show_bug.cgi?id=36110
>> t3 = $(call as-insn,$(CC),".macro FOO;.endm"$(close); asm volatile $(open)".macro FOO;.endm",-no-integrated-as)
>>
>> -CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
>> +# Check whether \(text) escaping in macro bodies is supported.
>> +t4 = $(call as-insn,$(CC),".macro m ret:req; \\(ret) $$\\ret; .endm; m 8",,-no-integrated-as)
>> +
>> +# Check whether macros can override insn mnemonics in inline assembly.
>> +t5 = $(call as-insn,$(CC),".macro ret; .error; .endm; .macro retq; .error; .endm",-no-integrated-as)
>
> I was going over this to post a bug report to LLVM, but it seems like
> gcc also doesn't overwrite ret when using the above snippet:
>
> https://godbolt.org/z/oqsPTv
I can't see what's different from
void test(void) {
asm volatile (".macro ret; .error; .endm; .macro retq; .error; .endm");
}
but this one produces "Error: .error directive invoked in source file"
for me with both old and new gcc.
Jan
On Wed, Nov 11, 2020 at 02:33:34PM +0100, Jan Beulich wrote:
> On 11.11.2020 12:15, Roger Pau Monné wrote:
> > On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
> >> Under certain conditions CPUs can speculate into the instruction stream
> >> past a RET instruction. Guard against this just like 3b7dab93f240
> >> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
> >> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
> >> achieve this that differ: A set of macros gets introduced to post-
> >> process RET insns issued by the compiler (or living in assembly files).
> >>
> >> Unfortunately for clang this requires further features their built-in
> >> assembler doesn't support: We need to be able to override insn mnemonics
> >> produced by the compiler (which may be impossible, if internally
> >> assembly mnemonics never get generated), and we want to use \(text)
> >> escaping / quoting in the auxiliary macro.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> >> ---
> >> TBD: Would be nice to avoid the additions in .init.text, but a query to
> >> the binutils folks regarding the ability to identify the section
> >> stuff is in (by Peter Zijlstra over a year ago:
> >> https://sourceware.org/pipermail/binutils/2019-July/107528.html)
> >> has been left without helpful replies.
> >> ---
> >> v3: Use .byte 0xc[23] instead of the nested macros.
> >> v2: Fix build with newer clang. Use int3 mnemonic. Also override retq.
> >>
> >> --- a/xen/Makefile
> >> +++ b/xen/Makefile
> >> @@ -145,7 +145,15 @@ t2 = $(call as-insn,$(CC) -I$(BASEDIR)/i
> >> # https://bugs.llvm.org/show_bug.cgi?id=36110
> >> t3 = $(call as-insn,$(CC),".macro FOO;.endm"$(close); asm volatile $(open)".macro FOO;.endm",-no-integrated-as)
> >>
> >> -CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
> >> +# Check whether \(text) escaping in macro bodies is supported.
> >> +t4 = $(call as-insn,$(CC),".macro m ret:req; \\(ret) $$\\ret; .endm; m 8",,-no-integrated-as)
> >> +
> >> +# Check whether macros can override insn mnemonics in inline assembly.
> >> +t5 = $(call as-insn,$(CC),".macro ret; .error; .endm; .macro retq; .error; .endm",-no-integrated-as)
> >
> > I was going over this to post a bug report to LLVM, but it seems like
> > gcc also doesn't overwrite ret when using the above snippet:
> >
> > https://godbolt.org/z/oqsPTv
>
> I can't see what's different from
>
> void test(void) {
> asm volatile (".macro ret; .error; .endm; .macro retq; .error; .endm");
> }
>
> but this one produces "Error: .error directive invoked in source file"
> for me with both old and new gcc.
You are right, I think godbolt is somehow busted?
I can reproduce your results with my version of gcc, so will just
report to LLVM.
Roger.
On 11.11.2020 15:19, Roger Pau Monné wrote:
> On Wed, Nov 11, 2020 at 02:33:34PM +0100, Jan Beulich wrote:
>> On 11.11.2020 12:15, Roger Pau Monné wrote:
>>> On Fri, Oct 23, 2020 at 10:38:04AM +0200, Jan Beulich wrote:
>>>> Under certain conditions CPUs can speculate into the instruction stream
>>>> past a RET instruction. Guard against this just like 3b7dab93f240
>>>> ("x86/spec-ctrl: Protect against CALL/JMP straight-line speculation")
>>>> did - by inserting an "INT $3" insn. It's merely the mechanics of how to
>>>> achieve this that differ: A set of macros gets introduced to post-
>>>> process RET insns issued by the compiler (or living in assembly files).
>>>>
>>>> Unfortunately for clang this requires further features their built-in
>>>> assembler doesn't support: We need to be able to override insn mnemonics
>>>> produced by the compiler (which may be impossible, if internally
>>>> assembly mnemonics never get generated), and we want to use \(text)
>>>> escaping / quoting in the auxiliary macro.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> ---
>>>> TBD: Would be nice to avoid the additions in .init.text, but a query to
>>>> the binutils folks regarding the ability to identify the section
>>>> stuff is in (by Peter Zijlstra over a year ago:
>>>> https://sourceware.org/pipermail/binutils/2019-July/107528.html)
>>>> has been left without helpful replies.
>>>> ---
>>>> v3: Use .byte 0xc[23] instead of the nested macros.
>>>> v2: Fix build with newer clang. Use int3 mnemonic. Also override retq.
>>>>
>>>> --- a/xen/Makefile
>>>> +++ b/xen/Makefile
>>>> @@ -145,7 +145,15 @@ t2 = $(call as-insn,$(CC) -I$(BASEDIR)/i
>>>> # https://bugs.llvm.org/show_bug.cgi?id=36110
>>>> t3 = $(call as-insn,$(CC),".macro FOO;.endm"$(close); asm volatile $(open)".macro FOO;.endm",-no-integrated-as)
>>>>
>>>> -CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
>>>> +# Check whether \(text) escaping in macro bodies is supported.
>>>> +t4 = $(call as-insn,$(CC),".macro m ret:req; \\(ret) $$\\ret; .endm; m 8",,-no-integrated-as)
>>>> +
>>>> +# Check whether macros can override insn mnemonics in inline assembly.
>>>> +t5 = $(call as-insn,$(CC),".macro ret; .error; .endm; .macro retq; .error; .endm",-no-integrated-as)
>>>
>>> I was going over this to post a bug report to LLVM, but it seems like
>>> gcc also doesn't overwrite ret when using the above snippet:
>>>
>>> https://godbolt.org/z/oqsPTv
>>
>> I can't see what's different from
>>
>> void test(void) {
>> asm volatile (".macro ret; .error; .endm; .macro retq; .error; .endm");
>> }
>>
>> but this one produces "Error: .error directive invoked in source file"
>> for me with both old and new gcc.
>
> You are right, I think godbolt is somehow busted?
Or maybe they really only compile to assembly, while the error results
from the assembler?
Jan
There's no point having every replacement variant to also specify the
INT3 - just have it once in the base macro. When patching, NOPs will get
inserted, which are fine to speculate through (until reaching the INT3).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
---
I also wonder whether the LFENCE in IND_THUNK_RETPOLINE couldn't be
replaced by INT3 as well. Of course the effect will be marginal, as the
size of the thunk will still be 16 bytes when including tail padding
resulting from alignment.
---
v3: Add comment.
v2: New.
--- a/xen/arch/x86/indirect-thunk.S
+++ b/xen/arch/x86/indirect-thunk.S
@@ -11,6 +11,9 @@
#include <asm/asm_defns.h>
+/* Don't tranform the "ret" further down. */
+.purgem ret
+
.macro IND_THUNK_RETPOLINE reg:req
call 2f
1:
@@ -24,12 +27,10 @@
.macro IND_THUNK_LFENCE reg:req
lfence
jmp *%\reg
- int3 /* Halt straight-line speculation */
.endm
.macro IND_THUNK_JMP reg:req
jmp *%\reg
- int3 /* Halt straight-line speculation */
.endm
/*
@@ -44,6 +45,8 @@ ENTRY(__x86_indirect_thunk_\reg)
__stringify(IND_THUNK_LFENCE \reg), X86_FEATURE_IND_THUNK_LFENCE, \
__stringify(IND_THUNK_JMP \reg), X86_FEATURE_IND_THUNK_JMP
+ int3 /* Halt straight-line speculation */
+
.size __x86_indirect_thunk_\reg, . - __x86_indirect_thunk_\reg
.type __x86_indirect_thunk_\reg, @function
.endm
Put insertion of INT3 behind CONFIG_SPECULATIVE_HARDEN_BRANCH
conditionals.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: New.
--- a/xen/arch/x86/indirect-thunk.S
+++ b/xen/arch/x86/indirect-thunk.S
@@ -11,8 +11,10 @@
#include <asm/asm_defns.h>
+#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
/* Don't transform the "ret" further down. */
.purgem ret
+#endif
.macro IND_THUNK_RETPOLINE reg:req
call 2f
@@ -45,7 +47,9 @@ ENTRY(__x86_indirect_thunk_\reg)
__stringify(IND_THUNK_LFENCE \reg), X86_FEATURE_IND_THUNK_LFENCE, \
__stringify(IND_THUNK_JMP \reg), X86_FEATURE_IND_THUNK_JMP
+#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
int3 /* Halt straight-line speculation */
+#endif
.size __x86_indirect_thunk_\reg, . - __x86_indirect_thunk_\reg
.type __x86_indirect_thunk_\reg, @function
--- a/xen/include/asm-x86/asm-defns.h
+++ b/xen/include/asm-x86/asm-defns.h
@@ -51,6 +51,8 @@
INDIRECT_BRANCH jmp \arg
.endm
+#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
+
/*
* To guard against speculation past RET, insert a breakpoint insn
* immediately after them.
@@ -66,3 +68,5 @@
.word \operand
.endif
.endm
+
+#endif
© 2016 - 2024 Red Hat, Inc.