[PATCH v3 0/4] s390/pci: Fix some zpci_report_status() issues

[PATCH v3 0/4] s390/pci: Fix some zpci_report_status() issues

[Niklas Schnelle]

Hi All,

This series fixes four issues found in zpci_report_status().

* A missing pci_dev_put() (patch 1)
* Dropping the device lock too early (patch 2)
* Missing an error report when there is no associated pdev (patch 3)
* An inconsistent error report when skipping recovery on permanent
  failure (patch 4)

Thanks,
Niklas

Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Changes in v3:
- Change in approach, pass the pdev as a parameter
- Add a device lock assertion and extract a helper function for pdev
  specific status
- Also report the status if no pdev is associated
- Fix an inconsistent report when recovery is skipped due to permanent
  failure
- Link to v2: https://lore.kernel.org/r/20260819-fix_zpci_report_status_pdev_leak-v2-0-da0c4aa7c0ca@linux.ibm.com

Changes in v2:
- Added a second fix to address Sashiko's finding of having
  to hold the device lock during the call to zpci_report_status()
- Add R-bs from Matthew and Farhan
- Link to v1: https://lore.kernel.org/r/20260818-fix_zpci_report_status_pdev_leak-v1-1-576520f4d068@linux.ibm.com

---
Niklas Schnelle (4):
      s390/pci: Fix leak of struct pci_dev reference in zpci_report_status()
      s390/pci: Fix missing device lock in zpci_report_status()
      s390/pci: Report SCLP status on error events when no pdev is associated
      s390/pci: Don't report recovery success on skipped recovery

 arch/s390/pci/pci_event.c  |  7 +++++--
 arch/s390/pci/pci_report.c | 32 ++++++++++++++++++++------------
 arch/s390/pci/pci_report.h |  4 +++-
 3 files changed, 28 insertions(+), 15 deletions(-)
---
base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
change-id: 20260817-fix_zpci_report_status_pdev_leak-1b101c1fe021

Best regards,
-- 
Niklas Schnelle

[PATCH v3 1/4] s390/pci: Fix leak of struct pci_dev reference in zpci_report_status()

[Niklas Schnelle]

In zpci_report_status(), a reference to the pdev associated with the
zdev being reported about is acquired using pci_get_slot(). This
reference needs to be dropped with pci_dev_put(), but this call is
missing, thus leaking the reference. On subsequent hot unplug, this will
cause the struct pci_dev to not be released, leaking memory and
preventing reattach.

At the same time, the only existing caller already holds a pdev
reference. So instead of reacquiring and then dropping another reference,
simply pass the existing pdev pointer to zpci_report_status(). This gets
rid of the need for pci_get_slot() as well as the zdev->zbus check.

Cc: stable@vger.kernel.org
Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 arch/s390/pci/pci_event.c  |  2 +-
 arch/s390/pci/pci_report.c | 11 +++++------
 arch/s390/pci/pci_report.h |  4 +++-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index f317a1465dad..3b4941b65840 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -298,7 +298,7 @@ static pci_ers_result_t zpci_event_attempt_error_recovery(struct pci_dev *pdev,
 	pci_uevent_ers(pdev, PCI_ERS_RESULT_RECOVERED);
 out_unlock:
 	device_unlock(&pdev->dev);
-	zpci_report_status(zdev, "recovery", status_str);
+	zpci_report_status(zdev, pdev, "recovery", status_str);
 
 	return ers_res;
 }
