Re: [PATCH v2 04/28] KVM: apply nGPA->GPA translation to KVM_HC_CLOCK_PAIRING

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] kvm: L2 VM escape via unaligned KVM_HC_CLOCK_PAIRING write
--

commit 852f8a2ef0b30977a5795285699a50deabf3525c
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: apply nGPA->GPA translation to KVM_HC_CLOCK_PAIRING

This commit updates KVM_HC_CLOCK_PAIRING to translate the guest physical
address from an L2 address to an L1 address when running nested.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 79468ddfe4736..2b81e7a478c45 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7217,6 +7217,11 @@ static int kvm_pv_clock_pairing(struct kvm_vcpu *vcpu, gpa_t paddr,
>  	clock_pairing.flags = 0;
>  	memset(&clock_pairing.pad, 0, sizeof(clock_pairing.pad));
>  
> +	paddr = kvm_translate_gpa(vcpu, &vcpu->arch.gva_walk, paddr,
> +				  PFERR_WRITE_MASK | PFERR_GUEST_FINAL_MASK, NULL, 0);

[Severity: Critical]
Does this translation adequately handle the case where the struct crosses a
page boundary?

When kvm_translate_gpa() is called in kvm_pv_clock_pairing(), it only
translates the base L2 GPA. If an untrusted L2 guest provides an unaligned
guest physical address (for example, at offset 4033 in the page), the 64-byte
kvm_clock_pairing struct will span across two L2 pages.

> +	if (paddr == INVALID_GPA)
> +		return -KVM_EFAULT;
> +
>  	ret = 0;
>  	if (kvm_write_guest(vcpu->kvm, paddr, &clock_pairing,
>  			    sizeof(struct kvm_clock_pairing)))

[Severity: Critical]
Could this regression lead to L1 memory corruption when crossing an L2 page
boundary?

Looking at kvm_write_guest() in virt/kvm/kvm_main.c, it writes linearly into
L1 physical address space. When a write hits a page boundary, it increments
the L1 GFN (++gfn) and continues writing to the next contiguous L1 page:

virt/kvm/kvm_main.c:kvm_write_guest() {
    ...
    while ((seg = next_segment(len, offset)) != 0) {
        ret = kvm_write_guest_page(kvm, gfn, data, offset, seg);
        ...
        ++gfn;
    }
    ...
}

Since contiguous L2 pages are not guaranteed to map to contiguous L1 pages,
incrementing the L1 GFN directly could write the remainder of the 64-byte
struct into an unrelated L1 page. This appears to allow an L2 guest to
overwrite up to 63 bytes of L1 memory.

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