| Message ID | 792c442d-c05a-7a00-c807-b94a54bca94f@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: XSA-348 follow-up | expand |
On Tue, Dec 15, 2020 at 05:11:41PM +0100, Jan Beulich wrote: > It is imperative that the functions passed here are taking no arguments, > return no values, and don't return in the first place. While the type > can be checked uniformly, the attribute check is limited to gcc 9 and > newer (no clang support for this so far afaict). > > Note that I didn't want to have the "true" fallback "implementation" of > __builtin_has_attribute(..., __noreturn__) generally available, as > "true" may not be a suitable fallback in other cases. > > Note further that the noreturn addition to startup_cpu_idle_loop()'s > declaration requires adding unreachable() to Arm's > switch_stack_and_jump(), or else the build would break. I suppose this > should have been there already. > > For vmx_asm_do_vmentry() along with adding the attribute, also restrict > its scope. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > Reviewed-by: Juergen Gross <jgross@suse.com> Reviewed-by: Wei Liu <wl@xen.org>
Hi Jan, On 15/12/2020 16:11, Jan Beulich wrote: > It is imperative that the functions passed here are taking no arguments, > return no values, and don't return in the first place. While the type > can be checked uniformly, the attribute check is limited to gcc 9 and > newer (no clang support for this so far afaict). > > Note that I didn't want to have the "true" fallback "implementation" of > __builtin_has_attribute(..., __noreturn__) generally available, as > "true" may not be a suitable fallback in other cases. > > Note further that the noreturn addition to startup_cpu_idle_loop()'s > declaration requires adding unreachable() to Arm's > switch_stack_and_jump(), or else the build would break. I suppose this > should have been there already. > > For vmx_asm_do_vmentry() along with adding the attribute, also restrict > its scope. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > Reviewed-by: Juergen Gross <jgross@suse.com> For the Arm bits: Acked-by: Julien Grall <jgrall@amazon.com> Cheers,
--- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -63,7 +63,7 @@ #include <asm/monitor.h> #include <asm/xstate.h> -void svm_asm_do_resume(void); +void noreturn svm_asm_do_resume(void); u32 svm_feature_flags; --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -1850,6 +1850,8 @@ void vmx_vmentry_failure(void) domain_crash(curr->domain); } +void noreturn vmx_asm_do_vmentry(void); + void vmx_do_resume(void) { struct vcpu *v = current; --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -619,7 +619,7 @@ static inline bool using_2M_mapping(void !l1_table_offset((unsigned long)__2M_rwdata_end); } -static void noinline init_done(void) +static void noreturn init_done(void) { void *va; unsigned long start, end; --- a/xen/include/asm-arm/current.h +++ b/xen/include/asm-arm/current.h @@ -43,8 +43,10 @@ static inline struct cpu_info *get_cpu_i #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs) -#define switch_stack_and_jump(stack, fn) \ - asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" ) +#define switch_stack_and_jump(stack, fn) do { \ + asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" ); \ + unreachable(); \ +} while ( false ) #define reset_stack_and_jump(fn) switch_stack_and_jump(get_cpu_info(), fn) --- a/xen/include/asm-x86/asm_defns.h +++ b/xen/include/asm-x86/asm_defns.h @@ -23,7 +23,7 @@ asm ( "\t.equ CONFIG_INDIRECT_THUNK, " #include <asm/indirect_thunk_asm.h> #ifndef __ASSEMBLY__ -void ret_from_intr(void); +void noreturn ret_from_intr(void); /* * This output constraint should be used for any inline asm which has a "call" --- a/xen/include/asm-x86/current.h +++ b/xen/include/asm-x86/current.h @@ -155,9 +155,18 @@ unsigned long get_stack_dump_bottom (uns # define SHADOW_STACK_WORK "" #endif +#if __GNUC__ >= 9 +# define ssaj_has_attr_noreturn(fn) __builtin_has_attribute(fn, __noreturn__) +#else +/* Simply can't check the property with older gcc. */ +# define ssaj_has_attr_noreturn(fn) true +#endif + #define switch_stack_and_jump(fn, instr, constr) \ ({ \ unsigned int tmp; \ + (void)((fn) == (void (*)(void))NULL); \ + BUILD_BUG_ON(!ssaj_has_attr_noreturn(fn)); \ __asm__ __volatile__ ( \ SHADOW_STACK_WORK \ "mov %[stk], %%rsp;" \ --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -93,7 +93,6 @@ typedef enum { #define PI_xAPIC_NDST_MASK 0xFF00 void vmx_asm_vmexit_handler(struct cpu_user_regs); -void vmx_asm_do_vmentry(void); void vmx_intr_assist(void); void noreturn vmx_do_resume(void); void vmx_vlapic_msr_changed(struct vcpu *v); --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -736,7 +736,7 @@ void sched_context_switched(struct vcpu void continue_running( struct vcpu *same); -void startup_cpu_idle_loop(void); +void noreturn startup_cpu_idle_loop(void); extern void (*pm_idle) (void); extern void (*dead_idle) (void);