diff --git a/arch/s390/pci/pci_report.c b/arch/s390/pci/pci_report.c
index 7030f7052926..867419779219 100644
--- a/arch/s390/pci/pci_report.c
+++ b/arch/s390/pci/pci_report.c
@@ -89,7 +89,8 @@ static struct debug_view debug_log_view = {
 
 /**
  * zpci_report_status - Report the status of operations on a PCI device
- * @zdev:	The PCI device for which to report status
+ * @zdev:	The zPCI device for which to report status
+ * @pdev:	The PCI device associated with the zdev if any, NULL otherwise
  * @operation:	A string representing the operation reported
  * @status:	A string representing the status of the operation
  *
@@ -103,15 +104,15 @@ static struct debug_view debug_log_view = {
  *
  * Return: 0 on success an error code < 0 otherwise.
  */
-int zpci_report_status(struct zpci_dev *zdev, const char *operation, const char *status)
+int zpci_report_status(struct zpci_dev *zdev, struct pci_dev *pdev,
+		       const char *operation, const char *status)
 {
 	struct zpci_report_error *report;
 	struct pci_driver *driver = NULL;
-	struct pci_dev *pdev = NULL;
 	char *buf, *end;
 	int ret;
 
-	if (!zdev || !zdev->zbus)
+	if (!zdev)
 		return -ENODEV;
 
 	/* Protected virtualization hosts get nothing from us */
@@ -121,8 +122,6 @@ int zpci_report_status(struct zpci_dev *zdev, const char *operation, const char
 	report = (void *)get_zeroed_page(GFP_KERNEL);
 	if (!report)
 		return -ENOMEM;
-	if (zdev->zbus->bus)
-		pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
 	if (pdev)
 		driver = to_pci_driver(pdev->dev.driver);
 
diff --git a/arch/s390/pci/pci_report.h b/arch/s390/pci/pci_report.h
index e08003d51a97..dd7b0b05001c 100644
--- a/arch/s390/pci/pci_report.h
+++ b/arch/s390/pci/pci_report.h
@@ -8,9 +8,11 @@
  */
 #ifndef __S390_PCI_REPORT_H
 #define __S390_PCI_REPORT_H
+#include <linux/pci.h>
 
 struct zpci_dev;
 
-int zpci_report_status(struct zpci_dev *zdev, const char *operation, const char *status);
+int zpci_report_status(struct zpci_dev *zdev, struct pci_dev *pdev,
+		       const char *operation, const char *status);
 
 #endif /* __S390_PCI_REPORT_H */

-- 
2.53.0

[PATCH v3 2/4] s390/pci: Fix missing device lock in zpci_report_status()

[Niklas Schnelle]

When pdev is non-NULL, zpci_report_status() accesses the device's driver.
To get a consistent state matching the recovery, the device lock needs to
be held. Do so by expanding the existing device lock critical section.

The lock only needs to be held when the pdev is non-NULL, so extract
the pdev-specific reporting into a helper function which also adds a
lockdep assertion to detect calls without the device lock held.

Cc: stable@vger.kernel.org
Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 arch/s390/pci/pci_event.c  |  2 +-
 arch/s390/pci/pci_report.c | 21 +++++++++++++++------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 3b4941b65840..ec93f34b6e19 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -297,8 +297,8 @@ static pci_ers_result_t zpci_event_attempt_error_recovery(struct pci_dev *pdev,
 		driver->err_handler->resume(pdev);
 	pci_uevent_ers(pdev, PCI_ERS_RESULT_RECOVERED);
 out_unlock:
-	device_unlock(&pdev->dev);
 	zpci_report_status(zdev, pdev, "recovery", status_str);
+	device_unlock(&pdev->dev);
 
 	return ers_res;
 }
diff --git a/arch/s390/pci/pci_report.c b/arch/s390/pci/pci_report.c
index 867419779219..72ecabf7003c 100644
--- a/arch/s390/pci/pci_report.c
+++ b/arch/s390/pci/pci_report.c
@@ -87,6 +87,19 @@ static struct debug_view debug_log_view = {
 	NULL
 };
 
+static ssize_t zpci_report_pdev(struct pci_dev *pdev, char *buf, size_t size)
+{
+	struct pci_driver *driver;
+	const char *start = buf;
+	char *end = buf + size;
+
+	device_lock_assert(&pdev->dev);
+	buf += scnprintf(buf, end - buf, "state: %s\n", zpci_state_str(pdev->error_state));
+	driver = to_pci_driver(pdev->dev.driver);
+	buf += scnprintf(buf, end - buf, "driver: %s\n", (driver) ? driver->name : "n/a");
+	return buf - start;
+}
+
 /**
  * zpci_report_status - Report the status of operations on a PCI device
  * @zdev:	The zPCI device for which to report status
@@ -108,7 +121,6 @@ int zpci_report_status(struct zpci_dev *zdev, struct pci_dev *pdev,
 		       const char *operation, const char *status)
 {
 	struct zpci_report_error *report;
-	struct pci_driver *driver = NULL;
 	char *buf, *end;
 	int ret;
 
@@ -122,16 +134,13 @@ int zpci_report_status(struct zpci_dev *zdev, struct pci_dev *pdev,
 	report = (void *)get_zeroed_page(GFP_KERNEL);
 	if (!report)
 		return -ENOMEM;
-	if (pdev)
-		driver = to_pci_driver(pdev->dev.driver);
 
 	buf = report->data.log_data;
 	end = report->data.log_data + ZPCI_REPORT_DATA_SIZE;
 	buf += scnprintf(buf, end - buf, "report: %s\n", operation);
 	buf += scnprintf(buf, end - buf, "status: %s\n", status);
-	buf += scnprintf(buf, end - buf, "state: %s\n",
-			 (pdev) ? zpci_state_str(pdev->error_state) : "n/a");
-	buf += scnprintf(buf, end - buf, "driver: %s\n", (driver) ? driver->name : "n/a");
+	if (pdev)
+		buf += zpci_report_pdev(pdev, buf, end - buf);
 	ret = debug_dump(pci_debug_msg_id, &debug_log_view, buf, end - buf, true);
 	if (ret < 0)
 		pr_err("Reading PCI debug messages failed with code %d\n", ret);

-- 
2.53.0

[PATCH v3 3/4] s390/pci: Report SCLP status on error events when no pdev is associated

[Niklas Schnelle]

With commit 4ec6054e7321 ("s390/pci: Report PCI error recovery results
via SCLP") SCLP reports are generated when recovery is performed in
response to an error event. If such an error event arrives but no pdev
is currently associated with the zdev, e.g. because it was removed or
not yet probed, no report is generated. Fix this by generating a report
specific to an error event for a zdev without an associated pdev.

Cc: stable@vger.kernel.org
Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 arch/s390/pci/pci_event.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index ec93f34b6e19..7b538c6ae11b 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -361,8 +361,10 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
 
 	__zpci_event_print_error(pdev, ccdf);
 
-	if (!pdev)
+	if (!pdev) {
+		zpci_report_status(zdev, NULL, "error event", "no pdev bound");
 		goto no_pdev;
+	}
 
 	switch (ccdf->pec) {
 	case 0x002a: /* Error event concerns FMB */

-- 
2.53.0

[PATCH v3 4/4] s390/pci: Don't report recovery success on skipped recovery

[Niklas Schnelle]

When a PCI device is already in the permanent failure state, recovery is
skipped, but the SCLP recovery report still shows success. Fix this by
changing the status string to explicitly state that recovery was skipped
due to permanent failure.

Cc: stable@vger.kernel.org
Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 arch/s390/pci/pci_event.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 7b538c6ae11b..d6af4015223e 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -226,6 +226,7 @@ static pci_ers_result_t zpci_event_attempt_error_recovery(struct pci_dev *pdev,
 	device_lock(&pdev->dev);
 	if (pdev->error_state == pci_channel_io_perm_failure) {
 		ers_res = PCI_ERS_RESULT_DISCONNECT;
+		status_str = "skipped (permanent failure)";
 		goto out_unlock;
 	}
 	pdev->error_state = pci_channel_io_frozen;

-- 
2.53.0

Re: [PATCH v3 1/4] s390/pci: Fix leak of struct pci_dev reference in zpci_report_status()

[Farhan Ali]

On 9/16/2026 8:14 AM, Niklas Schnelle wrote:
> In zpci_report_status(), a reference to the pdev associated with the
> zdev being reported about is acquired using pci_get_slot(). This
> reference needs to be dropped with pci_dev_put(), but this call is
> missing, thus leaking the reference. On subsequent hot unplug, this will
> cause the struct pci_dev to not be released, leaking memory and
> preventing reattach.
>
> At the same time, the only existing caller already holds a pdev
> reference. So instead of reacquiring and then dropping another reference,
> simply pass the existing pdev pointer to zpci_report_status(). This gets
> rid of the need for pci_get_slot() as well as the zdev->zbus check.
>
> Cc:stable@vger.kernel.org
> Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
> Signed-off-by: Niklas Schnelle<schnelle@linux.ibm.com>
> ---
>   arch/s390/pci/pci_event.c  |  2 +-
>   arch/s390/pci/pci_report.c | 11 +++++------
>   arch/s390/pci/pci_report.h |  4 +++-
>   3 files changed, 9 insertions(+), 8 deletions(-)

Reviewed-by: Farhan Ali <alifm@linux.ibm.com>

Re: [PATCH v3 2/4] s390/pci: Fix missing device lock in zpci_report_status()

[Farhan Ali]

On 9/16/2026 8:14 AM, Niklas Schnelle wrote:
> When pdev is non-NULL, zpci_report_status() accesses the device's driver.
> To get a consistent state matching the recovery, the device lock needs to
> be held. Do so by expanding the existing device lock critical section.
>
> The lock only needs to be held when the pdev is non-NULL, so extract
> the pdev-specific reporting into a helper function which also adds a
> lockdep assertion to detect calls without the device lock held.
>
> Cc:stable@vger.kernel.org
> Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
> Signed-off-by: Niklas Schnelle<schnelle@linux.ibm.com>
> ---
>   arch/s390/pci/pci_event.c  |  2 +-
>   arch/s390/pci/pci_report.c | 21 +++++++++++++++------
>   2 files changed, 16 insertions(+), 7 deletions(-)

Reviewed-by: Farhan Ali <alifm@linux.ibm.com>

Re: [PATCH v3 3/4] s390/pci: Report SCLP status on error events when no pdev is associated

[Farhan Ali]

On 9/16/2026 8:14 AM, Niklas Schnelle wrote:
> With commit 4ec6054e7321 ("s390/pci: Report PCI error recovery results
> via SCLP") SCLP reports are generated when recovery is performed in
> response to an error event. If such an error event arrives but no pdev
> is currently associated with the zdev, e.g. because it was removed or
> not yet probed, no report is generated. Fix this by generating a report
> specific to an error event for a zdev without an associated pdev.
>
> Cc:stable@vger.kernel.org
> Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
> Signed-off-by: Niklas Schnelle<schnelle@linux.ibm.com>
> ---
>   arch/s390/pci/pci_event.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Farhan Ali <alifm@linux.ibm.com>

Re: [PATCH v3 4/4] s390/pci: Don't report recovery success on skipped recovery

[Farhan Ali]

On 9/16/2026 8:14 AM, Niklas Schnelle wrote:
> When a PCI device is already in the permanent failure state, recovery is
> skipped, but the SCLP recovery report still shows success. Fix this by
> changing the status string to explicitly state that recovery was skipped
> due to permanent failure.
>
> Cc:stable@vger.kernel.org
> Fixes: 4ec6054e7321 ("s390/pci: Report PCI error recovery results via SCLP")
> Signed-off-by: Niklas Schnelle<schnelle@linux.ibm.com>
> ---
>   arch/s390/pci/pci_event.c | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Farhan Ali <alifm@linux.ibm.com>