Subject: [PATCH 1/1] Fix NULL pointer access in PowerPC MSI teardown code

From: Radu Rendec <[email-masked]>
Date: Tue, 27 Nov 2018 22:20:48 -0500

The arch_teardown_msi_irqs() function assumes that controller ops
pointers were already checked in arch_setup_msi_irqs(), but this
assumption is wrong: arch_teardown_msi_irqs() can be called even when
arch_setup_msi_irqs() returns an error (-ENOSYS).

This can happen in the following scenario:

  * msi_capability_init() calls pci_msi_setup_msi_irqs()
  * pci_msi_setup_msi_irqs() returns -ENOSYS
  * msi_capability_init() notices the error and calls free_msi_irqs()
  * free_msi_irqs() calls pci_msi_teardown_msi_irqs()

This is easier to see when CONFIG_PCI_MSI_IRQ_DOMAIN is not set and
pci_msi_setup_msi_irqs() and pci_msi_teardown_msi_irqs() are just
aliases to arch_setup_msi_irqs() and arch_teardown_msi_irqs().

The call to free_msi_irqs() upon pci_msi_setup_msi_irqs() failure seems
legit, as it does additional cleanup; e.g. list_del(&entry->list) and
kfree(entry) inside free_msi_irqs() do happen (MSI descriptors are
allocated before pci_msi_setup_msi_irqs() is called and need to be
cleaned up if that fails).

Signed-off-by: Radu Rendec <[email-masked]>
---
 arch/powerpc/kernel/msi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/msi.c b/arch/powerpc/kernel/msi.c
index dab616a33b8d..83c2043cc685 100644
--- a/arch/powerpc/kernel/msi.c
+++ b/arch/powerpc/kernel/msi.c
@@ -34,5 +34,12 @@ void arch_teardown_msi_irqs(struct pci_dev *dev)
 {
 	struct pci_controller *phb = pci_bus_to_host(dev->bus);
 
-	phb->controller_ops.teardown_msi_irqs(dev);
+	/*
+	 * We can be called even when arch_setup_msi_irqs() returns -ENOSYS,
+	 * so check the pointer again. Example: msi_capability_init() calls
+	 * pci_msi_setup_msi_irqs(), then free_msi_irqs(), which in turn calls
+	 * pci_msi_teardown_msi_irqs().
+	 */
+	if (phb->controller_ops.teardown_msi_irqs)
+		phb->controller_ops.teardown_msi_irqs(dev);
 }
-- 
2.17.2