s390/pci: fix leak of PCI device structure

BugLink: https://bugs.launchpad.net/bugs/1943464
In commit 05bc1be6db4b2 ("s390/pci: create zPCI bus") we removed the
pci_dev_put() call matching the earlier pci_get_slot() done as part of
__zpci_event_availability(). This was based on the wrong understanding
that the device_put() done as part of pci_destroy_device() would counter
the pci_get_slot() when it only counters the initial reference. This
same understanding and existing bad example also lead to not doing
a pci_dev_put() in zpci_remove_device().

Since releasing the PCI devices, unlike releasing the PCI slot, does not
print any debug message for testing I added one in pci_release_dev().
This revealed that we are indeed leaking the PCI device on PCI
hotunplug. Further testing also revealed another missing pci_dev_put() in
disable_slot().

Fix this by adding the missing pci_dev_put() in disable_slot() and fix
zpci_remove_device() with the correct pci_dev_put() calls. Also instead
of calling pci_get_slot() in __zpci_event_availability() to determine if
a PCI device is registered and then doing the same again in
zpci_remove_device() do this once in zpci_remove_device() which makes
sure that the pdev in __zpci_event_availability() is only used for the
result of pci_scan_single_device() which does not need a reference count
decremnt as its ownership goes to the PCI bus.

Also move the check if zdev->zbus->bus is set into zpci_remove_device()
since it may be that we're removing a device with devfn != 0 which never
had a PCI bus. So we can still set the pdev->error_state to indicate
that the device is not usable anymore, add a flag to set the error state.

Fixes: 05bc1be6db4b2 ("s390/pci: create zPCI bus")
Cc: <stable@vger.kernel.org> # 5.8+: e1bff843cde6 s390/pci: remove superfluous zdev->zbus check
Cc: <stable@vger.kernel.org> # 5.8+: ba764dd703fe s390/pci: refactor zpci_create_device()
Cc: <stable@vger.kernel.org> # 5.8+
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
(backported from commit 0b13525c20febcfecccf6fc1db5969727401317d)
Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Acked-by: Tim Gardner <tim.gardner@canonical.com>
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index ea0daab3eb80e96dc660fff54736d3c9586dd420..5bcac3ac872d4e05816365685f6e78c3653c049b 100644 (file)
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -201,8 +201,8 @@ extern unsigned int s390_pci_no_rid;
   Prototypes
 ----------------------------------------------------------------------------- */
 /* Base stuff */
-int zpci_create_device(struct zpci_dev *);
-void zpci_remove_device(struct zpci_dev *zdev);
+int zpci_create_device(struct zpci_dev *zdev);
+void zpci_remove_device(struct zpci_dev *zdev, bool set_error);
 int zpci_enable_device(struct zpci_dev *);
 int zpci_disable_device(struct zpci_dev *);
 int zpci_register_ioat(struct zpci_dev *, u8, u64, u64, u64);

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 55b7a8a489405f05bcf867870f13936cd80d2669..5e9b6b2185c9f9e803bf9d6cae0553db76969072 100644 (file)
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -717,16 +717,36 @@ int zpci_disable_device(struct zpci_dev *zdev)
 }
 EXPORT_SYMBOL_GPL(zpci_disable_device);
 
-void zpci_remove_device(struct zpci_dev *zdev)
+/* zpci_remove_device - Removes the given zdev from the PCI core
+ * @zdev: the zdev to be removed from the PCI core
+ * @set_error: if true the device's error state is set to permanent failure
+ *
+ * Sets a zPCI device to a configured but offline state; the zPCI
+ * device is still accessible through its hotplug slot and the zPCI
+ * API but is removed from the common code PCI bus, making it
+ * no longer available to drivers.
+ */
+void zpci_remove_device(struct zpci_dev *zdev, bool set_error)
 {
        struct zpci_bus *zbus = zdev->zbus;
        struct pci_dev *pdev;
 
+       if (!zdev->zbus->bus)
+               return;
+
        pdev = pci_get_slot(zbus->bus, zdev->devfn);
        if (pdev) {
-               if (pdev->is_virtfn)
-                       return zpci_remove_virtfn(pdev, zdev->vfn);
+               if (set_error)
+                       pdev->error_state = pci_channel_io_perm_failure;
+               if (pdev->is_virtfn) {
+                       zpci_remove_virtfn(pdev, zdev->vfn);
+                       /* balance pci_get_slot */
+                       pci_dev_put(pdev);
+                       return;
+               }
                pci_stop_and_remove_bus_device_locked(pdev);
+               /* balance pci_get_slot */
+               pci_dev_put(pdev);
        }
 }
 
@@ -775,7 +795,7 @@ void zpci_release_device(struct kref *kref)
        struct zpci_dev *zdev = container_of(kref, struct zpci_dev, kref);
 
        if (zdev->zbus->bus)
-               zpci_remove_device(zdev);
+               zpci_remove_device(zdev, false);
 
        switch (zdev->state) {
        case ZPCI_FN_STATE_ONLINE:

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 40a569b548cc156e5812037586d445705d515617..c5b7980ff937eed94da369a3eb98be541ecfa600 100644 (file)
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -76,13 +76,10 @@ void zpci_event_error(void *data)
 static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 {
        struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
-       struct pci_dev *pdev = NULL;
        enum zpci_state state;
+       struct pci_dev *pdev;
        int ret;
 
-       if (zdev && zdev->zbus && zdev->zbus->bus)
-               pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
-
        zpci_err("avail CCDF:\n");
        zpci_err_hex(ccdf, sizeof(*ccdf));
 
@@ -124,8 +121,7 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
        case 0x0303: /* Deconfiguration requested */
                if (!zdev)
                        break;
-               if (pdev)
-                       zpci_remove_device(zdev);
+               zpci_remove_device(zdev, false);
 
                ret = zpci_disable_device(zdev);
                if (ret)
@@ -140,12 +136,10 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
        case 0x0304: /* Configured -> Standby|Reserved */
                if (!zdev)
                        break;
-               if (pdev) {
-                       /* Give the driver a hint that the function is
-                        * already unusable. */
-                       pdev->error_state = pci_channel_io_perm_failure;
-                       zpci_remove_device(zdev);
-               }
+               /* Give the driver a hint that the function is
+                * already unusable.
+                */
+               zpci_remove_device(zdev, true);
 
                zdev->fh = ccdf->fh;
                zpci_disable_device(zdev);

diff --git a/drivers/pci/hotplug/s390_pci_hpc.c b/drivers/pci/hotplug/s390_pci_hpc.c
index c9e790c74051f6215b7c8f1057234f2a169ee965..a047c421debe2e10606f8649f015436953daa7d1 100644 (file)
--- a/drivers/pci/hotplug/s390_pci_hpc.c
+++ b/drivers/pci/hotplug/s390_pci_hpc.c
@@ -93,8 +93,9 @@ static int disable_slot(struct hotplug_slot *hotplug_slot)
                pci_dev_put(pdev);
                return -EBUSY;
        }
+       pci_dev_put(pdev);
 
-       zpci_remove_device(zdev);
+       zpci_remove_device(zdev, false);
 
        rc = zpci_disable_device(zdev);
        if (rc)