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;
     }
 
-- 
2.34.1





Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.

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.

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