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

[Marc Zyngier <maz@kernel.org>]

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.