Re: [PATCH 15/24] KVM: SEV: Move sev_free_vcpu() down below sev_es_unmap_ghcb()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] `sev_es_unmap_ghcb` executes a sleepable operation (`kvm_write_guest`) while preemption is disabled, causing a sleeping-while-atomic violation.
--

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

KVM: SEV: Move sev_free_vcpu() down below sev_es_unmap_ghcb()

Relocate sev_free_vcpu() down in sev.c so that it's definition comes after
sev_es_unmap_ghcb().  This will allow sharing unmap functionality between
the two functions without needing a forward declaration (or weird placement
of the common code).

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 4ebe0d4497898..437282f0ea943 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
[ ... ]
> @@ -3618,6 +3587,37 @@ void sev_es_unmap_ghcb(struct vcpu_svm *svm)
>  	svm->sev_es.ghcb = NULL;
>  }

[Severity: Critical]
This is a pre-existing issue, but since we are looking at sev_es_unmap_ghcb(),
does this unmap routine execute a sleepable operation while preemption is
disabled?

When handling a guest exit where the SEV-ES scratch area is outside the GHCB,
ghcb_sa_sync can be set to true. If the IO requires exiting to userspace,
sev_es_unmap_ghcb() is delayed until the next vcpu_enter_guest() execution.

During guest reentry, vcpu_enter_guest() disables preemption before calling
the architecture hook:

arch/x86/kvm/x86.c:vcpu_enter_guest() {
    ...
	preempt_disable();

	kvm_x86_call(prepare_switch_to_guest)(vcpu);
    ...
}

For SVM, this hook resolves to svm_prepare_switch_to_guest(), which calls
sev_es_unmap_ghcb() while preemption is still disabled:

arch/x86/kvm/svm/svm.c:svm_prepare_switch_to_guest() {
    ...
	if (is_sev_es_guest(vcpu))
		sev_es_unmap_ghcb(svm);
    ...
}

Inside sev_es_unmap_ghcb(), if ghcb_sa_sync is true, it calls kvm_write_guest()
which is a sleepable operation:

arch/x86/kvm/svm/sev.c:sev_es_unmap_ghcb() {
    ...
		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);
    ...
}

Calling a sleepable function with preemption disabled leads to a "sleeping
function called from invalid context" bug. If the host virtual address
corresponding to the scratch area is not resident in memory, the kernel will
attempt to sleep on the page fault, leading to a "scheduling while atomic"
panic.

Could this be triggered by an unprivileged guest providing an out-of-bounds
scratch area, leading to a host denial of service?

> +void sev_free_vcpu(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm;
> +
> +	if (!is_sev_es_guest(vcpu))
> +		return;
[ ... ]

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