On Fri, Nov 15, 2019 at 10:36:20AM +0530, P J P wrote: > From: P J P <pjp@xxxxxxxxxxxxxxxxx> > > openpic_src_write sets interrupt level 'src->output' masked with > ILR_INTTGT_MASK(=0xFF). It's then used to index 'dst->outputs_active' > array. With NUM_OUTPUTS=3, it may lead to OOB array access. Limit > active IRQ sources to < NUM_OUTPUTS. > > Reported-by: Reno Robert <renorobert@xxxxxxxxx> > Signed-off-by: P J P <pjp@xxxxxxxxxxxxxxxxx> > --- > arch/powerpc/kvm/mpic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Update v2: limit IRQ sources to NUM_OUTPUTS > -> https://www.spinics.net/lists/kvm-ppc/msg16554.html > > diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c > index fe312c160d97..fe4afd54c6e7 100644 > --- a/arch/powerpc/kvm/mpic.c > +++ b/arch/powerpc/kvm/mpic.c > @@ -628,7 +628,7 @@ static inline void write_IRQreg_ilr(struct openpic *opp, int n_IRQ, > if (opp->flags & OPENPIC_FLAG_ILR) { > struct irq_source *src = &opp->src[n_IRQ]; > > - src->output = val & ILR_INTTGT_MASK; > + src->output = val % NUM_OUTPUTS; Still not right, I'm afraid, since it could leave src->output set to 3, which would lead to an out-of-bounds array access. I think it needs to be if (val < NUM_OUTPUTS) src->output = val; else src->output = ILR_INTTGT_INT; or something like that. Paul.
