Re: [Qemu-arm] [PATCH] intc: arm_gicv3: limit GICR ipriority index

[Philippe Mathieu-Daudé]

On 09/05/2017 08:58 AM, Peter Maydell wrote:
> On 5 September 2017 at 12:21, P J P <address@hidden> wrote:
> > From: Prasad J Pandit <address@hidden>
> >
> > When reading or writing to GICR ipriority array, index 'irq'
> > could go beyond its bounds; Restrict it within array limits.
> >
> > Reported-by: Guoxiang Niu <address@hidden>
> > Signed-off-by: Prasad J Pandit <address@hidden>
> > ---
> >   hw/intc/arm_gicv3_redist.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> > index 77e5cfa327..7683c4cc7f 100644
> > --- a/hw/intc/arm_gicv3_redist.c
> > +++ b/hw/intc/arm_gicv3_redist.c
> > @@ -187,7 +187,7 @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr 
> > offset,
> >       case GICR_ICACTIVER0:
> >           *data = gicr_read_bitmap_reg(cs, attrs, cs->gicr_iactiver0);
> >           return MEMTX_OK;
> > -    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1f:

0x1f are only the cpu (private) irqs, then the range is valid up-to:
(extract64(cs->gicr_typer, 0, 5) + 1) * 32 - 1 supported irqs

> > +    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1c:
> >       {
> >           int i, irq = offset - GICR_IPRIORITYR;
> >           uint32_t value = 0;
> > @@ -310,7 +310,7 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr 
> > offset,
> >       case GICR_ICACTIVER0:
> >           gicr_write_clear_bitmap_reg(cs, attrs, &cs->gicr_iactiver0, value);
> >           return MEMTX_OK;
> > -    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1f:
> > +    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1c:
> >       {
> >           int i, irq = offset - GICR_IPRIORITYR;
>
> Why do you think the buffer can be overrun? These functions
> are the word (4 byte) access functions, and they cannot
> be called with a non-4-aligned offset (see the asserts in
> gicv3_redist_read() and gicv3_redist_write()).
>
> thanks
> -- PMM