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

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-08-07 at 13:07 +0000, sashiko-bot@kernel.org wrote:
> 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 <Sascha.Bischoff@arm.com>
> 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.

That's true, but what else can we really do? If we've failed to free
the IST here, then we're in a very broken state. The hardware still has
a handle to it, and therefore could use the memory. Unless we can be
sure that this can't happen, we can't continue the teardown. This
memory, and other associated VM structures need to remain in place
until we're sure that the hardware won't go and access them.

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

Yes, and that needs to be the case. See above.

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

We cannot make it this far without a vcpu. We must have at least one,
and therefore we use the first one.

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

We block that in vgic_v5_init() so, no, we cannot make it this far.

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

It does, and it has it, albeit implicitly. dcache_clean_inval_poc()
uses dcache_by_line_op, which itself ends with dsb sy.

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

No.

> 
> If a guest requests a large LPI ID space (e.g., id_bits = 30), n can
> reach 32.

We limit the guest to 16-bits of ID space.

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

Things are limited such that this cannot happen.

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

Did you check the mask? It goes up to bit 55 - it supports 56-bit
addresses. Not an issue.

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

Not if the VM is invalid. Once the VM is invalid, the hardware is not
allowed to access anything that is referenced by the VMTE.

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

As above, no.

> 
> 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;
> > +}
> 

Thanks,
Sascha