[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pci: assert that irqnum is between 0 and bus->nirqs in pci_c
From: |
Mark Cave-Ayland |
Subject: |
Re: [PATCH] pci: assert that irqnum is between 0 and bus->nirqs in pci_change_irq_level() |
Date: |
Sun, 11 Oct 2020 09:27:38 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 |
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-ayland@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.