Re: [PATCH v3 10/40] KVM: arm64: gic-v5: Introduce guest IST alloc and management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Integer truncation of IST allocation size causes zero-sized allocations and hardware memory corruption.
- [Low] Endianness type mismatch when parsing VMTE state using `FIELD_GET()`.
--

commit 1bc57da799f1125b47ee47d0156802cc04200696
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Introduce guest IST alloc and management

GICv5 guests use Interrupt State Tables (ISTs) to track and manage the
interrupt state for SPIs and LPIs. This commit provisions memory for
managing a guest's SPI state and shadow LPI ISTs passed to the host IRS.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5-tables.c b/arch/arm64/kvm/vgic/vgic-v5-tables.c
> index db8cb6a384994..79b305d2d5097 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5-tables.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5-tables.c

[ ... ]

> +static int vgic_v5_alloc_linear_ist(struct kvm *kvm, bool spi_ist,
> +				    unsigned int id_bits, unsigned int istsz)
> +{
> +	const size_t n = id_bits + 1 + istsz;
> +	u32 vm_id = vgic_v5_vm_id(kvm);
> +	struct vgic_v5_vm_info *vmi;
> +	__le64 *ist;
> +	u32 l1sz;
> +
> +	vmi = xa_load(&vm_info, vm_id);
> +	if (!vmi)
> +		return -EINVAL;
> +
> +	/*
> +	 * Allocate the IST. We only have one level, so we just use the L2 ISTE.
> +	 */
> +	l1sz = BIT(n + 1);
> +	ist = kzalloc(l1sz, GFP_KERNEL_ACCOUNT);

[Severity: High]
Can this cause integer truncation and zero-sized allocations if a guest
requests a large LPI ID space?

In vgic_v5_alloc_linear_ist(), the allocation size is calculated as BIT(n + 1)
and stored in a 32-bit u32 l1sz. The value n is derived from id_bits + 1 +
istsz. If a guest requests a large LPI ID space (e.g., id_bits = 31) on a
system that falls back to linear ISTs, n evaluates to 34.

BIT(35) evaluates to a 64-bit value that is silently truncated to 0 when
assigned to l1sz. The subsequent kzalloc(0) succeeds but returns
ZERO_SIZE_PTR. KVM then blindly passes the invalid pseudo-physical address to
the hardware VMTE, which can lead to severe host memory corruption or IOMMU
faults when the hardware routes virtual interrupts.

[ ... ]

> +static int vgic_v5_two_level_ist_free(struct kvm *kvm, bool spi)
> +{
> +	unsigned int id_bits, istsz, l2sz;
> +	u32 vm_id = vgic_v5_vm_id(kvm);
> +	struct vgic_v5_vm_info *vmi;
> +	struct vmtl2_entry *vmte;
> +	__le64 tmp;
> +	int section;
> +
> +	/* We don't create two-level SPI ISTs, so freeing is a bad idea! */
> +	if (spi)
> +		return -EINVAL;
> +
> +	vmi = xa_load(&vm_info, vm_id);
> +	if (!vmi)
> +		return -EINVAL;
> +
> +	section = GICV5_VMTEL2_LPI_SECTION;
> +
> +	if (!vmi->h_lpi_ist_structure)
> +		return -EINVAL;
> +
> +	vmte = vgic_v5_get_l2_vmte(vm_id);
> +	if (IS_ERR(vmte))
> +		return PTR_ERR(vmte);
> +
> +	tmp = le64_to_cpu(READ_ONCE(vmte->val[section]));
> +
> +	id_bits = FIELD_GET(GICV5_VMTEL2E_IST_ID_BITS, tmp);

[Severity: Low]
This isn't a bug, but does this cause sparse warnings due to an endianness
type mismatch?

In vgic_v5_two_level_ist_free(), le64_to_cpu() is used to convert the VMTE
entry to native endianness. However, the result is assigned to tmp, which is
declared as __le64. The native value is then passed into FIELD_GET(), which
expects a native unsigned integer type. While this compiles and functions
identically to u64 because __le64 is internally an unsigned 64-bit integer,
it breaks endianness semantic contracts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703154811.3355680-1-sascha.bischoff@arm.com?part=10