[PATCH v2] KVM: arm/arm64: vgic-its: Add error handling in vgic_its_cache_translation

[PATCH v2] KVM: arm/arm64: vgic-its: Add error handling in vgic_its_cache_translation

[Keisuke Nishimura]

The return value of xa_store() needs to be checked. This fix adds an
error handling path that resolves the kref inconsistency on failure. As
suggested by Oliver Upton, this function does not return the error code
intentionally because the translation cache is best effort.

Fixes: 8201d1028caa ("KVM: arm64: vgic-its: Maintain a translation cache per ITS")
Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
---
 arch/arm64/kvm/vgic/vgic-its.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 198296933e7e..a08835d00aba 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -573,12 +573,22 @@ static void vgic_its_cache_translation(struct kvm *kvm, struct vgic_its *its,
 	lockdep_assert_held(&its->its_lock);
 	vgic_get_irq_kref(irq);
 
+	old = xa_store(&its->translation_cache, cache_key, irq, GFP_KERNEL_ACCOUNT);
+
+	/*
+	 * Put the reference taken on @irq if the store fails. Intentionally do
+	 * not return the error as the translation cache is best effort.
+	 */
+	if (xa_is_err(old)) {
+		vgic_put_irq(kvm, irq);
+		return;
+	}
+
 	/*
 	 * We could have raced with another CPU caching the same
 	 * translation behind our back, ensure we don't leak a
 	 * reference if that is the case.
 	 */
-	old = xa_store(&its->translation_cache, cache_key, irq, GFP_KERNEL_ACCOUNT);
 	if (old)
 		vgic_put_irq(kvm, old);
 }
-- 
2.34.1

Re: [PATCH v2] KVM: arm/arm64: vgic-its: Add error handling in vgic_its_cache_translation

[Marc Zyngier]

On Sat, 30 Nov 2024 14:49:53 +0000,
Keisuke Nishimura <keisuke.nishimura@inria.fr> wrote:
> 
> The return value of xa_store() needs to be checked. This fix adds an
> error handling path that resolves the kref inconsistency on failure. As
> suggested by Oliver Upton, this function does not return the error code
> intentionally because the translation cache is best effort.
> 
> Fixes: 8201d1028caa ("KVM: arm64: vgic-its: Maintain a translation cache per ITS")
> Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
> ---
>  arch/arm64/kvm/vgic/vgic-its.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index 198296933e7e..a08835d00aba 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -573,12 +573,22 @@ static void vgic_its_cache_translation(struct kvm *kvm, struct vgic_its *its,
>  	lockdep_assert_held(&its->its_lock);
>  	vgic_get_irq_kref(irq);
>  
> +	old = xa_store(&its->translation_cache, cache_key, irq, GFP_KERNEL_ACCOUNT);
> +
> +	/*
> +	 * Put the reference taken on @irq if the store fails. Intentionally do
> +	 * not return the error as the translation cache is best effort.
> +	 */
> +	if (xa_is_err(old)) {
> +		vgic_put_irq(kvm, irq);
> +		return;
> +	}
> +
>  	/*
>  	 * We could have raced with another CPU caching the same
>  	 * translation behind our back, ensure we don't leak a
>  	 * reference if that is the case.
>  	 */
> -	old = xa_store(&its->translation_cache, cache_key, irq, GFP_KERNEL_ACCOUNT);
>  	if (old)
>  		vgic_put_irq(kvm, old);
>  }

Suggested-by: Oliver Upton <oliver.upton@linux.dev>
Acked-by: Marc Zyngier <maz@kernel.org>

	M.

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

Re: [PATCH v2] KVM: arm/arm64: vgic-its: Add error handling in vgic_its_cache_translation

[Oliver Upton]

On Sat, 30 Nov 2024 15:49:53 +0100, Keisuke Nishimura wrote:
> The return value of xa_store() needs to be checked. This fix adds an
> error handling path that resolves the kref inconsistency on failure. As
> suggested by Oliver Upton, this function does not return the error code
> intentionally because the translation cache is best effort.
> 
> 

Applied to fixes, thanks!

[1/1] KVM: arm/arm64: vgic-its: Add error handling in vgic_its_cache_translation
      https://git.kernel.org/kvmarm/kvmarm/c/be7e61127422

--
Best,
Oliver