| Submitter | Scott Wood |
|---|---|
| Date | Dec. 22, 2012, 2:15 a.m. |
| Message ID | <1356142552-13453-14-git-send-email-scottwood@freescale.com> |
| Download | mbox | patch |
| Permalink | /patch/207917/ |
| State | New |
| Headers | show |
Comments
On 22.12.2012, at 03:15, Scott Wood wrote: > The two checks with abort() guard against potential QEMU-internal > problems, but the EOI check stops the guest from causing updates to queue > position -1 and other havoc if it writes EOI with no interrupt in > service. > > Signed-off-by: Scott Wood <scottwood@freescale.com> Did you ever actually experience this? MAX_IRQ should match the memory region size, so we shouldn't be able to receive any interrupt above it. I might be inclined to accept an assert() there for internal sanity checking though. The last hunk looks fine. Alex > --- > hw/openpic.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/hw/openpic.c b/hw/openpic.c > index 5accff5..a3fcefd 100644 > --- a/hw/openpic.c > +++ b/hw/openpic.c > @@ -289,6 +289,10 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q) > irq = base + offset; > map &= ~(1UL << offset); > > + if (irq >= MAX_IRQ) { > + abort(); > + } > + > DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n", > irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority); > > @@ -428,6 +432,11 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level) > OpenPICState *opp = opaque; > IRQ_src_t *src; > > + if (n_IRQ >= MAX_IRQ) { > + fprintf(stderr, "%s: IRQ %d out of range\n", __func__, n_IRQ); > + abort(); > + } > + > src = &opp->src[n_IRQ]; > DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n", > n_IRQ, level, src->ivpr); > @@ -923,6 +932,12 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr, > case 0xB0: /* EOI */ > DPRINTF("EOI\n"); > s_IRQ = IRQ_get_next(opp, &dst->servicing); > + > + if (s_IRQ < 0) { > + DPRINTF("%s: EOI with no interrupt in service\n", __func__); > + break; > + } > + > IRQ_resetbit(&dst->servicing, s_IRQ); > /* Set up next servicing IRQ */ > s_IRQ = IRQ_get_next(opp, &dst->servicing); > -- > 1.7.9.5 > >
Patch
diff --git a/hw/openpic.c b/hw/openpic.c index 5accff5..a3fcefd 100644 --- a/hw/openpic.c +++ b/hw/openpic.c @@ -289,6 +289,10 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q) irq = base + offset; map &= ~(1UL << offset); + if (irq >= MAX_IRQ) { + abort(); + } + DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n", irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority); @@ -428,6 +432,11 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int level) OpenPICState *opp = opaque; IRQ_src_t *src; + if (n_IRQ >= MAX_IRQ) { + fprintf(stderr, "%s: IRQ %d out of range\n", __func__, n_IRQ); + abort(); + } + src = &opp->src[n_IRQ]; DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n", n_IRQ, level, src->ivpr); @@ -923,6 +932,12 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr, case 0xB0: /* EOI */ DPRINTF("EOI\n"); s_IRQ = IRQ_get_next(opp, &dst->servicing); + + if (s_IRQ < 0) { + DPRINTF("%s: EOI with no interrupt in service\n", __func__); + break; + } + IRQ_resetbit(&dst->servicing, s_IRQ); /* Set up next servicing IRQ */ s_IRQ = IRQ_get_next(opp, &dst->servicing);
The two checks with abort() guard against potential QEMU-internal problems, but the EOI check stops the guest from causing updates to queue position -1 and other havoc if it writes EOI with no interrupt in service. Signed-off-by: Scott Wood <scottwood@freescale.com> --- hw/openpic.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)