| Message ID | 4eb4c9868798cbfd2819c317a80037f4820b0502.camel@amazon.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i386/xen: fix off-by-one in xen_evtchn_set_gsi() | expand |
On Tue, 4 Jul 2023 at 16:13, Woodhouse, David <dwmw@amazon.co.uk> wrote: > > Coverity points out (CID 1508128) a bounds checking error. We need to check > for gsi >= IOAPIC_NUM_PINS, not just greater-than. > > Also fix up an assert() that has the same problem, that Coverity didn't see. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > hw/i386/kvm/xen_evtchn.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c > index 3d810dbd59..0e9c108614 100644 > --- a/hw/i386/kvm/xen_evtchn.c > +++ b/hw/i386/kvm/xen_evtchn.c > @@ -1587,7 +1587,7 @@ static int allocate_pirq(XenEvtchnState *s, int type, int gsi) > found: > pirq_inuse_word(s, pirq) |= pirq_inuse_bit(pirq); > if (gsi >= 0) { > - assert(gsi <= IOAPIC_NUM_PINS); > + assert(gsi < IOAPIC_NUM_PINS); > s->gsi_pirq[gsi] = pirq; > } > s->pirq[pirq].gsi = gsi; > @@ -1601,7 +1601,7 @@ bool xen_evtchn_set_gsi(int gsi, int level) > > assert(qemu_mutex_iothread_locked()); > > - if (!s || gsi < 0 || gsi > IOAPIC_NUM_PINS) { > + if (!s || gsi < 0 || gsi >= IOAPIC_NUM_PINS) { > return false; > } Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
Hi David, On 4/7/23 17:12, Woodhouse, David via wrote: > Coverity points out (CID 1508128) a bounds checking error. We need to check > for gsi >= IOAPIC_NUM_PINS, not just greater-than. > > Also fix up an assert() that has the same problem, that Coverity didn't see. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > hw/i386/kvm/xen_evtchn.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Better to post new patches as new thread: Patches are easier to find if they start a new top-level thread, rather than being buried in-reply-to another existing thread. (Per https://www.qemu.org/docs/master/devel/submitting-a-patch.html#use-git-format-patch) Regards, Phil.
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c index 3d810dbd59..0e9c108614 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -1587,7 +1587,7 @@ static int allocate_pirq(XenEvtchnState *s, int type, int gsi) found: pirq_inuse_word(s, pirq) |= pirq_inuse_bit(pirq); if (gsi >= 0) { - assert(gsi <= IOAPIC_NUM_PINS); + assert(gsi < IOAPIC_NUM_PINS); s->gsi_pirq[gsi] = pirq; } s->pirq[pirq].gsi = gsi; @@ -1601,7 +1601,7 @@ bool xen_evtchn_set_gsi(int gsi, int level) assert(qemu_mutex_iothread_locked()); - if (!s || gsi < 0 || gsi > IOAPIC_NUM_PINS) { + if (!s || gsi < 0 || gsi >= IOAPIC_NUM_PINS) { return false; }
Coverity points out (CID 1508128) a bounds checking error. We need to check for gsi >= IOAPIC_NUM_PINS, not just greater-than. Also fix up an assert() that has the same problem, that Coverity didn't see. Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> --- hw/i386/kvm/xen_evtchn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)