Re: [PATCH] KVM: SVM: Clear AVIC Physical ID table entry if vCPU creation fails

[Atish Patra <atish.patra@linux.dev>]

On 9/2/26 11:28 PM, Naveen N Rao (AMD) wrote:
> If vCPU creation fails after kvm_arch_vcpu_create(), the AVIC Physical
> ID table entry corresponding to that vCPU continues to point to the
> freed APIC backing page which can result in UAF. Address this by
> clearing out the corresponding AVIC Physical ID table entry in the
> vcpu_free() callback, similar to the VMX commit b41f2ca6c060 ("KVM: VMX:
> Fix stale PID-pointer table entry left after vCPU free").
>
> Though unlikely, it is also possible that svm_vcpu_create() itself fails
> after the AVIC Physical ID table entry has been setup if memory
> allocation fails in svm_vcpu_alloc_msrpm(). Clear the entry in this path
> as well.
>
> Note: this change depends on commit 97d65b544f48 ("KVM: Check for
> duplicate vcpu_id as early as possible"), which ensures that a vCPU with
> a duplicate ID is never created. Otherwise, a valid AVIC Physical ID
> table entry for an existing vCPU will be cleared.
>
> Fixes: 44a95dae1d22 ("KVM: x86: Detect and Initialize AVIC support")
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> ---
>   arch/x86/kvm/svm/svm.h  |  1 +
>   arch/x86/kvm/svm/avic.c | 10 ++++++++++
>   arch/x86/kvm/svm/svm.c  |  6 +++++-
>   3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index e958943b8162..790bd96a9791 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -954,6 +954,7 @@ void avic_init_vmcb(struct vcpu_svm *svm, struct vmcb *vmcb);
>   int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu);
>   int avic_unaccelerated_access_interception(struct kvm_vcpu *vcpu);
>   int avic_init_vcpu(struct vcpu_svm *svm);
> +void avic_vcpu_free(struct kvm_vcpu *vcpu);
>   void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
>   void avic_vcpu_put(struct kvm_vcpu *vcpu);
>   void avic_apicv_post_state_restore(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 3b037e385523..bc2c699380b3 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -885,6 +885,16 @@ int avic_init_vcpu(struct vcpu_svm *svm)
>   	return ret;
>   }
>   
> +void avic_vcpu_free(struct kvm_vcpu *vcpu)
> +{
> +	u32 max_id = x2avic_enabled ? x2avic_max_physical_id : AVIC_MAX_PHYSICAL_ID;
> +	struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
> +	u32 id = vcpu->vcpu_id;
> +
> +	if (kvm_svm->avic_physical_id_table && id <= max_id)
> +		WRITE_ONCE(kvm_svm->avic_physical_id_table[id], 0);
> +}
> +
>   void avic_apicv_post_state_restore(struct kvm_vcpu *vcpu)
>   {
>   	avic_handle_dfr_update(vcpu);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ea647938a2a6..c2a2ae7d8ec3 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1337,7 +1337,7 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
>   	svm->msrpm = svm_vcpu_alloc_msrpm();
>   	if (!svm->msrpm) {
>   		err = -ENOMEM;
> -		goto error_free_sev;
> +		goto error_free_avic;
>   	}
>   
>   	svm->x2avic_msrs_intercepted = true;
> @@ -1351,6 +1351,8 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
>   
>   	return 0;
>   
> +error_free_avic:
> +	avic_vcpu_free(vcpu);
>   error_free_sev:
>   	sev_free_vcpu(vcpu);
>   error_free_vmcb_page:
> @@ -1365,6 +1367,8 @@ static void svm_vcpu_free(struct kvm_vcpu *vcpu)
>   
>   	WARN_ON_ONCE(!list_empty(&svm->ir_list));
>   
> +	avic_vcpu_free(vcpu);
> +
>   	svm_leave_nested(vcpu);
>   	svm_free_nested(svm);
>   
>
> base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97

FWIW, we had the same bug report from kres. This patch fixes that.
Verified on AMD beragmo with x2AVIC enabled and a reproducer that makes 
KVM_CREATE_VCPU fail with EMFILE after the entry is published.


Tested-by: Atish Patra <atishp@meta.com>