Re: [PATCH 1/4] kvm: svm: Streamline VMSA setting for VCPUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Dynamically freeing an active initial VMSA page during SNP AP creation without using SNP_PAGE_RECLAIM triggers an RMP violation, which bug-halts the VM and leaks memory.

Pre-existing issues:
- [Critical] A malicious SEV-SNP guest can crash the host kernel by punching a hole in a guest_memfd page that is actively assigned as an AP VMSA.
--

commit dfce748c773ed97c4a6e2b3a1ea5718862d5c1bf
Author: Joerg Roedel <joerg.roedel@amd.com>

kvm: svm: Streamline VMSA setting for VCPUs

This commit streamlines the VMSA setting state of vcpus, allowing a VMSA
to be either KVM-allocated or guest-provided. It consolidates the various
tracking states around VMSAs.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 0c3962647116e..9e00aa7bdca8c 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c

[ ... ]

> @@ -156,6 +159,173 @@ struct enc_region {

[ ... ]

> +static void sev_es_vcpu_free_vmsa(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	void *vmsa_ptr;
> +
> +	switch (svm->sev_es.vmsa.vmsa_state) {
> +	case VMSA_NONE:
> +	case VMSA_GUEST:
> +		break;
> +	case VMSA_PRIVATE:
> +		vmsa_ptr = page_address(svm->sev_es.vmsa.vmsa_page);
> +
> +		if (is_sev_snp_guest(vcpu)) {
> +			u64 pfn = __pa(vmsa_ptr) >> PAGE_SHIFT;
> +
> +			if (kvm_rmp_make_shared(vcpu->kvm, pfn, PG_LEVEL_4K)) {

[Severity: High]
Does this sequence trigger an RMP violation? 
    
When an SEV-SNP guest provisions an AP and triggers svm_vmgexit_ap_creation(),
KVM replaces the initial KVM-managed VMSA with the guest-provided one using
sev_es_set_guest_vmsa(), which eagerly frees the old VMSA via
sev_es_vcpu_free_vmsa().

For SNP guests, the old VMSA page is in the VMSA_PRIVATE state and has been
transitioned to the hardware VMSA state. To transition it back to shared
memory, AMD architecture requires reclaiming the page via snp_page_reclaim()
before issuing an RMPUPDATE.
    
Since sev_es_vcpu_free_vmsa() bypasses snp_page_reclaim() and directly calls
kvm_rmp_make_shared(), could this fail with an RMP violation, causing the VM
to bug-halt and leak the physical page?

> +				pr_err("Failed to make VMSA page shared - leaking it to avoid re-use\n");
> +				goto out;
> +			}
> +		}
> +
> +		if (vcpu->arch.guest_state_protected)
> +			sev_flush_encrypted_page(vcpu, vmsa_ptr);
> +
> +		fallthrough;

[ ... ]

> +static u64 sev_es_vmsa_pa(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	enum vmsa_state vmsa_state = svm->sev_es.vmsa.vmsa_state;
> +	u64 vmsa_pa = INVALID_PAGE;
> +
> +	if (vmsa_state == VMSA_GUEST) {

[ ... ]

> +		/*
> +		 * The new VMSA will be private memory guest memory, so retrieve the
> +		 * PFN from the gmem backend.
> +		 */
> +		if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, &page, NULL))
> +			goto out;
> +
> +		vmsa_pa = pfn_to_hpa(pfn);
> +
> +		/*
> +		 * gmem pages aren't currently migratable, but if this ever changes
> +		 * then care should be taken to ensure the guest vmsa is pinned
> +		 * through some other means.
> +		 */
> +		kvm_release_page_clean(page);

[Severity: Critical]
This is a pre-existing issue, but does dropping the page reference here allow
a malicious guest to crash the host kernel?

When a guest-provided VMSA is used for an AP, KVM resolves the GPA to a PFN
and immediately drops its page reference here. The physical address is then
programmed into the hardware VMCB.

If a malicious guest issues a Page State Change request to transition this VMSA
page back to shared memory, host userspace can handle this by calling
fallocate(PUNCH_HOLE) on the guest_memfd. This would trigger
sev_gmem_invalidate(), which attempts rmp_make_shared().

Because the page is still in the VMSA state, rmp_make_shared() would fail.
sev_gmem_invalidate() only prints a warning and proceeds, freeing the page
back to the buddy allocator while its RMP entry is still guest-restricted.

When the host buddy allocator reallocates and accesses this page, would it
trigger an RMP violation and a host kernel panic?

> +	} else if (vmsa_state == VMSA_PRIVATE || vmsa_state == VMSA_SHARED) {
> +		vmsa_pa = __pa(page_address(svm->sev_es.vmsa.vmsa_page));
> +	}
> +
> +out:
> +	return vmsa_pa;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611123528.572255-1-joro@8bytes.org?part=1