KVM: arm64: Correctly cap TLBI Range to the architural limit

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 open here

arch/arm64/include/asm/kvm_nested.h static inline u64 decode_range_tlbi(u64 val, u64 *range, u16 *asid) · line 299

Quoted 7 lines

arch/arm64/include/asm/kvm_nested.h +0 -0

index cdf3e8422ea12..b68806b3bd6e5 100644
@@ -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*?

arch/arm64/include/asm/kvm_nested.h static inline u64 decode_range_tlbi(u64 val, u64 *range, u16 *asid) · line 310

Quoted 5 lines

arch/arm64/include/asm/kvm_nested.h +4 -0

+
+	/* 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?

[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.

arch/arm64/include/asm/kvm_nested.h static inline u64 decode_range_tlbi(u64 val, u64 *range, u16 *asid) · line 312

Quoted 3 lines

arch/arm64/include/asm/kvm_nested.h +2 -0

+	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?

Quoted 5 lines

arch/arm64/include/asm/kvm_nested.h +1 -0

+
 	return base;
 }
 

Signature · 1 line

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

TLB Invalidation by Range has a fairly powerful way of encoding pretty large ranges in a small number of bits. This range can be based on an arbitrary VA, which means it is pretty easy for a guest to generate an overflow should the hypervisor be naive enough to add the range to the base...

Make sure the range is capped to the limit dictated by the address bit that determines the VA range. For an IPA invalidation, this is further corrected down the line to ignore the upper range.

Fixes 4ffa72ad8f37e ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")

Reported-by Wei-Lin Chang <[email protected]>

Link https://lore.kernel.org/r/yifz3wn5gk5sr6mapi32trgk5m5kp33bquctsjmkifebnsnndt@fix6u4rthx4g open here

Signed-off-by Marc Zyngier <[email protected]>

Cc [email protected]

---
 arch/arm64/include/asm/kvm_nested.h | 6 ++++++
 1 file changed, 6 insertions(+)

arch/arm64/include/asm/kvm_nested.h +0 -0

index c83be6d0e79ac..1ed7083358096 100644
@@ -305,6 +305,12 @@ static inline u64 decode_range_tlbi(u64 val, u64 *range, u16 *asid)
 	num	= FIELD_GET(GENMASK(43, 39), val);

Signature · 2 lines

-- 
2.47.3

TLB Invalidation by Range has a fairly powerful way of encoding pretty large ranges in a small number of bits. This range can be based on an arbitrary VA, which means it is pretty easy for a guest to generate an overflow should the hypervisor be naive enough to add the range to the base...

Make sure the range is capped to the limit dictated by the address bit that determines the VA range. For an IPA invalidation, this is further corrected down the line to ignore the upper range.

[...]