PCI: s390: Fix use-after-free of PCI resources with per-function hotplug

[ Upstream commit 22e98ad36790b584c3af7a9c2941a0ff26db0814 ]

On s390 PCI functions may be hotplugged individually even when they
belong to a multi-function device. In particular on an SR-IOV device VFs
may be removed and later re-added.

In commit 9f075e2b9fd8 ("s390/pci: separate zbus creation from
scanning") it was missed however that struct pci_bus and struct
zpci_bus's resource list retained a reference to the PCI functions MMIO
resources even though those resources are released and freed on
hot-unplug. These stale resources may subsequently be claimed when the
PCI function re-appears resulting in use-after-free.

One idea of fixing this use-after-free in s390 specific code that was
investigated was to simply keep resources around from the moment a PCI
function first appeared until the whole virtual PCI bus created for
a multi-function device disappears. The problem with this however is
that due to the requirement of artificial MMIO addreesses (address
cookies) extra logic is then needed to keep the address cookies
compatible on re-plug. At the same time the MMIO resources semantically
belong to the PCI function so tying their lifecycle to the function
seems more logical.

Instead a simpler approach is to remove the resources of an individually
hot-unplugged PCI function from the PCI bus's resource list while
keeping the resources of other PCI functions on the PCI bus untouched.

This is done by introducing pci_bus_remove_resource() to remove an
individual resource. Similarly the resource also needs to be removed
from the struct zpci_bus's resource list. It turns out however, that
there is really no need to add the MMIO resources to the struct
zpci_bus's resource list at all and instead we can simply use the
zpci_bar_struct's resource pointer directly.

Fixes: 9f075e2b9fd8 ("s390/pci: separate zbus creation from scanning")
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://lore.kernel.org/r/20230306151014.60913-2-schnelle@linux.ibm.com
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 73cdc55393847adee690f015d1ef0ce37714f97e..2c99f9552b2f5c8d4dd913e86a8f654fe0e80aee 100644 (file)
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -544,8 +544,7 @@ static struct resource *__alloc_res(struct zpci_dev *zdev, unsigned long start,
        return r;
 }
 
-int zpci_setup_bus_resources(struct zpci_dev *zdev,
-                            struct list_head *resources)
+int zpci_setup_bus_resources(struct zpci_dev *zdev)
 {
        unsigned long addr, size, flags;
        struct resource *res;
@@ -581,7 +580,6 @@ int zpci_setup_bus_resources(struct zpci_dev *zdev,
                        return -ENOMEM;
                }
                zdev->bars[i].res = res;
-               pci_add_resource(resources, res);
        }
        zdev->has_resources = 1;
 
@@ -590,17 +588,23 @@ int zpci_setup_bus_resources(struct zpci_dev *zdev,
 
 static void zpci_cleanup_bus_resources(struct zpci_dev *zdev)
 {
+       struct resource *res;
        int i;
 
+       pci_lock_rescan_remove();
        for (i = 0; i < PCI_STD_NUM_BARS; i++) {
-               if (!zdev->bars[i].size || !zdev->bars[i].res)
+               res = zdev->bars[i].res;
+               if (!res)
                        continue;
 
+               release_resource(res);
+               pci_bus_remove_resource(zdev->zbus->bus, res);
                zpci_free_iomap(zdev, zdev->bars[i].map_idx);
-               release_resource(zdev->bars[i].res);
-               kfree(zdev->bars[i].res);
+               zdev->bars[i].res = NULL;
+               kfree(res);
        }
        zdev->has_resources = 0;
+       pci_unlock_rescan_remove();
 }
 
 int pcibios_device_add(struct pci_dev *pdev)

diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c
index 6a8da1b742ae5aa76d08d6abb34e94735bc1f08e..a99926af2b69a352cdd7480ed1f2888f1728a7f9 100644 (file)
--- a/arch/s390/pci/pci_bus.c
+++ b/arch/s390/pci/pci_bus.c
@@ -41,9 +41,7 @@ static int zpci_nb_devices;
  */
 static int zpci_bus_prepare_device(struct zpci_dev *zdev)
 {
-       struct resource_entry *window, *n;
-       struct resource *res;
-       int rc;
+       int rc, i;
 
        if (!zdev_enabled(zdev)) {
                rc = zpci_enable_device(zdev);
@@ -57,10 +55,10 @@ static int zpci_bus_prepare_device(struct zpci_dev *zdev)
        }
 
        if (!zdev->has_resources) {
-               zpci_setup_bus_resources(zdev, &zdev->zbus->resources);
-               resource_list_for_each_entry_safe(window, n, &zdev->zbus->resources) {
-                       res = window->res;
-                       pci_bus_add_resource(zdev->zbus->bus, res, 0);
+               zpci_setup_bus_resources(zdev);
+               for (i = 0; i < PCI_STD_NUM_BARS; i++) {
+                       if (zdev->bars[i].res)
+                               pci_bus_add_resource(zdev->zbus->bus, zdev->bars[i].res, 0);
                }
        }
 

diff --git a/arch/s390/pci/pci_bus.h b/arch/s390/pci/pci_bus.h
index e96c9860e0644b4d1dd144d5c0fccbdf484a60cb..af9f0ac79a1b1b8a249e7a22703f3dec45dcdad8 100644 (file)
--- a/arch/s390/pci/pci_bus.h
+++ b/arch/s390/pci/pci_bus.h
@@ -30,8 +30,7 @@ static inline void zpci_zdev_get(struct zpci_dev *zdev)
 
 int zpci_alloc_domain(int domain);
 void zpci_free_domain(int domain);
-int zpci_setup_bus_resources(struct zpci_dev *zdev,
-                            struct list_head *resources);
+int zpci_setup_bus_resources(struct zpci_dev *zdev);
 
 static inline struct zpci_dev *zdev_from_bus(struct pci_bus *bus,
                                             unsigned int devfn)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 3cef835b375fd6a6adaacd1384987a5f905e9c46..feafa378bf8ea1cd547dba104edc1365a46afd3b 100644 (file)
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -76,6 +76,27 @@ struct resource *pci_bus_resource_n(const struct pci_bus *bus, int n)
 }
 EXPORT_SYMBOL_GPL(pci_bus_resource_n);
 
+void pci_bus_remove_resource(struct pci_bus *bus, struct resource *res)
+{
+       struct pci_bus_resource *bus_res, *tmp;
+       int i;
+
+       for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) {
+               if (bus->resource[i] == res) {
+                       bus->resource[i] = NULL;
+                       return;
+               }
+       }
+
+       list_for_each_entry_safe(bus_res, tmp, &bus->resources, list) {
+               if (bus_res->res == res) {
+                       list_del(&bus_res->list);
+                       kfree(bus_res);
+                       return;
+               }
+       }
+}
+
 void pci_bus_remove_resources(struct pci_bus *bus)
 {
        int i;

diff --git a/include/linux/pci.h b/include/linux/pci.h
index cb538bc5797106d8bf70c741b331acba5065ab11..d20695184e0b9cdad36ce4c66cdb02ed51c222ba 100644 (file)
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1417,6 +1417,7 @@ void pci_bus_add_resource(struct pci_bus *bus, struct resource *res,
                          unsigned int flags);
 struct resource *pci_bus_resource_n(const struct pci_bus *bus, int n);
 void pci_bus_remove_resources(struct pci_bus *bus);
+void pci_bus_remove_resource(struct pci_bus *bus, struct resource *res);
 int devm_request_pci_bus_resources(struct device *dev,
                                   struct list_head *resources);