Re: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Stage-1 huge block mappings (e.g., 4TB, 64GB) are incorrectly truncated to 1GB due to a 32-bit return type and a hardcoded fallback, leading to under-invalidation of the VNCR software cache.
--

--- Patch [2]: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation ---
commit 55a3817e6985fb11462e68c9b396c2d3ee056461
Author: Marc Zyngier <[email protected]>

KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation

This patch fixes a bug where negative Stage-1 mapping levels (used to indicate
S1_MMU_DISABLED) were mishandled by pgshift_level_to_ttl(), leading to an
invalidation size of 0. It introduces __ttl_to_size() and ensures ttl_to_size()
always falls back to SZ_1G when no valid TTL size is found.

Link: https://lore.kernel.org/r/ameGoxbn2wzBq2kL@v4bel

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 983ba0adaf4e6..b026d160f253d 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c

[ ... ]

> @@ -506,7 +506,7 @@ int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
>  	return ret;
>  }
>  
> -static unsigned int ttl_to_size(u8 ttl)
> +static unsigned int __ttl_to_size(u8 ttl)

[Severity: High]
This is a pre-existing issue, but does this unsigned int return type cause
huge block mapping sizes to be incorrectly truncated?

Since __ttl_to_size() and ttl_to_size() return a 32-bit unsigned int, they
cannot represent sizes 4GB or larger (like 4TB for 64K granule Level 1, or
64GB for 16K granule Level 1).

> @@ -562,10 +562,22 @@ static unsigned int ttl_to_size(u8 ttl)
>  	return max_size;
>  }
>  
> -static u8 pgshift_level_to_ttl(u16 shift, u8 level)
> +static unsigned int ttl_to_size(u8 ttl)
> +{
> +	return __ttl_to_size(ttl) ?: SZ_1G;
> +}

If a nested L1 guest maps its VNCR page in such a huge block using 52-bit
VAs and later issues a TLBI with a VA far from the VNCR page VA, KVM will
calculate a severely undersized invalidation window. Because the true size
overflows unsigned int (or evaluates to 0), the fallback restricts it to 1GB.

This could leave KVM using a stale IPA, potentially corrupting nested guest
state by writing into unmapped or reassigned guest physical memory.

Should these size calculation functions be returning an unsigned long or u64
instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2