Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits
@ 2026-05-17 17:49 Michael Bommarito
  2026-05-18  6:05 ` Yao Yuan
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Michael Bommarito @ 2026-05-17 17:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Joey Gouly, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
	Will Deacon, linux-arm-kernel, kvmarm, linux-kernel

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.

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.
+	 */
+	if (num_eventid_bits > VITS_TYPER_IDBITS)
+		return -EINVAL;
+
 	if (!vgic_its_check_id(its, baser, id, NULL))
 		return -EINVAL;
 
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits
  2026-05-17 17:49 [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits Michael Bommarito
@ 2026-05-18  6:05 ` Yao Yuan
  2026-05-18  8:23   ` Marc Zyngier
  2026-05-18  8:33 ` Marc Zyngier
  2026-05-19 13:25 ` [PATCH v2] " Michael Bommarito
  2 siblings, 1 reply; 5+ messages in thread
From: Yao Yuan @ 2026-05-18  6:05 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Suzuki K Poulose,
	Zenghui Yu, Catalin Marinas, Will Deacon, linux-arm-kernel,
	kvmarm, linux-kernel

On Sun, May 17, 2026 at 01:49:55PM +0800, Michael Bommarito 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.
>
> 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.
> +	 */
> +	if (num_eventid_bits > VITS_TYPER_IDBITS)
> +		return -EINVAL;

Hi,

IIUC, the same issue is still there when VITS_TYPER_IDBITS
change to >=28, I know it's limited to 16 in GITS_TYPER's
definition. I mean the issue is still there w/o really be
fixed.

Change the scan_its_table() and other related code path to
avoid such date conversion issue is more reasonable
fixing to me, please also wait others' input yet.

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits
  2026-05-18  6:05 ` Yao Yuan
@ 2026-05-18  8:23   ` Marc Zyngier
  0 siblings, 0 replies; 5+ messages in thread
From: Marc Zyngier @ 2026-05-18  8:23 UTC (permalink / raw)
  To: Yao Yuan
  Cc: Michael Bommarito, Oliver Upton, Joey Gouly, Suzuki K Poulose,
	Zenghui Yu, Catalin Marinas, Will Deacon, linux-arm-kernel,
	kvmarm, linux-kernel

On Mon, 18 May 2026 07:05:13 +0100,
Yao Yuan <yaoyuan@linux.alibaba.com> wrote:
> 
> On Sun, May 17, 2026 at 01:49:55PM +0800, Michael Bommarito wrote:

[...]

> > 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.
> > +	 */
> > +	if (num_eventid_bits > VITS_TYPER_IDBITS)
> > +		return -EINVAL;
> 
> Hi,
> 
> IIUC, the same issue is still there when VITS_TYPER_IDBITS
> change to >=28, I know it's limited to 16 in GITS_TYPER's
> definition. I mean the issue is still there w/o really be
> fixed.

Change how? This is a hard-coded limit that reflect a practical use of
the ITS (and is already 32 times larger than what PCIe allows).

Are you suggesting a possibility of making this userspace
configurable?

> 
> Change the scan_its_table() and other related code path to
> avoid such date conversion issue is more reasonable
> fixing to me, please also wait others' input yet.

I don't think this is a reasonable course of action. scan_its_table()
is generic (it doesn't know about any table in particular), and
assumes that the scan parameters are validated upfront.

Thanks,

	M.

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits
  2026-05-17 17:49 [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits Michael Bommarito
  2026-05-18  6:05 ` Yao Yuan
@ 2026-05-18  8:33 ` Marc Zyngier
  2026-05-19 13:25 ` [PATCH v2] " Michael Bommarito
  2 siblings, 0 replies; 5+ messages in thread
From: Marc Zyngier @ 2026-05-18  8:33 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Oliver Upton, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, linux-arm-kernel, kvmarm,
	linux-kernel

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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits
  2026-05-17 17:49 [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits Michael Bommarito
  2026-05-18  6:05 ` Yao Yuan
  2026-05-18  8:33 ` Marc Zyngier
@ 2026-05-19 13:25 ` Michael Bommarito
  2 siblings, 0 replies; 5+ messages in thread
From: Michael Bommarito @ 2026-05-19 13:25 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Yao Yuan, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, linux-arm-kernel, kvmarm,
	linux-kernel

Userspace can restore an ITS Device Table Entry whose Size field encodes
more EventID bits than the virtual ITS supports.  The live MAPD path
rejects that state, but vgic_its_restore_dte() accepts it and stores the
out-of-range value in dev->num_eventid_bits.

Reject restored DTEs with num_eventid_bits > VITS_TYPER_IDBITS before
allocating the device.  This mirrors the MAPD check and prevents the
restored state from reaching vgic_its_restore_itt(), where the unchecked
value can be converted into an oversized scan_its_table() range.

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>
---
Changes in v2:
- Trim the commit message to the root cause and fix.
- Shorten the in-code comment as suggested by Marc.
- Keep the validation logic unchanged.

 arch/arm64/kvm/vgic/vgic-its.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 2ea9f1c7ebcd0..1d7e5d560af4c 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -2307,6 +2307,10 @@ 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;
 
+	/* Mimic the MAPD behaviour and reject invalid EID bits. */
+	if (num_eventid_bits > VITS_TYPER_IDBITS)
+		return -EINVAL;
+
 	if (!vgic_its_check_id(its, baser, id, NULL))
 		return -EINVAL;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-19 13:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 17:49 [PATCH] KVM: arm64: vgic-its: reject restored DTE with out-of-range num_eventid_bits Michael Bommarito
2026-05-18  6:05 ` Yao Yuan
2026-05-18  8:23   ` Marc Zyngier
2026-05-18  8:33 ` Marc Zyngier
2026-05-19 13:25 ` [PATCH v2] " Michael Bommarito

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox