Re: [PATCH v3 2/4] xen/arm: validate IRQs before descriptor lookup

[Mykola Kvach <mykola_kvach@xxxxxxxx>]

On Tue, Aug 25, 2026 at 03:30:43AM +0300, Volodymyr Babchuk wrote:
> Hi,
> 
> Mykola Kvach <mykola_kvach@xxxxxxxx> writes:
> 
> > GICv3 eSPI support makes nr_irqs span the architectural INTID namespace
> > through ESPI_MAX_INTID, but descriptor storage is sparse. local_irq_desc[]
> > and irq_desc[] cover INTIDs below NR_IRQS, while espi_desc[] covers eSPIs.
> > INTIDs 1024 through 4095 have no backing descriptors.
> >
> > Validation based only on nr_irqs accepts an INTID in this gap.
> > __irq_to_desc() then indexes beyond irq_desc[], and callers may lock or
> > update unrelated Xen memory.
> >
> > Reject INTIDs that the GIC reports as unimplemented in setup_irq() before
> > looking up a descriptor. irq_set_spi_type() can run before the implemented
> > GIC line counts are available, so validate descriptor-backed ranges there
> > before looking up a descriptor.
> >
> > Assert the regular descriptor bound in __irq_to_desc() so direct callers
> > cannot silently index the sparse gap in debug builds.
> >
> > Fixes: 98f7060b9ed5 ("xen/arm/irq: add handling for IRQs in the eSPI range")
> > Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> > ---
> > Changes in v3:
> > - Add the requested bound assertion and retain the SPI-only comment.
> >
> > Changes in v2:
> > - Validate descriptor-backed ranges in irq_set_spi_type().
> > - Validate implemented GIC lines in setup_irq().
> > - Preserve is_espi() validation with CONFIG_GICV3_ESPI disabled.
> > ---
> >  xen/arch/arm/irq.c | 26 ++++++++++++++++++++++----
> >  1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> > index 73e58a5108..bf14180f97 100644
> > --- a/xen/arch/arm/irq.c
> > +++ b/xen/arch/arm/irq.c
> > @@ -23,6 +23,12 @@ const unsigned int nr_irqs = 
> > IS_ENABLED(CONFIG_GICV3_ESPI) ?
> >                                          (ESPI_MAX_INTID + 1) :
> >                                          NR_IRQS;
> >  
> > +static bool irq_has_desc(unsigned int irq)
> 
> You are using this function only in one place, where you are actually
> testing for SPI. So, maybe introduce irq_is_spi() helper instead? And
> use it below?

I will keep irq_has_desc() and use it in __irq_to_desc() too,
as Michal suggested. This will keep the range checks in sync.

> 
> > +{
> > +    return irq < NR_IRQS ||
> > +           (IS_ENABLED(CONFIG_GICV3_ESPI) && is_espi(irq));
> > +}
> > +
> >  static unsigned int local_irqs_type[NR_LOCAL_IRQS];
> >  static DEFINE_SPINLOCK(local_irqs_type_lock);
> >  
> > @@ -76,7 +82,6 @@ static int __init init_espi_data(void)
> >      return 0;
> >  }
> >  #else
> > -
> 
> Please, no unnecessary changes

Ack.

Best regards,
Mykola