[PATCH v2 0/2] pci: assert that irqnum is between 0 and bus->nirqs in pci_change_irq_level()

[PATCH v2 0/2] pci: assert that irqnum is between 0 and bus->nirqs in pci_change_irq_level()

[Philippe Mathieu-Daudé]

Respin of Mark's patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg749459.html

Supersedes: <20201011082022.3016-1-mark.cave-ayland@ilande.co.uk>

Mark Cave-Ayland (1):
  pci: Assert irqnum is between 0 and bus->nirqs in
    pci_bus_change_irq_level

Philippe Mathieu-Daudé (1):
  hw/pci: Extract pci_bus_change_irq_level() from pci_change_irq_level()

 hw/pci/pci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.26.2

[PATCH v2 1/2] hw/pci: Extract pci_bus_change_irq_level() from pci_change_irq_level()

[Philippe Mathieu-Daudé]

Extract pci_bus_change_irq_level() from pci_change_irq_level() to
make it clearer it operates on the bus.

Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci/pci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 100c9381c2f..081ddcadd15 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -248,6 +248,12 @@ static inline void pci_set_irq_state(PCIDevice *d, int irq_num, int level)
         d->irq_state |= level << irq_num;
 }
 
+static void pci_bus_change_irq_level(PCIBus *bus, int irq_num, int change)
+{
+    bus->irq_count[irq_num] += change;
+    bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
+}
+
 static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
 {
     PCIBus *bus;
@@ -258,8 +264,7 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
             break;
         pci_dev = bus->parent_dev;
     }
-    bus->irq_count[irq_num] += change;
-    bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
+    pci_bus_change_irq_level(bus, irq_num, change);
 }
 
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num)
-- 
2.26.2

[PATCH v2 2/2] pci: Assert irqnum is between 0 and bus->nirqs in pci_bus_change_irq_level

[Philippe Mathieu-Daudé]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

These assertions similar to those in the adjacent pci_bus_get_irq_level()
function ensure that irqnum lies within the valid PCI bus IRQ range.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20201011082022.3016-1-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 081ddcadd15..dc4019865be 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -250,6 +250,8 @@ static inline void pci_set_irq_state(PCIDevice *d, int irq_num, int level)
 
 static void pci_bus_change_irq_level(PCIBus *bus, int irq_num, int change)
 {
+    assert(irq_num >= 0);
+    assert(irq_num < bus->nirq);
     bus->irq_count[irq_num] += change;
     bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
 }
-- 
2.26.2

Re: [PATCH v2 0/2] pci: assert that irqnum is between 0 and bus->nirqs in pci_change_irq_level()

[Mark Cave-Ayland]

On 24/10/2020 21:38, Philippe Mathieu-Daudé wrote:

> Respin of Mark's patch:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg749459.html
> 
> Supersedes: <20201011082022.3016-1-mark.cave-ayland@ilande.co.uk>
> 
> Mark Cave-Ayland (1):
>    pci: Assert irqnum is between 0 and bus->nirqs in
>      pci_bus_change_irq_level
> 
> Philippe Mathieu-Daudé (1):
>    hw/pci: Extract pci_bus_change_irq_level() from pci_change_irq_level()
> 
>   hw/pci/pci.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)

This version LGTM - I think it's just waiting on review from the PCI folks to see 
whether they agree with the change.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.