Re: [PATCH v2 18/28] 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: RCU stall in kvm_range_has_any_memory_attributes() due to XArray bounds mishandling
- [High] kvm: LoongArch compilation failure due to removed generation field
--

commit fa1df61c99941c6c01fe6e64de2dd5831dc8ad7d
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. It ensures that cached guest physical addresses and
permissions are validated against this generation before access.

> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index a568d8e6f4e82..7d911220e00d2 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -74,7 +74,8 @@ typedef u64            hfn_t;
>  typedef hfn_t kvm_pfn_t;
>  
>  struct gfn_to_hva_cache {
> -	u64 generation;
> +	u64 slots_generation;
> +	u64 attrs_generation;
>  	gpa_t gpa;
>  	unsigned long hva;

[Severity: High]
Does this change cause a compilation failure on LoongArch builds? 

It looks like kvm_update_stolen_time() in arch/loongarch/kvm/vcpu.c still 
contains a direct reference to the removed generation field:

    if (slots->generation != ghc->generation || gpa != ghc->gpa) {

Should this architecture-specific file be updated to check the new 
slots_generation and attrs_generation fields?

[ ... ]

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index eede017d41411..2d85d7c9e0046 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2480,6 +2481,51 @@ bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
>  	return true;
>  }
>  
> +/*
> + * Returns true if _any_ gfns in the range [@start, @end) have attributes that
> + * match _any_ bit in @mask.
> + */
> +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;
> +
> +	mask &= kvm_supported_mem_attributes(kvm);
> +	if (!mask)
> +		return false;
> +
> +	if (end == start + 1)
> +		return !!(kvm_get_memory_attributes(kvm, start) & mask);
> +
> +	guard(rcu)();
> +	for (;;) {
> +		do {
> +			entry = xas_next(&xas);
> +		} while (xas_retry(&xas, entry));
> +
> +		if (xas.xa_index >= end)
> +			break;

[Severity: High]
Can this loop stall infinitely if a user provides a range that extends 
beyond the populated indices of the mem_attrs XArray? 

If xas_next() goes out of bounds, it returns NULL and sets the state to 
XAS_BOUNDS, which means xas.xa_index stops advancing. Because the loop 
relies strictly on xas.xa_index >= end to terminate, an out-of-bounds 
query would never break out of the loop and cause an RCU stall.

> +
> +		if (xa_to_value(entry) & mask)
> +			return true;
> +	}
> +
> +	return false;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918081543.139871-1-pbonzini@redhat.com?part=18