Message ID | 20250430112350.335273952@infradead.org |
---|---|
State | New |
Headers | show |
Series | objtool: Detect and warn about indirect calls in __nocfi functions | expand |
From: Peter Zijlstra <peterz@infradead.org> Sent: Wednesday, April 30, 2025 4:08 AM > > Instead of using an indirect call to the hypercall page, use a direct > call instead. This avoids all CFI problems, including the one where > the hypercall page doesn't have IBT on. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > arch/x86/hyperv/hv_init.c | 60 +++++++++++++++++++++++----------------------- > 1 file changed, 30 insertions(+), 30 deletions(-) > > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -37,23 +37,41 @@ > void *hv_hypercall_pg; > > #ifdef CONFIG_X86_64 > +static u64 __hv_hyperfail(u64 control, u64 param1, u64 param2) > +{ > + return U64_MAX; > +} > + > +DEFINE_STATIC_CALL(__hv_hypercall, __hv_hyperfail); > + > u64 hv_std_hypercall(u64 control, u64 param1, u64 param2) > { > u64 hv_status; > > - if (!hv_hypercall_pg) > - return U64_MAX; > - > register u64 __r8 asm("r8") = param2; > - asm volatile (CALL_NOSPEC > + asm volatile ("call " STATIC_CALL_TRAMP_STR(__hv_hypercall) > : "=a" (hv_status), ASM_CALL_CONSTRAINT, > "+c" (control), "+d" (param1), "+r" (__r8) > - : THUNK_TARGET(hv_hypercall_pg) > - : "cc", "memory", "r9", "r10", "r11"); > + : : "cc", "memory", "r9", "r10", "r11"); > > return hv_status; > } > + > +typedef u64 (*hv_hypercall_f)(u64 control, u64 param1, u64 param2); > + > +static inline void hv_set_hypercall_pg(void *ptr) > +{ > + hv_hypercall_pg = ptr; > + > + if (!ptr) > + ptr = &__hv_hyperfail; > + static_call_update(__hv_hypercall, (hv_hypercall_f)ptr); > +} > #else > +static inline void hv_set_hypercall_pg(void *ptr) > +{ > + hv_hypercall_pg = ptr; > +} > EXPORT_SYMBOL_GPL(hv_hypercall_pg); > #endif > > @@ -348,7 +366,7 @@ static int hv_suspend(void) > * pointer is restored on resume. > */ > hv_hypercall_pg_saved = hv_hypercall_pg; > - hv_hypercall_pg = NULL; > + hv_set_hypercall_pg(NULL); > > /* Disable the hypercall page in the hypervisor */ > rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); > @@ -374,7 +392,7 @@ static void hv_resume(void) > vmalloc_to_pfn(hv_hypercall_pg_saved); > wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); > > - hv_hypercall_pg = hv_hypercall_pg_saved; > + hv_set_hypercall_pg(hv_hypercall_pg_saved); > hv_hypercall_pg_saved = NULL; > > /* > @@ -528,8 +546,8 @@ void __init hyperv_init(void) > if (hv_isolation_type_tdx() && !ms_hyperv.paravisor_present) > goto skip_hypercall_pg_init; > > - hv_hypercall_pg = __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, > - VMALLOC_END, GFP_KERNEL, PAGE_KERNEL_ROX, > + hv_hypercall_pg = __vmalloc_node_range(PAGE_SIZE, 1, MODULES_VADDR, > + MODULES_END, GFP_KERNEL, PAGE_KERNEL_ROX, Curiosity question (which I forgot ask about in v1): Is this change so that the hypercall page kernel address is "close enough" for the direct call to work from built-in code and from module code? Or is there some other reason? > VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > __builtin_return_address(0)); > if (hv_hypercall_pg == NULL) > @@ -567,27 +585,9 @@ void __init hyperv_init(void) > wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); > } > > -skip_hypercall_pg_init: > - /* > - * Some versions of Hyper-V that provide IBT in guest VMs have a bug > - * in that there's no ENDBR64 instruction at the entry to the > - * hypercall page. Because hypercalls are invoked via an indirect call > - * to the hypercall page, all hypercall attempts fail when IBT is > - * enabled, and Linux panics. For such buggy versions, disable IBT. > - * > - * Fixed versions of Hyper-V always provide ENDBR64 on the hypercall > - * page, so if future Linux kernel versions enable IBT for 32-bit > - * builds, additional hypercall page hackery will be required here > - * to provide an ENDBR32. > - */ > -#ifdef CONFIG_X86_KERNEL_IBT > - if (cpu_feature_enabled(X86_FEATURE_IBT) && > - *(u32 *)hv_hypercall_pg != gen_endbr()) { > - setup_clear_cpu_cap(X86_FEATURE_IBT); > - pr_warn("Disabling IBT because of Hyper-V bug\n"); > - } > -#endif Nit: With this IBT code removed, the #include <asm/ibt.h> at the top of this source code file should be removed. > + hv_set_hypercall_pg(hv_hypercall_pg); > > +skip_hypercall_pg_init: > /* > * hyperv_init() is called before LAPIC is initialized: see > * apic_intr_mode_init() -> x86_platform.apic_post_init() and > > The nit notwithstanding, Reviewed-by: Michael Kelley <mhklinux@outlook.com>
--- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -37,23 +37,41 @@ void *hv_hypercall_pg; #ifdef CONFIG_X86_64 +static u64 __hv_hyperfail(u64 control, u64 param1, u64 param2) +{ + return U64_MAX; +} + +DEFINE_STATIC_CALL(__hv_hypercall, __hv_hyperfail); + u64 hv_std_hypercall(u64 control, u64 param1, u64 param2) { u64 hv_status; - if (!hv_hypercall_pg) - return U64_MAX; - register u64 __r8 asm("r8") = param2; - asm volatile (CALL_NOSPEC + asm volatile ("call " STATIC_CALL_TRAMP_STR(__hv_hypercall) : "=a" (hv_status), ASM_CALL_CONSTRAINT, "+c" (control), "+d" (param1), "+r" (__r8) - : THUNK_TARGET(hv_hypercall_pg) - : "cc", "memory", "r9", "r10", "r11"); + : : "cc", "memory", "r9", "r10", "r11"); return hv_status; } + +typedef u64 (*hv_hypercall_f)(u64 control, u64 param1, u64 param2); + +static inline void hv_set_hypercall_pg(void *ptr) +{ + hv_hypercall_pg = ptr; + + if (!ptr) + ptr = &__hv_hyperfail; + static_call_update(__hv_hypercall, (hv_hypercall_f)ptr); +} #else +static inline void hv_set_hypercall_pg(void *ptr) +{ + hv_hypercall_pg = ptr; +} EXPORT_SYMBOL_GPL(hv_hypercall_pg); #endif @@ -348,7 +366,7 @@ static int hv_suspend(void) * pointer is restored on resume. */ hv_hypercall_pg_saved = hv_hypercall_pg; - hv_hypercall_pg = NULL; + hv_set_hypercall_pg(NULL); /* Disable the hypercall page in the hypervisor */ rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); @@ -374,7 +392,7 @@ static void hv_resume(void) vmalloc_to_pfn(hv_hypercall_pg_saved); wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); - hv_hypercall_pg = hv_hypercall_pg_saved; + hv_set_hypercall_pg(hv_hypercall_pg_saved); hv_hypercall_pg_saved = NULL; /* @@ -528,8 +546,8 @@ void __init hyperv_init(void) if (hv_isolation_type_tdx() && !ms_hyperv.paravisor_present) goto skip_hypercall_pg_init; - hv_hypercall_pg = __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, - VMALLOC_END, GFP_KERNEL, PAGE_KERNEL_ROX, + hv_hypercall_pg = __vmalloc_node_range(PAGE_SIZE, 1, MODULES_VADDR, + MODULES_END, GFP_KERNEL, PAGE_KERNEL_ROX, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, __builtin_return_address(0)); if (hv_hypercall_pg == NULL) @@ -567,27 +585,9 @@ void __init hyperv_init(void) wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); } -skip_hypercall_pg_init: - /* - * Some versions of Hyper-V that provide IBT in guest VMs have a bug - * in that there's no ENDBR64 instruction at the entry to the - * hypercall page. Because hypercalls are invoked via an indirect call - * to the hypercall page, all hypercall attempts fail when IBT is - * enabled, and Linux panics. For such buggy versions, disable IBT. - * - * Fixed versions of Hyper-V always provide ENDBR64 on the hypercall - * page, so if future Linux kernel versions enable IBT for 32-bit - * builds, additional hypercall page hackery will be required here - * to provide an ENDBR32. - */ -#ifdef CONFIG_X86_KERNEL_IBT - if (cpu_feature_enabled(X86_FEATURE_IBT) && - *(u32 *)hv_hypercall_pg != gen_endbr()) { - setup_clear_cpu_cap(X86_FEATURE_IBT); - pr_warn("Disabling IBT because of Hyper-V bug\n"); - } -#endif + hv_set_hypercall_pg(hv_hypercall_pg); +skip_hypercall_pg_init: /* * hyperv_init() is called before LAPIC is initialized: see * apic_intr_mode_init() -> x86_platform.apic_post_init() and
Instead of using an indirect call to the hypercall page, use a direct call instead. This avoids all CFI problems, including the one where the hypercall page doesn't have IBT on. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/x86/hyperv/hv_init.c | 60 +++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 30 deletions(-)