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

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

New issues:
- [Critical] Hardware Use-After-Free (UAF) due to missing IRS invalidation command during LPI IST teardown.
- [Critical] Integer truncation in `vgic_v5_alloc_linear_ist` causes ZERO_SIZE_PTR allocation and silent memory corruption.
- [High] Unconditional NULL pointer dereference of `vcpu0` during IST assignment.
- [High] Silent physical address truncation in L2 IST allocation due to unvalidated masking.
- [Medium] Permanent memory leak of VM resources due to early aborts in `vgic_v5_vmte_release`.

Pre-existing issues:
- [High] Missing DSB barrier after cache maintenance allows the hardware IRS to read stale VMTE data.
--

--- Patch [10]: [PATCH v5 10/49] KVM: arm64: gic-v5: Introduce guest IST alloc and management ---
commit 4b81b458fb30bd2b57a30edcbe23a8f8de22edbb
Author: Sascha Bischoff <[email protected]>
Date:   Fri Aug 7 11:17:41 2026 +0000

    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 introduces the machinery
needed to allocate a linear SPI IST and a shadow LPI IST, and pass them
to the host's IRS via the VMTE.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5-tables.c b/arch/arm64/kvm/vgic/vgic-v5-tables.c
> index 7252d48431a5a..f088006f39674 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5-tables.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5-tables.c
[ ... ]
> @@ -581,6 +608,22 @@ int vgic_v5_vmte_release(struct kvm *kvm)
>  	if (!vmi)
>  		goto no_vmi;
>  
> +	/* If we have an LPI IST, free it */
> +	if (vmi->h_lpi_ist) {
> +		ret = vgic_v5_lpi_ist_free(kvm);
> +		if (ret)
> +			return ret;

[Severity: Medium]
What happens to the remaining allocated resources if this returns early?

If vgic_v5_lpi_ist_free() returns an error, the function aborts immediately.
This bypasses the release of vped_base, vpet_base, and vmd_base, and skips
removing the entry from vm_info, permanently leaking these VM resources.

> +	}
> +	vmi->h_lpi_ist = NULL;
> +
> +	/* If we have an SPI IST, free it */
> +	if (vmi->h_spi_ist) {
> +		ret = vgic_v5_spi_ist_free(kvm);
> +		if (ret)
> +			return ret;

[Severity: Medium]
Similarly, could an error here leak the same resources?

> +	}
> +	vmi->h_spi_ist = NULL;
[ ... ]
> @@ -679,3 +722,511 @@ int vgic_v5_vmte_free_vpe(struct kvm_vcpu *vcpu)
>  
>  	return 0;
>  }
> +
> +/*
> + * Assign an already allocated IST to the VM by populating the fields in the
> + * corresponding VMTE. We re-use this code for both an SPI IST and LPI IST, even
> + * if the paths to reach it might be vastly different.
> + */
> +static int vgic_v5_vmte_assign_ist(struct kvm *kvm, phys_addr_t ist_base,
> +				   bool two_level, unsigned int id_bits,
> +				   unsigned int l2sz, unsigned int istsz,
> +				   bool spi_ist)
> +{
> +	struct kvm_vcpu *vcpu0 = kvm_get_vcpu(kvm, 0);
[ ... ]
> +	scoped_guard(raw_spinlock_irqsave, &vgic_v5_irs_lock) {
> +		/* Bail if already allocated */
> +		vgic_v5_clean_inval(vmte, sizeof(*vmte));
> +		if (le64_to_cpu(READ_ONCE(vmte->val[section])) &
> +		    GICV5_VMTEL2E_IST_VALID)
> +			return -EINVAL;
> +
> +		WRITE_ONCE(vmte->val[section], cpu_to_le64(tmp));
> +		vgic_v5_clean_inval(vmte, sizeof(*vmte));
> +	}
> +
> +	/* Finally, mark the entry as valid */
> +	cmd = spi_ist ? SPI_VIST_MAKE_VALID : LPI_VIST_MAKE_VALID;
> +	ret = irq_set_vcpu_affinity(vgic_v5_vpe_db(vcpu0), &cmd);

[Severity: High]
What happens if vcpu0 is NULL? 

Userspace can invoke KVM VGIC initialization ioctls before any VCPUs are
created (where kvm->online_vcpus is 0). In this state, kvm_get_vcpu() returns
NULL. When vgic_v5_vpe_db() attempts to dereference vcpu0 to get the doorbell
address, it would cause a host kernel panic.

[Severity: High]
This is a pre-existing issue, but does vgic_v5_clean_inval() require a barrier
to guarantee completion?

The vgic_v5_clean_inval() function calls dcache_clean_inval_poc() to flush
caches for a non-coherent IRS. However, it lacks a trailing dsb(sy) or dsb(ish)
barrier. Because of this, the memory-mapped write to the doorbell in
irq_set_vcpu_affinity() might execute before the cache flush completes,
allowing the IRS hardware to read stale VMTE data.

> +
> +	return ret;
> +}
[ ... ]
> +/*
> + * Allocate a Linear IST - always used for SPIs and potentially LPIs.
> + *
> + * The calculation for n has been taken from section 11.2.2 of the GICv5 EAC0
> + * spec.
> + *
> + * NOTE: istsz is the FIELD used by GICv5, not the actual size (or log2() of the
> + * size).
> + */
> +static int vgic_v5_alloc_linear_ist(struct kvm *kvm, bool spi_ist,
> +				    unsigned int id_bits, unsigned int istsz)
> +{
> +	const size_t n = max(5, 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);

[Severity: Critical]
Can l1sz overflow and truncate to zero?

If a guest requests a large LPI ID space (e.g., id_bits = 30), n can reach 32.
BIT(33) evaluates to 8,589,934,592, which truncates to exactly 0 when assigned
to the 32-bit u32 l1sz variable.

> +	ist = kzalloc(l1sz, GFP_KERNEL_ACCOUNT);

When kzalloc() is called with a size of 0, it returns ZERO_SIZE_PTR (0x10).
KVM will then translate this via virt_to_phys() and program the bogus physical
address into the hardware's VMTE. The IRS will perform DMA to this address,
leading to host memory corruption.

> +	if (!ist)
> +		return -ENOMEM;
[ ... ]
> +/*
> + * Allocate ALL of the second level ISTs for a two-level IST - LPI, only.
> + *
> + * The calculation for n has been taken from section 11.2.2 of the GICv5 EAC0
> + * spec. The l2_size calculation is from section 11.2.3 of the same document.
> + *
> + * NOTE: istsz and l2sz are the FIELDS used by GICv5, not the actual sizes (or
> + * log2() of the sizes).
> + */
> +static int vgic_v5_alloc_l2_ists(struct kvm *kvm, unsigned int id_bits,
> +				 unsigned int istsz, unsigned int l2sz)
> +{
[ ... ]
> +		/*
> +		 * We are not doing on-demand allocation of the L2 ISTs, and are
> +		 * instead provisioning the whole IST up front. This means that
> +		 * we are able to mark the L2 ISTs as valid in the L1 ISTEs as
> +		 * the overall IST is not yet valid.
> +		 */
> +		val = (virt_to_phys(l2ist) & GICV5_ISTL1E_L2_ADDR_MASK) |
> +		      GICV5_ISTL1E_VALID;

[Severity: High]
Is it possible for virt_to_phys(l2ist) to have bits above the
GICV5_ISTL1E_L2_ADDR_MASK?

Unlike vgic_v5_vmte_assign_ist() which checks if the address fits the hardware
mask, this function unconditionally applies a bitwise AND. On systems with
LVA/LPA (up to 52-bit PAs), if the allocated page resides above the bits
covered by the mask, the upper bits are silently truncated. The IRS would then
write incoming LPI states to an aliased, incorrect physical memory address.

> +		l1ist[index] = cpu_to_le64(val);
> +
> +		vmi->h_lpi_l2_ists[index] = l2ist;
> +
> +		vgic_v5_clean_inval(l2ist, l2_size);
> +	}
[ ... ]
> +/*
> + * Free a Linear IST. Can only happen once the VM is dead.
> + */
> +static int vgic_v5_linear_ist_free(struct kvm *kvm, bool spi)
> +{
> +	u32 vm_id = vgic_v5_vm_id(kvm);
> +	struct vmtl2_entry *vmte;
> +	struct vgic_v5_vm_info *vmi;
> +	int section;
[ ... ]
> +	if (spi) {
> +		section = GICV5_VMTEL2_SPI_SECTION;
> +		vgic_v5_free_allocated_spi_ist(kvm);
> +	} else {
> +		section = GICV5_VMTEL2_LPI_SECTION;
> +		vgic_v5_free_allocated_lpi_ist(vmi, 0, 0, 0);
> +	}
> +
> +	/* The VM should be dead here, so we can just zero the VMT section */
> +	scoped_guard(raw_spinlock_irqsave, &vgic_v5_irs_lock) {
> +		WRITE_ONCE(vmte->val[section], cpu_to_le64(0));
> +		vgic_v5_clean_inval(vmte, sizeof(*vmte));
> +	}

[Severity: Critical]
Does the hardware IRS need to be notified that the LPI IST is now invalid?

The memory is explicitly freed and the software VMTE is zeroed, but no
LPI_VIST_MAKE_INVALID command is sent to the IRS hardware. Because the hardware
IRS is never notified, it will retain the physical address in its cache and
will continue to DMA incoming LPI state updates to the newly freed host memory
pages, leading to a Use-After-Free and host memory corruption.

> +
> +	return 0;
> +}
> +
> +/*
> + * Free a Two-Level IST. Can only happen once the VM is dead.
> + */
> +static int vgic_v5_two_level_ist_free(struct kvm *kvm, bool spi)
> +{
[ ... ]
> +	vgic_v5_free_allocated_lpi_ist(vmi, id_bits, istsz, l2sz);
> +
> +	/* The VM must be dead, so we can just zero the VMT section */
> +	scoped_guard(raw_spinlock_irqsave, &vgic_v5_irs_lock) {
> +		WRITE_ONCE(vmte->val[section], cpu_to_le64(0));
> +		vgic_v5_clean_inval(vmte, sizeof(*vmte));
> +	}

[Severity: Critical]
Similarly, does the two-level IST teardown also require an
LPI_VIST_MAKE_INVALID hardware command?

The LPI IST memory is freed and the VMTE is zeroed without sending the
invalidation command to the IRS, opening the same race window for hardware
Use-After-Free.

> +
> +	return 0;
> +}

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