Re: [PATCH 03/24] KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unconditionally validates the input parameter GPA (`hc.ingpa`) for all slow hypercalls, causing Guest DoS for hypercalls without input parameters.
- [High] Improperly enforces write access on the input parameter block (`hc.ingpa`) and fails to validate the output parameter block (`hc.outgpa`).
- [High] Treats untranslated L2 GPAs as L1 GPAs, allowing an L2 guest to trivially DoS the L1 VMM.
- [Medium] Reports an inaccurate fault range to userspace by using an unaligned GPA and a hardcoded PAGE_SIZE.
- [Medium] Unsynchronized mutability of the `KVM_CAP_HYPERV_HCALL_FAULT_EXIT` capability violates KVM guidelines.
--

--- Patch [3]: [PATCH 03/24] KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas ---
commit 2ee0ff5e023dcf16d5e8bf3cfc26d7ec91c1d297
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas

This commit introduces a new capability, KVM_CAP_HYPERV_HCALL_FAULT_EXIT,
which allows KVM to exit to userspace with a memory fault when a guest
provides an invalid input GPA during a hypercall.

> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
[ ... ]
> @@ -2531,11 +2531,30 @@ static bool hv_check_hypercall_access(struct kvm_vcpu_hv *hv_vcpu, u16 code)
>  	return true;
>  }
>  
> +static unsigned int kvm_hv_hypercall_mem_access(u16 code)
> +{
> +	switch (code) {
> +	case HVCALL_SIGNAL_EVENT:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
> +	case HVCALL_SEND_IPI:
> +	case HVCALL_SEND_IPI_EX:
> +		return KVM_MEMORY_EXIT_FLAG_READ;
> +	}
> +
> +	return KVM_MEMORY_EXIT_FLAG_WRITE;

[Severity: High]
Does this default to requiring write access for all unlisted hypercalls?

If a guest issues an unlisted hypercall (e.g., HVCALL_POST_MESSAGE) with its
input parameter block in read-only memory, this would return
KVM_MEMORY_EXIT_FLAG_WRITE, causing KVM to incorrectly demand write access
and reject a valid read-only buffer with a memory fault.

> +}
> +
>  int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
> +	struct kvm *kvm = vcpu->kvm;
>  	struct kvm_hv_hcall hc;
>  	u64 ret = HV_STATUS_SUCCESS;
> +	unsigned int access;
> +	unsigned long addr;
>  
>  	/*
>  	 * hypercall generates UD from non zero cpl and real mode
> @@ -2590,6 +2609,19 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  		kvm_hv_hypercall_read_xmm(&hc);
>  	}
>  
> +	if (!hc.fast && kvm->arch.hyperv.hcall_fault_exit) {
> +		bool writable = true;
> +		access = kvm_hv_hypercall_mem_access(hc.code);
> +		addr = kvm_vcpu_gfn_to_hva_prot(vcpu, gpa_to_gfn(hc.ingpa), &writable);

[Severity: High]
Does this code unconditionally validate hc.ingpa for all slow hypercalls?

If a guest invokes a hypercall that does not take an input parameter
(like HVCALL_NOTIFY_LONG_SPIN_WAIT), its parameter registers may be
uninitialized. If the uninitialized register contains an unmapped GPA,
wouldn't KVM inject a KVM_EXIT_MEMORY_FAULT and return -EFAULT, causing
userspace to terminate the VM?

[Severity: High]
Are untranslated L2 GPAs treated as L1 GPAs here?

When an L2 guest executes a hypercall, KVM reads hc.ingpa and blindly passes
it to kvm_vcpu_gfn_to_hva_prot() without translating it or checking
is_guest_mode().

If an L2 guest passes an invalid L1 GPA, this could return -EFAULT and
trigger a KVM_EXIT_MEMORY_FAULT to L0 userspace, terminating the entire
L1 VM. Could this allow an L2 guest to trivially crash L1?

> +		if (addr == KVM_HVA_ERR_BAD ||
> +		    (access == KVM_MEMORY_EXIT_FLAG_WRITE && !writable)) {

[Severity: High]
Is the output parameter block (hc.outgpa) validated anywhere in this new
mechanism?

It looks like we enforce the write access requirement on the input
parameter GPA, but completely omit validation for hc.outgpa. Hypercalls
writing to unmapped memory might bypass the memory fault mechanism entirely.

> +			kvm_prepare_memory_fault_exit(vcpu, hc.ingpa, PAGE_SIZE,
> +						      access == KVM_MEMORY_EXIT_FLAG_WRITE,
> +						      false, false);

[Severity: Medium]
Could this report an inaccurate fault range to userspace?

We are preparing a memory fault exit using the exact unaligned address
provided by the guest (hc.ingpa) but with a hardcoded size of PAGE_SIZE. If
hc.ingpa is near a page boundary, the reported fault range would span two
pages, even though we only validated a single GFN for hc.ingpa.

> +			return -EFAULT;
> +		}
> +	}
> +
>  	switch (hc.code) {
>  	case HVCALL_NOTIFY_LONG_SPIN_WAIT:
[ ... ]
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -4190,6 +4191,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  		mutex_unlock(&kvm->lock);
>  		break;
>  	}
> +#ifdef CONFIG_KVM_HYPERV
> +	case KVM_CAP_HYPERV_HCALL_FAULT_EXIT:
> +		kvm->arch.hyperv.hcall_fault_exit = cap->args[0];

[Severity: Medium]
Should this capability state be protected by kvm->lock and restricted to
before vCPUs are created?

Userspace can toggle the capability dynamically, and modifying this flag
without checks creates state-machine transitions that could race with actively
executing hypercalls. New flags that change VM or vCPU behavior typically
default to immutable once vCPUs are running.

> +		r = 0;
> +		break;
> +#endif
>  	default:
>  		r = -EINVAL;
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=3