Re: [PATCH 17/24] KVM: SEV: Unmap and unpin the GHCB as needed on vCPU free

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] `sev_es_unmap_ghcb` calls sleepable functions (`memunmap`) and performs unsafe guest memory writes while executing in an atomic context.
--

commit b4a31af8d74ea9fb855e6bc78164835ceed7d4bd
Author: Sean Christopherson <seanjc@google.com>

KVM: SEV: Unmap and unpin the GHCB as needed on vCPU free

Unmap and unpin the GHCB as needed when freeing a vCPU.  If the VM is
destroyed after mapping+pinning the GHCB on #VMGEXIT, without re-running
the vCPU, KVM will effectively leak the GHCB and any mappings created for
the GHCB.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 11d46600cbdc0..6c6a6d663e296 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
[ ... ]
> @@ -3552,6 +3552,20 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm)
>  	return 1;
>  }
>  
> +static void __sev_es_unmap_ghcb(struct vcpu_svm *svm)
> +{
> +	if (svm->sev_es.ghcb_sa_free) {
> +		kvfree(svm->sev_es.ghcb_sa);
> +		svm->sev_es.ghcb_sa = NULL;
> +		svm->sev_es.ghcb_sa_free = false;
> +	}
> +
> +	if (svm->sev_es.ghcb) {
> +		kvm_vcpu_unmap(&svm->vcpu, &svm->sev_es.ghcb_map);

[Severity: High]
This is a pre-existing issue, but does this execution path end up calling
sleepable functions and performing unsafe memory writes while preemption is
disabled?

vcpu_enter_guest() disables preemption before invoking
svm_prepare_switch_to_guest(), establishing an atomic execution context.
That function then calls sev_es_unmap_ghcb() for SEV-ES guests.

If a guest sets its GHCB GPA to an IOMEM region (e.g., an assigned device BAR),
kvm_vcpu_map() maps it using memremap(). When __sev_es_unmap_ghcb() later calls
kvm_vcpu_unmap() here, it delegates to memunmap(). Since memunmap() can sleep,
could this trigger a scheduling while atomic bug and allow a guest-triggered
host DoS?

Additionally, just before __sev_es_unmap_ghcb() is called, sev_es_unmap_ghcb()
calls kvm_write_guest():

void sev_es_unmap_ghcb(struct vcpu_svm *svm)
{
        ...
        if (svm->sev_es.ghcb_sa_sync) {
                kvm_write_guest(svm->vcpu.kvm, ...);
                ...
        }
        ...
}

If the guest scratch area is swapped out, the page fault handler will abort the
fault due to the atomic context, causing kvm_write_guest() to fail with -EFAULT.
Since KVM ignores this return value, could this lead to silent data corruption
in the guest, such as missing MMIO read data?

> +		svm->sev_es.ghcb = NULL;
> +	}
> +}

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