On 11/10/2020 09:20, Mark Cave-Ayland wrote: > 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-ayl...@ilande.co.uk> > --- > > This would have immediately picked up on the sabre PCI bus IRQ overflow fixed > by > the patch I just posted. > > --- > hw/pci/pci.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 3c8f10b461..b1484b3747 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -258,6 +258,8 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int > irq_num, int change) > break; > pci_dev = bus->parent_dev; > } > + 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); > }
Actually something else is odd here: I've just done a quick check on the callers to pci_change_irq_level() and it appears that both pci_update_irq_disabled() and pci_irq_handler() assume that irqnum is a PCI device IRQ i.e between 0 and 3, whereas pci_change_irq_level() assumes it is working with a PCI bus IRQ between 0 and bus->nirqs. It feels like pci_change_irq_level() should be renamed to pci_bus_change_irq_level() similar to pci_bus_get_irq_level() but in that case are pci_update_irq_disabled() and pci_irq_handler() both incorrect? ATB, Mark.
