Skip to content

Commit 0b13525

Browse files
niklas88hcahca
authored andcommitted
s390/pci: fix leak of PCI device structure
In commit 05bc1be ("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: 05bc1be ("s390/pci: create zPCI bus") Cc: <stable@vger.kernel.org> # 5.8+: e1bff84 s390/pci: remove superfluous zdev->zbus check Cc: <stable@vger.kernel.org> # 5.8+: ba764dd 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>
1 parent d54cb7d commit 0b13525

4 files changed

Lines changed: 33 additions & 18 deletions

File tree

arch/s390/include/asm/pci.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ extern unsigned int s390_pci_no_rid;
202202
----------------------------------------------------------------------------- */
203203
/* Base stuff */
204204
int zpci_create_device(u32 fid, u32 fh, enum zpci_state state);
205-
void zpci_remove_device(struct zpci_dev *zdev);
205+
void zpci_remove_device(struct zpci_dev *zdev, bool set_error);
206206
int zpci_enable_device(struct zpci_dev *);
207207
int zpci_disable_device(struct zpci_dev *);
208208
int zpci_register_ioat(struct zpci_dev *, u8, u64, u64, u64);

arch/s390/pci/pci.c

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -682,16 +682,36 @@ int zpci_disable_device(struct zpci_dev *zdev)
682682
}
683683
EXPORT_SYMBOL_GPL(zpci_disable_device);
684684

685-
void zpci_remove_device(struct zpci_dev *zdev)
685+
/* zpci_remove_device - Removes the given zdev from the PCI core
686+
* @zdev: the zdev to be removed from the PCI core
687+
* @set_error: if true the device's error state is set to permanent failure
688+
*
689+
* Sets a zPCI device to a configured but offline state; the zPCI
690+
* device is still accessible through its hotplug slot and the zPCI
691+
* API but is removed from the common code PCI bus, making it
692+
* no longer available to drivers.
693+
*/
694+
void zpci_remove_device(struct zpci_dev *zdev, bool set_error)
686695
{
687696
struct zpci_bus *zbus = zdev->zbus;
688697
struct pci_dev *pdev;
689698

699+
if (!zdev->zbus->bus)
700+
return;
701+
690702
pdev = pci_get_slot(zbus->bus, zdev->devfn);
691703
if (pdev) {
692-
if (pdev->is_virtfn)
693-
return zpci_iov_remove_virtfn(pdev, zdev->vfn);
704+
if (set_error)
705+
pdev->error_state = pci_channel_io_perm_failure;
706+
if (pdev->is_virtfn) {
707+
zpci_iov_remove_virtfn(pdev, zdev->vfn);
708+
/* balance pci_get_slot */
709+
pci_dev_put(pdev);
710+
return;
711+
}
694712
pci_stop_and_remove_bus_device_locked(pdev);
713+
/* balance pci_get_slot */
714+
pci_dev_put(pdev);
695715
}
696716
}
697717

@@ -765,7 +785,7 @@ void zpci_release_device(struct kref *kref)
765785
struct zpci_dev *zdev = container_of(kref, struct zpci_dev, kref);
766786

767787
if (zdev->zbus->bus)
768-
zpci_remove_device(zdev);
788+
zpci_remove_device(zdev, false);
769789

770790
switch (zdev->state) {
771791
case ZPCI_FN_STATE_ONLINE:

arch/s390/pci/pci_event.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,10 @@ void zpci_event_error(void *data)
7676
static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
7777
{
7878
struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
79-
struct pci_dev *pdev = NULL;
8079
enum zpci_state state;
80+
struct pci_dev *pdev;
8181
int ret;
8282

83-
if (zdev && zdev->zbus->bus)
84-
pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
85-
8683
zpci_err("avail CCDF:\n");
8784
zpci_err_hex(ccdf, sizeof(*ccdf));
8885

@@ -124,8 +121,7 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
124121
case 0x0303: /* Deconfiguration requested */
125122
if (!zdev)
126123
break;
127-
if (pdev)
128-
zpci_remove_device(zdev);
124+
zpci_remove_device(zdev, false);
129125

130126
ret = zpci_disable_device(zdev);
131127
if (ret)
@@ -140,12 +136,10 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
140136
case 0x0304: /* Configured -> Standby|Reserved */
141137
if (!zdev)
142138
break;
143-
if (pdev) {
144-
/* Give the driver a hint that the function is
145-
* already unusable. */
146-
pdev->error_state = pci_channel_io_perm_failure;
147-
zpci_remove_device(zdev);
148-
}
139+
/* Give the driver a hint that the function is
140+
* already unusable.
141+
*/
142+
zpci_remove_device(zdev, true);
149143

150144
zdev->fh = ccdf->fh;
151145
zpci_disable_device(zdev);

drivers/pci/hotplug/s390_pci_hpc.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,9 @@ static int disable_slot(struct hotplug_slot *hotplug_slot)
9393
pci_dev_put(pdev);
9494
return -EBUSY;
9595
}
96+
pci_dev_put(pdev);
9697

97-
zpci_remove_device(zdev);
98+
zpci_remove_device(zdev, false);
9899

99100
rc = zpci_disable_device(zdev);
100101
if (rc)

0 commit comments

Comments
 (0)