Re: [PATCH v2 05/28] KVM: x86: Introduce memory fault on invalid hypercalls reads/writes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] x86/hyperv: spurious memory fault on HVCALL_SIGNAL_EVENT due to double validation
--

commit 9e7f4342fa28fd49b13df88579465f8fb51e4a93
Author: Paolo Bonzini <pbonzini@redhat.com>
KVM: x86: Introduce memory fault on invalid hypercalls reads/writes

This commit allows userspace to perform arbitrary actions when a hypercall
refers to an invalid address, by exiting with KVM_EXIT_MEMORY_FAULT.

> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 604651cb27399..9f5adcd26cba3 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -2547,11 +2617,30 @@ static bool hv_check_hypercall_access(struct kvm_vcpu_hv *hv_vcpu, u16 code)
>  	return true;
>  }
>  
> +static int kvm_hv_hypercall_userspace_exit(struct kvm_vcpu *vcpu,
> +					   struct kvm_hv_hcall *hc)
> +{
> +	int r;
> +
> +	r = kvm_hv_hypercall_check_params(vcpu, hc);
> +	if (r)
> +		return r;

[Severity: High]
Does this code perform a physical memory validation on hc->ingpa, which has
already been overwritten with the connection ID for slow hypercalls?

When a guest makes a slow HVCALL_SIGNAL_EVENT hypercall with a connection ID
that is valid in userspace but not registered in the kernel conn_to_evt,
kvm_hvcall_signal_event() reads the connection ID and overwrites hc->ingpa:

kvm_hvcall_signal_event() {
	if (unlikely(!hc->fast)) {
		...
		ret = kvm_vcpu_read_guest(vcpu, gpa,
					  &hc->ingpa, sizeof(hc->ingpa));
	...
}

It then returns HV_STATUS_INVALID_PORT_ID, and the switch statement in
kvm_hv_hypercall() falls through to HVCALL_POST_MESSAGE and calls
kvm_hv_hypercall_userspace_exit().

Will this cause a spurious KVM_EXIT_MEMORY_FAULT instead of a normal
KVM_EXIT_HYPERV because kvm_hv_hypercall_check_params() incorrectly
re-validates the overwritten hc->ingpa as a GPA?

> +
> +	vcpu->run->exit_reason = KVM_EXIT_HYPERV;
> +	vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
> +	vcpu->run->hyperv.u.hcall.input = hc->param;
> +	vcpu->run->hyperv.u.hcall.params[0] = hc->ingpa;
> +	vcpu->run->hyperv.u.hcall.params[1] = hc->outgpa;
> +	vcpu->arch.complete_userspace_io = kvm_hv_hypercall_complete_userspace;
> +	return 0;
> +}

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