Re: [PATCH] KVM: arm64: Correctly cap TLBI Range to the architural limit
Wei-Lin Chang <[email protected]> Mon, 10 Aug 2026 19:59:52 +0100
| Newsgroups | dev.linux.lists.kvmarm,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <kepali2qvsom3x55qfdsvsf6vs6vcg3ubkz6zisjfoncrwsvaq@5nftl3thh7i2> |
On Mon, Aug 10, 2026 at 05:43:08PM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: > > New issues: > - [High] Hardcoded BIT(48) cap truncates valid 52-bit IPAs and underflows for higher addresses > - [High] Upper-half range cap `~base + 1` is completely ineffective due to missing sign-extension > > Pre-existing issues: > - [High] Range TLB invalidations for upper-half VAs silently fail to invalidate the VNCR cache > - [High] Malicious guests can still trigger a host WARN_ON because the BIT(48) cap ignores smaller VM ia_bits > -- > > --- Patch [1]: [PATCH] KVM: arm64: Correctly cap TLBI Range to the architural limit --- > commit 09ab12a5dd613f7e322a9123897db5c50369508d > Author: Marc Zyngier <[email protected]> > > KVM: arm64: Correctly cap TLBI Range to the architural limit > > This commit caps the range of a TLB invalidation based on the base address > to prevent guests from triggering an overflow when the hypervisor adds the > range to the base address. > > Link: https://lore.kernel.org/r/yifz3wn5gk5sr6mapi32trgk5m5kp33bquctsjmkifebnsnndt@fix6u4rthx4g > > > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h > > index cdf3e8422ea12..b68806b3bd6e5 100644 > > --- a/arch/arm64/include/asm/kvm_nested.h > > +++ b/arch/arm64/include/asm/kvm_nested.h > > @@ -298,6 +298,12 @@ static inline u64 decode_range_tlbi(u64 val, u64 *range, u16 *asid) > > num = FIELD_GET(GENMASK(43, 39), val); > > *range = __TLBI_RANGE_PAGES(num, scale) << shift; > > [Severity: High] > This isn't a bug introduced by this patch, but earlier in > decode_range_tlbi(), base is extracted without sign-extension: > > base = (val & GENMASK(36, 0)) << shift; > > For upper-half kernel virtual addresses, base will be zero-extended. When > invalidate_vncr_va() in arch/arm64/kvm/nested.c checks if this invalidation > intersects with a cached VNCR TLB (vt->gva), the cached TLB is correctly > sign-extended. The bounds check incorrectly fails due to comparing a > sign-extended gva against a zero-extended scope->va: > > if (va_end <= scope->va || > va_start >= (scope->va + scope->size)) > continue; > > Does this leave stale entries in the VNCR cache and break memory isolation > for nested guests when they invalidate an upper-half VA range via > TLBI RVAE1*? base is correctly sign-extended now [1]. [1]: https://git.kernel.org/kvmarm/kvmarm/c/239347008564 > > > + > > + /* Cap the range to the correct half of the address space */ > > + if (!(base & BIT(48))) > > + *range = min(*range, (BIT(48) - base)); > > [Severity: High] > Does this hardcoded BIT(48) cap incorrectly truncate valid 52-bit IPAs? > > For guests with a 52-bit IPA space (FEAT_LPA), a valid TLB invalidation over > the entire address space (base=0, range=BIT(52)) appears to be truncated to > BIT(48), potentially leaving stale mappings in the upper 48-52 bits. > > Additionally, if the base address exceeds BIT(48) but has bit 48 cleared > (for example, base=BIT(49)), won't (BIT(48) - base) underflow to a massive > unsigned value, bypassing the cap and failing to prevent the overflow? There is no 52-bit support for NV guests. > > [Severity: High] > This is a pre-existing issue, but since this cap uses BIT(48), can a nested > guest still trigger the host WARN_ON in __unmap_stage2_range() if the VM's > configured PARange (ia_bits) is smaller than 48? > > If a guest supplies a base and range that stays below BIT(48) but exceeds > the VM's actual limit (for example, BIT(40)), it seems _kvm_pgtable_walk() in > arch/arm64/kvm/hyp/pgtable.c will still return -ERANGE: > > if (data->addr > limit || data->end > limit) > return -ERANGE; > > This bypasses the new cap and can trigger the warning, which could act as a > Denial of Service if panic_on_warn is set. This looks correct to me, but irrelevant to this patch. > > > + else > > + *range = min(*range, ~base + 1); > > [Severity: High] > Is this upper-half range cap mathematically effective without base being > sign-extended? > > Since decode_range_tlbi() leaves base as a positive unsigned 64-bit integer, > ~base + 1 evaluates to an extremely large positive value near 2^64. Will > min(*range, ~base + 1) always evaluate to *range, leaving the range uncapped > and still vulnerable to the overflow for upper-half virtual addresses? base is sign-extended now, see above. Thanks, Wei-Lin Chang > > > + > > return base; > > } > > > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1