Re: [PATCH 16/24] KVM: SEV: Decouple the need to sync the GHCB SA from the need to free the SA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] kvm_write_guest() is called with preemption disabled, which can trigger a kernel warning or silent guest memory corruption.
--

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

KVM: SEV: Decouple the need to sync the GHCB SA from the need to free the SA
    
This commit decouples synchronizing the GHCB scratch area from freeing and
unpinning it. This change allows the free and unpin path to be reused when
freeing a vCPU, and opportunistically adds a warning to prevent stomping over
an already-allocated scratch area.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 437282f0ea943..11d46600cbdc0 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3560,20 +3560,17 @@ void sev_es_unmap_ghcb(struct vcpu_svm *svm)
>  	if (!svm->sev_es.ghcb)
>  		return;
>  
> -	if (svm->sev_es.ghcb_sa_free) {
> -		/*
> -		 * The scratch area lives outside the GHCB, so there is a
> -		 * buffer that, depending on the operation performed, may
> -		 * need to be synced, then freed.
> -		 */
> -		if (svm->sev_es.ghcb_sa_sync) {
> -			kvm_write_guest(svm->vcpu.kvm,
> -					svm->sev_es.sw_scratch,
> -					svm->sev_es.ghcb_sa,
> -					svm->sev_es.ghcb_sa_len);
> -			svm->sev_es.ghcb_sa_sync = false;
> -		}
> +	/*
> +	 * If the scratch area lives outside the GHCB, there's a buffer that,
> +	 * depending on the operation performed, may need to be synced.
> +	 */
> +	if (svm->sev_es.ghcb_sa_sync) {
> +		kvm_write_guest(svm->vcpu.kvm, svm->sev_es.sw_scratch,
> +				svm->sev_es.ghcb_sa, svm->sev_es.ghcb_sa_len);
> +		svm->sev_es.ghcb_sa_sync = false;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but can this call to
kvm_write_guest() trigger a warning or cause silent guest memory corruption?

In arch/x86/kvm/x86.c:vcpu_enter_guest(), KVM disables preemption before
calling prepare_switch_to_guest():

	preempt_disable();

	kvm_x86_call(prepare_switch_to_guest)(vcpu);

For SEV-ES guests, this invokes svm_prepare_switch_to_guest() ->
sev_es_unmap_ghcb(). Within sev_es_unmap_ghcb(), if svm->sev_es.ghcb_sa_sync is
true, KVM calls kvm_write_guest().

Since kvm_write_guest() is a sleepable function and preemption is disabled,
this could trigger a warning in might_fault(), potentially leading to a host
denial of service if panic_on_warn is set.

Additionally, if the user address faults, the page fault handler might detect
the atomic context and fail the fault, causing the copy to fail silently and
corrupting the guest's execution.

Could a guest using GHCB version 1 force ghcb_sa_sync to true by supplying a
scratch area outside the GHCB, thereby triggering this path?

[ ... ]

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