Re: [PATCH 21/31] KVM: Introduce a generation number for memory attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: infinite loop in mem_attributes checking due to xas_next() misuse
- [Low] kvm: loongarch: build failure due to missing closing parenthesis in unlikely()
--

commit ca19ec78b0ccf685019346276e3422a33ce7e2e0
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: Introduce a generation number for memory attributes

This commit introduces a generation number to track memory attribute
modifications in KVM. It ensures components accessing guest memory via cached
information validate the generation within a KVM SRCU read-side critical
section.

> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -163,7 +163,8 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
>  		return;
>  
>  	gpa &= KVM_STEAL_PHYS_MASK;
> -	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->generation) || gpa != ghc->gpa ||
> +	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->slots_generation, ghc->attrs_generation) ||
> +		     gpa != ghc->gpa ||
>  		     kvm_is_error_hva(ghc->hva) || !ghc->memslot) {

[Severity: Low]
Does this missing closing parenthesis for unlikely cause a build regression?

It looks like the macro is left unclosed, which breaks compilation on
LoongArch.

[ ... ]

> @@ -1808,7 +1809,8 @@ static void kvm_vcpu_set_pv_preempted(struct kvm_vcpu *vcpu)
>  
>  	gpa &= KVM_STEAL_PHYS_MASK;
>  	ghc = &vcpu->arch.st.cache;
> -	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->generation) || gpa != ghc->gpa ||
> +	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->slots_generation, ghc->attrs_generation) ||
> +		     gpa != ghc->gpa ||
>  		     kvm_is_error_hva(ghc->hva) || !ghc->memslot) {

[Severity: Low]
Does this missing closing parenthesis for unlikely cause a build regression
here too?

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
[ ... ]
> +bool kvm_range_has_any_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> +					unsigned long mask)
> +{
> +	XA_STATE(xas, &kvm->mem_attrs.array, start);
> +	void *entry;
[ ... ]
> +	guard(rcu)();
> +	for (;;) {
> +		do {
> +			entry = xas_next(&xas);
> +		} while (xas_retry(&xas, entry));
> +
> +		if (xas.xa_index >= end)
> +			break;

[Severity: High]
Can this loop regression lead to an infinite loop if the queried range extends
beyond the populated entries of the XArray?

When xas_next() returns NULL and transitions the state to XAS_BOUNDS,
subsequent calls to xas_next() do not increment xas.xa_index. Because xa_index
stops advancing, the termination condition xas.xa_index >= end is never met.

This can cause a hard lockup inside an RCU read-side critical section when
reachable via untrusted userspace ioctls.

The same xas_next() misuse appears to be present in
kvm_range_has_memory_attributes() which can cause it to incorrectly return
false for ranges beyond populated bounds due to the xas.xa_index != index
check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918135030.171564-2-pbonzini@redhat.com?part=21