Re: [PATCH] pci: check bus pointer before dereference

[P J P <ppandit@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

+-- On Tue, 15 Sep 2020, Philippe Mathieu-Daudé wrote --+
| > I think in normal this 'bus' will be not NULL. I have look at the link in 
| > the commit msg. I find it is another DMA to MMIO issue which we have 
| > discussed a lot but didn't come up with an satisfying solution.

  If 'bus' is unlikely to be NULL, should this be a regular non-CVE bug?
 
| As usual, question is how we got here.
| As Li said, it is another DMA to MMIO bug class.
| 
| lsi_execute_script
|  -> address_space_write
|     -> acpi_pcihp_eject_slot
|        -> bus_remove_child
| 
| So at this point the PCI device is still MMIO-mapped but eject from the 
| bus... ??? Then IRQ is triggered, which the device wants to propagate via 
| its PCI bus but it doesn't have any more and b00m.
| 
| If a device is hotpluggable, who is responsible to unmap its regions?

  Not sure, I guess I'll leave it for the upstream maintainers to device a 
better solution.

| Nack, this should be an abort().

===
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index de0fae10ab..0ccb991410 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -253,6 +253,7 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int 
irq_num, int change)
     PCIBus *bus;
     for (;;) {
         bus = pci_get_bus(pci_dev);
+        assert(bus);
         irq_num = bus->map_irq(pci_dev, irq_num);
         if (bus->set_irq)
             break;
===

This should be okay for now?


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D