Re: [PATCH] KVM: arm64: Tear down vGIC on failed vCPU creation

[Marc Zyngier <maz@kernel.org>]

On Fri, 14 Mar 2025 13:34:09 +0000,
Will Deacon <will@kernel.org> wrote:
> 
> If kvm_arch_vcpu_create() fails to share the vCPU page with the
> hypervisor, we propagate the error back to the ioctl but leave the
> vGIC vCPU data initialised. Note only does this leak the corresponding
> memory when the vCPU is destroyed but it can also lead to use-after-free
> if the redistributor device handling tries to walk into the vCPU.
> 
> Add the missing cleanup to kvm_arch_vcpu_create(), ensuring that the
> vGIC vCPU structures are destroyed on error.
> 
> Cc: <stable@vger.kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Quentin Perret <qperret@google.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
> 
> It's hard to come up with a "Fixes:" tag for this. Prior to 3f868e142c0b
> ("KVM: arm64: Introduce kvm_share_hyp()"), create_hyp_mappings() could
> still have failed, although if you go back before 66c57edd3bc7 ("KVM:
> arm64: Restrict EL2 stage-1 changes in protected mode") then it's
> vanishingly unlikely.
> 
>  arch/arm64/kvm/arm.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index b8e55a441282..fa71cee02faa 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -466,7 +466,11 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>  	if (err)
>  		return err;
>  
> -	return kvm_share_hyp(vcpu, vcpu + 1);
> +	err = kvm_share_hyp(vcpu, vcpu + 1);
> +	if (err)
> +		kvm_vgic_vcpu_destroy(vcpu);
> +
> +	return err;
>  }
>  
>  void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.