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

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

[Will Deacon]

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)
-- 
2.49.0.rc1.451.g8f38331e32-goog

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

[Marc Zyngier]

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.

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

[Oliver Upton]

On Fri, 14 Mar 2025 13:34:09 +0000, Will Deacon 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.
> 
> [...]

Applied to next, thanks!

[1/1] KVM: arm64: Tear down vGIC on failed vCPU creation
      https://git.kernel.org/kvmarm/kvmarm/c/250f25367b58

--
Best,
Oliver