Re: [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits

[Marc Zyngier <maz@kernel.org>]

On Sun, 17 May 2026 18:49:55 +0100,
Michael Bommarito <michael.bommarito@gmail.com> wrote:
> 
> Userspace can trigger a host-side denial of service via
> KVM_DEV_ARM_ITS_RESTORE_TABLES. A Device Table Entry whose Size
> field encodes num_eventid_bits > VITS_TYPER_IDBITS reaches
> scan_its_table() with a sign-extended ~18 EiB length, where the
> loop holds the per-ITS mutex and never calls cond_resched(),
> pinning a host CPU for a time linear in registered guest memslot
> size. The accessor is any process that can open /dev/kvm and
> create a VM. The same out-of-range value also disables a
> subsequent live bounds check on EventID, as described below.
> 
> The MAPD command handler already rejects this case in the live
> path:
> 
>   if (valid && num_eventid_bits > VITS_TYPER_IDBITS)
>           return E_ITS_MAPD_ITTSIZE_OOR;
> 
> vgic_its_restore_dte() reconstructs num_eventid_bits from the DTE
> Size field but does not apply the same cap, so userspace can
> install device state that the live MAPD path is documented to
> reject. The restored value is stored in dev->num_eventid_bits and
> is then used by vgic_its_restore_itt():
> 
>   size_t max_size = BIT_ULL(dev->num_eventid_bits) * ite_esz;
>   ret = scan_its_table(its, base, max_size, ite_esz, ...);
> 
> scan_its_table() takes the size as int and assigns it to
> unsigned long in the callee:
> 
>   static int scan_its_table(struct vgic_its *its, gpa_t base,
>                             int size, u32 esz, ...)
>   {
>           unsigned long len = size;
> 
> For num_eventid_bits = 28 the size_t value 0x80000000 truncates to
> INT_MIN as int and sign-extends to ~18 EiB as unsigned long. The
> scan loop then walks the registered guest memslot one ite_esz at
> a time with the per-ITS mutex held and no cond_resched(). In a
> QEMU TCG arm64 guest at EL2 on v7.1-rc1, with an empty ITT, the
> ioctl returned -EFAULT after about 14 seconds with a 256 MiB
> memslot and about 56 seconds with a 1 GiB memslot (linear in
> memslot size). The per-iteration cost on native arm64 KVM
> hardware will differ; the loop shape, and so the linear scaling,
> will not.
> 
> The same out-of-range num_eventid_bits also disables the live
> vgic_its_check_event_id() bounds check, because event_id is u32
> and BIT_ULL(32) is unreachable in that comparison, leaving
> subsequent MAPI/MAPTI handling without an effective EventID cap.
> 
> Mirror the MAPD cap in vgic_its_restore_dte() before allocating
> the device, so out-of-range restored DTEs are rejected with
> -EINVAL up front rather than triggering the int-truncated scan or
> installing a device whose num_eventid_bits silently disables the
> live bounds check. Sizes within [1, VITS_TYPER_IDBITS] are
> unaffected.

This commit message is way too verbose, and really looks like a copy
paste from an LLM analysis. This needs trimming (the initial paragraph
has most of it...).

> 
> Fixes: 57a9a117154c ("KVM: arm64: vgic-its: Device table save/restore")
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  arch/arm64/kvm/vgic/vgic-its.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index 2ea9f1c7ebcd0..a5dcf9a6a2854 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -2307,6 +2307,15 @@ static int vgic_its_restore_dte(struct vgic_its *its, u32 id,
>  	/* dte entry is valid */
>  	offset = (entry & KVM_ITS_DTE_NEXT_MASK) >> KVM_ITS_DTE_NEXT_SHIFT;
>  
> +	/*
> +	 * The MAPD command rejects this case; mirror the cap here so a
> +	 * restored DTE cannot install an out-of-range num_eventid_bits
> +	 * that vgic_its_restore_itt() would then convert into a
> +	 * sign-extended scan_its_table() length.
> +	 */

Same here. Something like:

	/* Mimic the MAPD behaviour and reject invalid EID bits */

would be enough.

> +	if (num_eventid_bits > VITS_TYPER_IDBITS)
> +		return -EINVAL;
> +
>  	if (!vgic_its_check_id(its, baser, id, NULL))
>  		return -EINVAL;
>  

Otherwise looks good.

	M.

-- 
Without deviation from the norm, progress is not possible.