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

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 a50297cf8235 ("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.

Niklas Schnelle committed 3 years ago

Commit ab909501 parent a52e5cd5 files · +38 −15Browse files

Unified · Split

5 files

arch/s390/pci/pci.c +10 −6
arch/s390/pci/pci_bus.c +5 −7
arch/s390/pci/pci_bus.h +1 −2
drivers/pci/bus.c +21 −0
include/linux/pci.h +1 −0

arch/s390/pci/pci.c+10−6

 	return r;
 }
-int zpci_setup_bus_resources(struct zpci_dev *zdev,
+int zpci_setup_bus_resources(struct zpci_dev *zdev)
-			     struct list_head *resources)
 {
 	unsigned long addr, size, flags;
 	struct resource *res;
 			return -ENOMEM;
 		}
 		zdev->bars[i].res = res;
-		pci_add_resource(resources, res);
 	}
 	zdev->has_resources = 1;
 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);
+		zdev->bars[i].res = NULL;
-		kfree(zdev->bars[i].res);
+		kfree(res);
 	}
 	zdev->has_resources = 0;
+	pci_unlock_rescan_remove();
 }
 int pcibios_device_add(struct pci_dev *pdev)

arch/s390/pci/pci_bus.c+5−7

  */
 static int zpci_bus_prepare_device(struct zpci_dev *zdev)
 {
-	struct resource_entry *window, *n;
+	int rc, i;
-	struct resource *res;
-	int rc;
 	if (!zdev_enabled(zdev)) {
 		rc = zpci_enable_device(zdev);
 	}
 	if (!zdev->has_resources) {
-		zpci_setup_bus_resources(zdev, &zdev->zbus->resources);
+		zpci_setup_bus_resources(zdev);
-		resource_list_for_each_entry_safe(window, n, &zdev->zbus->resources) {
+		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
-			res = window->res;
+			if (zdev->bars[i].res)
-			pci_bus_add_resource(zdev->zbus->bus, res, 0);
+				pci_bus_add_resource(zdev->zbus->bus, zdev->bars[i].res, 0);
 		}
 	}

arch/s390/pci/pci_bus.h+1−2

@@ -30,8 +30,7 @@ static inline void zpci_zdev_get(struct zpci_dev *zdev) · 29 unmodified lines
30		30
31	int zpci_alloc_domain(int domain);	31	int zpci_alloc_domain(int domain);
32	void zpci_free_domain(int domain);	32	void zpci_free_domain(int domain);
33	int zpci_setup_bus_resources(struct zpci_dev *zdev,	33	int zpci_setup_bus_resources(struct zpci_dev *zdev);
34	struct list_head *resources);
35		34
36	static inline struct zpci_dev zdev_from_bus(struct pci_bus bus,	35	static inline struct zpci_dev zdev_from_bus(struct pci_bus bus,
37	unsigned int devfn)	36	unsigned int devfn)

drivers/pci/bus.c+21−0

@@ -76,6 +76,27 @@ struct resource pci_bus_resource_n(const struct pci_bus bus, int n) · 75 unmodified lines
76	}	76	}
77	EXPORT_SYMBOL_GPL(pci_bus_resource_n);	77	EXPORT_SYMBOL_GPL(pci_bus_resource_n);
78		78
		79	void pci_bus_remove_resource(struct pci_bus bus, struct resource res)
		80	{
		81	struct pci_bus_resource bus_res, tmp;
		82	int i;
		83
		84	for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) {
		85	if (bus->resource[i] == res) {
		86	bus->resource[i] = NULL;
		87	return;
		88	}
		89	}
		90
		91	list_for_each_entry_safe(bus_res, tmp, &bus->resources, list) {
		92	if (bus_res->res == res) {
		93	list_del(&bus_res->list);
		94	kfree(bus_res);
		95	return;
		96	}
		97	}
		98	}
		99
79	void pci_bus_remove_resources(struct pci_bus *bus)	100	void pci_bus_remove_resources(struct pci_bus *bus)
80	{	101	{
81	int i;	102	int i;

include/linux/pci.h+1−0

@@ -1438,6 +1438,7 @@ void pci_bus_add_resource(struct pci_bus bus, struct resource res, · 1437 unmodified lines
1438	unsigned int flags);	1438	unsigned int flags);
1439	struct resource pci_bus_resource_n(const struct pci_bus bus, int n);	1439	struct resource pci_bus_resource_n(const struct pci_bus bus, int n);
1440	void pci_bus_remove_resources(struct pci_bus *bus);	1440	void pci_bus_remove_resources(struct pci_bus *bus);
		1441	void pci_bus_remove_resource(struct pci_bus bus, struct resource res);
1441	int devm_request_pci_bus_resources(struct device *dev,	1442	int devm_request_pci_bus_resources(struct device *dev,
1442	struct list_head *resources);	1443	struct list_head *resources);
1443		1444