On 25/04/2025 5:51 pm, Ariadne Conill wrote:
> diff --git a/xen/arch/x86/guest/hyperv/hyperv.c
> b/xen/arch/x86/guest/hyperv/hyperv.c
> index 6989af38f1..637b4bf335 100644
> --- a/xen/arch/x86/guest/hyperv/hyperv.c
> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> @@ -98,10 +97,22 @@ static void __init setup_hypercall_page(void)
> rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> if ( !hypercall_msr.enable )
> {
> - mfn = HV_HCALL_MFN;
> + hv_hcall_page = alloc_xenheap_page();
> + if ( !hv_hcall_page )
> + {
> + printk("Hyper-V: Failed to allocate hypercall trampoline
> page\n");
> + return -ENOMEM;
> + }
> +
> + printk("Hyper-V: Allocated hypercall page @ %p.\n", hv_hcall_page);
> +
> + mfn = virt_to_mfn(hv_hcall_page);
Up to here is ok; this is just choosing a different page, but...
> hypercall_msr.enable = 1;
> hypercall_msr.guest_physical_address = mfn;
> wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +
> + start = (unsigned long) hv_hcall_page;
> + modify_xen_mappings(start, start + PAGE_SIZE, PAGE_HYPERVISOR_RX);
... this and ...
> }
> else
> mfn = hypercall_msr.guest_physical_address;
> @@ -109,9 +120,9 @@ static void __init setup_hypercall_page(void)
> rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> BUG_ON(!hypercall_msr.enable);
>
> - set_fixmap_x(FIX_X_HYPERV_HCALL, mfn << PAGE_SHIFT);
... this break the case where the overlay is already chosen and cannot move.
It really needs to stay using set_fixmap_x(), which in turn means you
can ...
> diff --git a/xen/arch/x86/include/asm/guest/hyperv-hcall.h
> b/xen/arch/x86/include/asm/guest/hyperv-hcall.h
> index b76dbf9ccc..b73edca7c6 100644
> --- a/xen/arch/x86/include/asm/guest/hyperv-hcall.h
> +++ b/xen/arch/x86/include/asm/guest/hyperv-hcall.h
> @@ -20,13 +20,13 @@ static inline uint64_t hv_do_hypercall(uint64_t control,
> paddr_t input_addr,
> paddr_t output_addr)
> {
> uint64_t status;
> - register unsigned long r8 asm ( "r8" ) = output_addr;
>
> /* See TLFS for volatile registers */
> - asm volatile ( "call hv_hcall_page"
> + asm volatile ( "mov %[output_addr], %%r8\n"
> + "call *%[target_addr]"
> : "=a" (status), "+c" (control),
> "+d" (input_addr) ASM_CALL_CONSTRAINT
> - : "r" (r8)
> + : [output_addr] "r" (output_addr), [target_addr] "r"
> (hv_hcall_page)
> : "memory" );
... undo this speculative security vulnerability you've got by not using
INDIRECT_CALL.
The point of FIXMAP_X is to provide a virtual address in the main 1G
range for .text/.data/.rodata/.bss, which can point to an arbitrary
location, and can be regularly CALL'd.
~Andrew
