Re: [PATCH 11/24] KVM: Take memory protections into account in kvm_read/write_guest()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Cached guest memory accesses bypass memory attribute protections.
--

--- Patch [11]: [PATCH 11/24] KVM: Take memory protections into account in kvm_read/write_guest() ---
commit 2db3d9f175080ff067377dd7b0148b9955953e26
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: Take memory protections into account in kvm_read/write_guest()
    
This patch adds memory attribute checks to the kvm_read_guest() and
kvm_write_guest() family of functions. It plumbs the kvm struct to the
internal page accessors so they can query memory attributes. If the
attributes do not permit the access, the functions return an error.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 56016aab0aad3..d920ce5b67392 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c

[ ... ]

> @@ -3261,6 +3261,10 @@ static int __kvm_read_guest_page(struct kvm_memory_slot *slot, gfn_t gfn,
>  	addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
>  	if (kvm_is_error_hva(addr))
>  		return -EFAULT;
> +
> +	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
> +		return -EFAULT;
> +

[Severity: Critical]
Since memory attributes are now enforced for standard guest reads and writes,
do the cached memory accessors also need to enforce these protections?

Functions like kvm_read_guest_offset_cached() and
kvm_write_guest_offset_cached() appear to read and write directly using the
cached HVA without verifying attributes:

virt/kvm/kvm_main.c:kvm_read_guest_offset_cached() {
    ...
    if (kvm_is_error_hva(ghc->hva))
        return -EFAULT;
    ...
    r = __copy_from_user(data, (void __user *)ghc->hva + offset, len);
    ...
}

Similarly, host kernel mapping paths like __kvm_vcpu_map() use
kvm_follow_pfn() and map guest memory without checking attributes:

virt/kvm/kvm_main.c:__kvm_vcpu_map() {
    struct kvm_follow_pfn kfp = {
        .slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn),
        .gfn = gfn,
        ...
    };
    ...
    map->pfn = kvm_follow_pfn(&kfp);
    ...
}

And gpc resolving paths like hva_to_pfn_retry() in virt/kvm/pfncache.c also
rely on kvm_follow_pfn() directly.

Could a malicious guest bypass memory attribute protections (e.g., read/write
restrictions or private memory boundaries) by pointing a cached memory
structure, such as a virtio ring or APIC, to restricted memory?

Also, if memory attributes are dynamically updated, how do we ensure
gfn_to_hva_cache and gfn_to_pfn_cache are properly invalidated, given that
attribute updates don't seem to bump the memslot generation or trigger
gfn_to_pfn_cache_invalidate_start()?

>  	r = __copy_from_user(data, (void __user *)addr + offset, len);
>  	if (r)
>  		return -EFAULT;

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