| 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
| ||
|
Message-ID:
<SN6PR02MB41577ED2C4E29F25B82548D7D4822@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Thu, 1 May 2025 02:36:26 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Peter Zijlstra <peterz@...radead.org>, "x86@...nel.org" <x86@...nel.org>
CC: "kys@...rosoft.com" <kys@...rosoft.com>, "haiyangz@...rosoft.com"
<haiyangz@...rosoft.com>, "wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>, "tglx@...utronix.de"
<tglx@...utronix.de>, "mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de"
<bp@...en8.de>, "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"hpa@...or.com" <hpa@...or.com>, "seanjc@...gle.com" <seanjc@...gle.com>,
"pbonzini@...hat.com" <pbonzini@...hat.com>, "ardb@...nel.org"
<ardb@...nel.org>, "kees@...nel.org" <kees@...nel.org>, Arnd Bergmann
<arnd@...db.de>, "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"jpoimboe@...nel.org" <jpoimboe@...nel.org>, "linux-hyperv@...r.kernel.org"
<linux-hyperv@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
"samitolvanen@...gle.com" <samitolvanen@...gle.com>, "ojeda@...nel.org"
<ojeda@...nel.org>
Subject: RE: [PATCH v2 12/13] x86_64,hyperv: Use direct call to hypercall-page
From: Peter Zijlstra <peterz@...radead.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@...radead.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@...look.com>
Powered by blists - more mailing lists