All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration
@ 2026-08-19  8:50 Tobias Schumacher
  2026-08-19  8:50 ` [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup Tobias Schumacher
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:50 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher, stable

Commit f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ
domain API") introduced several bugs in error handling and cleanup
paths. This series fixes these issues:

1. Null pointer dereference and double-free in MSI cleanup
2. Use-after-free race in floating interrupt cleanup
3. Resource leak in MSI setup error path
4. Wrong number of IRQs freed in directed-mode teardown
5. Missing NULL checks in zpci_msi_clear_airq()
6. Resource leak in zpci_directed_irq_init() error path
7. Inefficient MSI affinity flag initialization

Patches 1-6 fix critical bugs that can cause crashes, memory
corruption, or resource exhaustion. Patch 7 is a cleanup that moves
flag initialization to a more appropriate location.

Tobias Schumacher (7):
  s390/pci: fix null pointer dereference and double-free in zpci MSI cleanup
  s390/pci: fix use-after-free race in zpci floating interrupt cleanup
  s390/pci: fix resource leak in zpci MSI setup
  s390/pci: fix MSI directed-mode teardown IRQ bit count
  s390/pci: add NULL check in zpci_msi_clear_airq()
  s390/pci: add error cleanup in zpci_directed_irq_init
  s390/pci: move MSI affinity flag initialization to boot time

Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
Tobias Schumacher (7):
      s390/pci: fix double-free in zpci MSI cleanup
      s390/pci: fix use-after-free race in zpci floating interrupt cleanup
      s390/pci: fix resource leak in zpci MSI setup
      s390/pci: fix MSI directed-mode teardown IRQ bit count
      s390/pci: add NULL check in zpci_msi_clear_airq()
      s390/pci: add error cleanup in zpci_directed_irq_init
      s390/pci: move MSI affinity flag initialization to boot time

 arch/s390/pci/pci_irq.c | 59 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 13 deletions(-)
---
base-commit: ff35cca8fb33b213bb41a20d7ed27ecf0fc3152a
change-id: 20260818-s390_irq_domain_fixes-ad74b3134c51

Best regards,
-- 
Tobias Schumacher <ts@linux.ibm.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
@ 2026-08-19  8:50 ` Tobias Schumacher
  2026-08-19  9:03   ` sashiko-bot
  2026-08-19  8:50 ` [PATCH 2/7] s390/pci: fix use-after-free race in zpci floating interrupt cleanup Tobias Schumacher
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:50 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher, stable

zpci_remove_parent_msi_domain() can be called multiple times on the
same zbus, causing a double-free. This occurs when pci_create_root_bus()
fails after successful MSI domain creation in zpci_bus_create_pci_bus():
the error path calls zpci_remove_parent_msi_domain() to clean up, but
doesn't NULL the pointer. Later, when zpci_bus_release() is called via
kref_put(), it calls zpci_remove_parent_msi_domain() again, attempting
to free the already-freed domain and fwnode.

Add NULL check at function entry and NULL the pointer after cleanup to
make the function idempotent and safe for multiple calls.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 9c9ed3d8d959..c9520a16ca75 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -533,9 +533,13 @@ void zpci_remove_parent_msi_domain(struct zpci_bus *zbus)
 {
 	struct fwnode_handle *fn;
 
+	if (!zbus->msi_parent_domain)
+		return;
+
 	fn = zbus->msi_parent_domain->fwnode;
 	irq_domain_remove(zbus->msi_parent_domain);
 	irq_domain_free_fwnode(fn);
+	zbus->msi_parent_domain = NULL;
 }
 
 static void __init cpu_enable_directed_irq(void *unused)

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/7] s390/pci: fix use-after-free race in zpci floating interrupt cleanup
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
  2026-08-19  8:50 ` [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup Tobias Schumacher
@ 2026-08-19  8:50 ` Tobias Schumacher
  2026-08-19  9:20   ` sashiko-bot
  2026-08-19  8:50 ` [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup Tobias Schumacher
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:50 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher, stable

The interrupt handler reads zpci_ibv[si] without synchronization while
concurrent teardown can release this memory, creating a race:
- handler reads the pointer,
- then teardown frees memory,
- then handler uses the freed pointer.
Fix by protecting array access with RCU synchronization.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index c9520a16ca75..94b03d16006b 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -278,8 +278,14 @@ static void zpci_floating_irq_handler(struct airq_struct *airq,
 			continue;
 		}
 
+		rcu_read_lock();
+
 		/* Scan the adapter interrupt vector for this device. */
-		aibv = zpci_ibv[si];
+		aibv = rcu_dereference(zpci_ibv[si]);
+		if (!aibv) {
+			rcu_read_unlock();
+			continue;
+		}
 		for (ai = 0;;) {
 			ai = airq_iv_scan(aibv, ai, airq_iv_end(aibv));
 			if (ai == -1UL)
@@ -291,6 +297,7 @@ static void zpci_floating_irq_handler(struct airq_struct *airq,
 			generic_handle_domain_irq(msi_domain, hwirq);
 			airq_iv_unlock(aibv, ai);
 		}
+		rcu_read_unlock();
 	}
 }
 
@@ -346,9 +353,12 @@ static void zpci_msi_teardown_directed(struct zpci_dev *zdev)
 
 static void zpci_msi_teardown_floating(struct zpci_dev *zdev)
 {
+	airq_iv_free_bit(zpci_sbv, zdev->aisb);
+	zpci_ibv[zdev->aisb] = NULL;
+	synchronize_rcu();
+
 	airq_iv_release(zdev->aibv);
 	zdev->aibv = NULL;
-	airq_iv_free_bit(zpci_sbv, zdev->aisb);
 	zdev->aisb = -1UL;
 	zdev->msi_first_bit = -1U;
 	zdev->msi_nr_irqs = 0;

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
  2026-08-19  8:50 ` [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup Tobias Schumacher
  2026-08-19  8:50 ` [PATCH 2/7] s390/pci: fix use-after-free race in zpci floating interrupt cleanup Tobias Schumacher
@ 2026-08-19  8:50 ` Tobias Schumacher
  2026-08-19  9:05   ` sashiko-bot
  2026-08-19  8:50 ` [PATCH 4/7] s390/pci: fix MSI directed-mode teardown IRQ bit count Tobias Schumacher
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:50 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher, stable

If airq_iv_create() fails in __alloc_airq(), the zpci_sbv bit allocated
by airq_iv_alloc_bit() is never freed. This permanently leaks one of the
ZPCI_NR_DEVICES summary bits (~128 total), reducing system capacity with
each failed device hotplug. In systems with repeated device insertion
failures or under memory pressure, all summary bits can be exhausted,
preventing new PCI devices from being added until reboot.

Add proper error handling to free the zpci_sbv bit and reset zdev->aisb
if the AIBV creation fails.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 94b03d16006b..5e934ac990ac 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -320,8 +320,11 @@ static int __alloc_airq(struct zpci_dev *zdev, int msi_vecs,
 		zdev->aibv = airq_iv_create(msi_vecs,
 					    AIRQ_IV_PTR | AIRQ_IV_DATA | AIRQ_IV_BITLOCK,
 					    NULL);
-		if (!zdev->aibv)
+		if (!zdev->aibv) {
+			airq_iv_free_bit(zpci_sbv, *bit);
+			zdev->aisb = -1UL;
 			return -ENOMEM;
+		}
 
 		/* Wire up shortcut pointer */
 		zpci_ibv[*bit] = zdev->aibv;

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/7] s390/pci: fix MSI directed-mode teardown IRQ bit count
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
                   ` (2 preceding siblings ...)
  2026-08-19  8:50 ` [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup Tobias Schumacher
@ 2026-08-19  8:50 ` Tobias Schumacher
  2026-08-19  9:07   ` sashiko-bot
  2026-08-19  8:50 ` [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq() Tobias Schumacher
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:50 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher, stable

On s390 with directed interrupts enabled, zpci_msi_teardown_directed()
frees the platform's maximum number of MSI bits (zdev->max_msi) instead
of the actual allocated count (zdev->msi_nr_irqs). This corrupts the
shared IRQ bitmap used by all PCI functions, causing lost interrupts and
heap corruption. Fix zpci_msi_teardown_directed() to only free the
actual allocated IRQ bit count.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 5e934ac990ac..e9eda846cb2d 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -349,7 +349,7 @@ static struct airq_struct zpci_airq = {
 
 static void zpci_msi_teardown_directed(struct zpci_dev *zdev)
 {
-	airq_iv_free(zpci_ibv[0], zdev->msi_first_bit, zdev->max_msi);
+	airq_iv_free(zpci_ibv[0], zdev->msi_first_bit, zdev->msi_nr_irqs);
 	zdev->msi_first_bit = -1U;
 	zdev->msi_nr_irqs = 0;
 }

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq()
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
                   ` (3 preceding siblings ...)
  2026-08-19  8:50 ` [PATCH 4/7] s390/pci: fix MSI directed-mode teardown IRQ bit count Tobias Schumacher
@ 2026-08-19  8:50 ` Tobias Schumacher
  2026-08-19  9:14   ` sashiko-bot
  2026-08-19  8:50 ` [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init Tobias Schumacher
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:50 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher, stable

zpci_msi_clear_airq() clears IRQ data in both DIRECTED and FLOATING
modes but does not check if the interrupt vector pointers are NULL
before dereferencing them.

In DIRECTED mode, zpci_ibv[cpu] can be NULL if:
- zpci_directed_irq_init() fails during boot after allocating some
  but not all per-CPU vectors, and cleanup is attempted
- The system is shutting down and zpci_irq_exit() has already
  released some vectors

In FLOATING mode, zdev->aibv can be NULL if:
- zpci_msi_prepare() fails after __alloc_airq() but before setting
  up the device's AIBV, and zpci_msi_domain_free() is called during
  error cleanup
- The device is being torn down and zpci_msi_teardown_floating()
  has already released the AIBV

Add NULL checks for zpci_ibv[cpu] in DIRECTED mode and zdev->aibv in
FLOATING mode to prevent crashes during these error and shutdown paths.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index e9eda846cb2d..1515d8d7460e 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -465,12 +465,16 @@ static void zpci_msi_clear_airq(struct irq_data *d, int i)
 
 	if (irq_delivery == DIRECTED) {
 		for_each_possible_cpu(cpu) {
-			airq_iv_set_ptr(zpci_ibv[cpu], bit + i, 0);
-			airq_iv_set_data(zpci_ibv[cpu], bit + i, 0);
+			if (zpci_ibv[cpu]) {
+				airq_iv_set_ptr(zpci_ibv[cpu], bit + i, 0);
+				airq_iv_set_data(zpci_ibv[cpu], bit + i, 0);
+			}
 		}
 	} else {
-		airq_iv_set_ptr(zdev->aibv, bit + i, 0);
-		airq_iv_set_data(zdev->aibv, bit + i, 0);
+		if (zdev->aibv) {
+			airq_iv_set_ptr(zdev->aibv, bit + i, 0);
+			airq_iv_set_data(zdev->aibv, bit + i, 0);
+		}
 	}
 }
 

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
                   ` (4 preceding siblings ...)
  2026-08-19  8:50 ` [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq() Tobias Schumacher
@ 2026-08-19  8:50 ` Tobias Schumacher
  2026-08-19  9:02   ` sashiko-bot
  2026-08-19  8:51 ` [PATCH 7/7] s390/pci: move MSI affinity flag initialization to boot time Tobias Schumacher
  2026-08-19  9:24 ` [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Niklas Schnelle
  7 siblings, 1 reply; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:50 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher, stable

If per-CPU airq_iv allocation fails in the loop, previously allocated
vectors and arrays leak. Add proper error path to release all resources
on failure.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 1515d8d7460e..1ddf6b3625a2 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -586,7 +586,7 @@ static int __init zpci_directed_irq_init(void)
 
 	zpci_ibv = kzalloc_objs(*zpci_ibv, num_possible_cpus());
 	if (!zpci_ibv)
-		return -ENOMEM;
+		goto out_free_sbv;
 
 	for_each_possible_cpu(cpu) {
 		/*
@@ -599,13 +599,25 @@ static int __init zpci_directed_irq_init(void)
 					       AIRQ_IV_CACHELINE |
 					       (!cpu ? AIRQ_IV_ALLOC : 0), NULL);
 		if (!zpci_ibv[cpu])
-			return -ENOMEM;
+			goto out_free_ibv;
 	}
 	on_each_cpu(cpu_enable_directed_irq, NULL, 1);
 
 	zpci_irq_chip.irq_set_affinity = zpci_set_irq_affinity;
 
 	return 0;
+
+out_free_ibv:
+	for_each_possible_cpu(cpu) {
+		if (zpci_ibv[cpu])
+			airq_iv_release(zpci_ibv[cpu]);
+	}
+	kfree(zpci_ibv);
+	zpci_ibv = NULL;
+out_free_sbv:
+	airq_iv_release(zpci_sbv);
+	zpci_sbv = NULL;
+	return -ENOMEM;
 }
 
 static int __init zpci_floating_irq_init(void)

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/7] s390/pci: move MSI affinity flag initialization to boot time
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
                   ` (5 preceding siblings ...)
  2026-08-19  8:50 ` [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init Tobias Schumacher
@ 2026-08-19  8:51 ` Tobias Schumacher
  2026-08-19  9:06   ` sashiko-bot
  2026-08-19  9:24 ` [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Niklas Schnelle
  7 siblings, 1 reply; 16+ messages in thread
From: Tobias Schumacher @ 2026-08-19  8:51 UTC (permalink / raw)
  To: Niklas Schnelle, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, Tobias Schumacher

The MSI_FLAG_NO_AFFINITY flag for FLOATING mode is currently set lazily
in zpci_create_parent_msi_domain(), which may be called multiple times
when discovering PCI buses. While setting the flag multiple times is a
no-op due to its idempotent nature, this is inefficient and unclear.

Since irq_delivery mode is determined once at boot time and never
changes, move the flag initialization to zpci_irq_init() where it
belongs. This clarifies intent and avoids redundant repeated operations
on the shared structure.

Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 1ddf6b3625a2..ed19f7cce9eb 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -533,9 +533,6 @@ int zpci_create_parent_msi_domain(struct zpci_bus *zbus)
 		return -ENOMEM;
 	}
 
-	if (irq_delivery == FLOATING)
-		zpci_msi_parent_ops.required_flags |= MSI_FLAG_NO_AFFINITY;
-
 	zbus->msi_parent_domain = msi_create_parent_irq_domain(&info, &zpci_msi_parent_ops);
 	if (!zbus->msi_parent_domain) {
 		irq_domain_free_fwnode(info.fwnode);
@@ -646,6 +643,9 @@ int __init zpci_irq_init(void)
 	if (s390_pci_force_floating)
 		irq_delivery = FLOATING;
 
+	if (irq_delivery == FLOATING)
+		zpci_msi_parent_ops.required_flags |= MSI_FLAG_NO_AFFINITY;
+
 	if (irq_delivery == DIRECTED)
 		zpci_airq.handler = zpci_directed_irq_handler;
 

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init
  2026-08-19  8:50 ` [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init Tobias Schumacher
@ 2026-08-19  9:02   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-19  9:02 UTC (permalink / raw)
  To: Tobias Schumacher
  Cc: Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
	Heiko Carstens, linux-s390

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added error cleanup path in `zpci_directed_irq_init` introduces a DMA use-after-free vulnerability.
--

--- Patch [6]: [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init ---
commit 5a0360870b410d13391fe09d0a8840ffaa9cb9be
Author: Tobias Schumacher <ts@linux.ibm.com>

s390/pci: add error cleanup in zpci_directed_irq_init

This commit adds an error cleanup path to zpci_directed_irq_init to release
airq_iv resources if per-CPU allocation fails, preventing a memory leak.

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index 1515d8d7460ea..1ddf6b3625a26 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -586,7 +586,7 @@ static int __init zpci_directed_irq_init(void)
>  
>  	zpci_ibv = kzalloc_objs(*zpci_ibv, num_possible_cpus());
>  	if (!zpci_ibv)
> -		return -ENOMEM;
> +		goto out_free_sbv;
>  
>  	for_each_possible_cpu(cpu) {

[ ... ]

> @@ -599,13 +599,25 @@ static int __init zpci_directed_irq_init(void)
>  					       AIRQ_IV_CACHELINE |
>  					       (!cpu ? AIRQ_IV_ALLOC : 0), NULL);
>  		if (!zpci_ibv[cpu])
> -			return -ENOMEM;
> +			goto out_free_ibv;
>  	}
>  	on_each_cpu(cpu_enable_directed_irq, NULL, 1);
>  
>  	zpci_irq_chip.irq_set_affinity = zpci_set_irq_affinity;
>  
>  	return 0;
> +
> +out_free_ibv:
> +	for_each_possible_cpu(cpu) {
> +		if (zpci_ibv[cpu])
> +			airq_iv_release(zpci_ibv[cpu]);
> +	}
> +	kfree(zpci_ibv);
> +	zpci_ibv = NULL;
> +out_free_sbv:
> +	airq_iv_release(zpci_sbv);

[Severity: High]
Does this new error path introduce a potential DMA use-after-free?

Just before the allocation of zpci_ibv, the hardware is configured with the
physical address of zpci_sbv->vector:

arch/s390/pci/pci_irq.c:zpci_directed_irq_init() {
    ...
    iib.diib.disb_addr = virt_to_phys(zpci_sbv->vector);
    zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib);

    zpci_ibv = kzalloc_objs(*zpci_ibv, num_possible_cpus());
    if (!zpci_ibv)
        goto out_free_sbv;
    ...
}

If we jump to out_free_sbv and free zpci_sbv via airq_iv_release(zpci_sbv)
without resetting the hardware configuration first, could the adapter
corrupt memory by writing to the freed physical page during a spurious
interrupt?

Would it make sense to either defer the zpci_set_irq_ctrl() hardware
configuration until all software allocations succeed, or unregister the
hardware in the out_free_sbv error path?

> +	zpci_sbv = NULL;
> +	return -ENOMEM;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com?part=6

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup
  2026-08-19  8:50 ` [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup Tobias Schumacher
@ 2026-08-19  9:03   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-19  9:03 UTC (permalink / raw)
  To: Tobias Schumacher
  Cc: Heiko Carstens, Alexander Gordeev, Vasily Gorbik, linux-s390,
	Christian Borntraeger

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Pre-existing memory leak of `struct resource_entry` in `zbus->resources` when PCI root bus creation fails.
--

--- Patch [1]: [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup ---
commit d582c0ae794fba4fb3a5cf7c09155cbe33bdffb9
Author: Tobias Schumacher <ts@linux.ibm.com>

s390/pci: fix double-free in zpci MSI cleanup

zpci_remove_parent_msi_domain() can be called multiple times on the
same zbus, causing a double-free. This occurs when pci_create_root_bus()
fails after successful MSI domain creation in zpci_bus_create_pci_bus():
the error path calls zpci_remove_parent_msi_domain() to clean up, but
doesn't NULL the pointer. Later, when zpci_bus_release() is called via
kref_put(), it calls zpci_remove_parent_msi_domain() again, attempting
to free the already-freed domain and fwnode.

Add NULL check at function entry and NULL the pointer after cleanup to
make the function idempotent and safe for multiple calls.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index 9c9ed3d8d9597..c9520a16ca753 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c

[Severity: Medium]
This is a pre-existing issue, but while reviewing the error path discussed in
the commit message (when pci_create_root_bus() fails), does zpci_bus_release()
leak zbus->resources?

In arch/s390/pci/pci_bus.c:zpci_bus_alloc(), a resource is added to the list:

    INIT_LIST_HEAD(&zbus->resources);
    zbus->bus_resource.start = 0;
    zbus->bus_resource.end = ZPCI_BUS_NR;
    zbus->bus_resource.flags = IORESOURCE_BUS;
    pci_add_resource(&zbus->resources, &zbus->bus_resource);

But in the error path where pci_create_root_bus() fails, zbus->bus is not set.
Later in zpci_bus_release(), pci_free_resource_list() is only called if
zbus->bus is true:

    if (zbus->bus) {
        pci_lock_rescan_remove();
        pci_stop_root_bus(zbus->bus);

        zpci_free_domain(zbus->domain_nr);
        pci_free_resource_list(&zbus->resources);

        pci_remove_root_bus(zbus->bus);
        pci_unlock_rescan_remove();
    }

Can this leak the struct resource_entry on this initialization error path?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com?part=1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup
  2026-08-19  8:50 ` [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup Tobias Schumacher
@ 2026-08-19  9:05   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-19  9:05 UTC (permalink / raw)
  To: Tobias Schumacher
  Cc: Heiko Carstens, Christian Borntraeger, linux-s390,
	Alexander Gordeev, Vasily Gorbik

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] zpci_msi_teardown_directed() unconditionally frees `zdev->max_msi` vectors instead of the dynamically allocated `zdev->msi_nr_irqs`, potentially freeing vectors owned by unrelated PCI devices.
- [High] Missing `rcu_assign_pointer()` when publishing the newly created adapter interrupt vector to the globally visible `zpci_ibv` array.
- [High] zpci_msi_clear_airq() double-counts the iteration index, resulting in skipped elements and out-of-bounds memory corruption.
--

--- Patch [3]: [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup ---
commit ac232ab92e1500cdc9f0a1a221f03f9be96fffe4
Author: Tobias Schumacher <ts@linux.ibm.com>

s390/pci: fix resource leak in zpci MSI setup

If airq_iv_create() fails in __alloc_airq(), the zpci_sbv bit allocated
by airq_iv_alloc_bit() is never freed. This permanently leaks one of the
ZPCI_NR_DEVICES summary bits (~128 total), reducing system capacity with
each failed device hotplug.

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index 94b03d16006ba..5e934ac990ac1 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -320,8 +320,11 @@ static int __alloc_airq(struct zpci_dev *zdev, int msi_vecs,
>  		zdev->aibv = airq_iv_create(msi_vecs,
>  					    AIRQ_IV_PTR | AIRQ_IV_DATA | AIRQ_IV_BITLOCK,
>  					    NULL);
> -		if (!zdev->aibv)
> +		if (!zdev->aibv) {
> +			airq_iv_free_bit(zpci_sbv, *bit);
> +			zdev->aisb = -1UL;
>  			return -ENOMEM;
> +		}
>  
>  		/* Wire up shortcut pointer */
>  		zpci_ibv[*bit] = zdev->aibv;

[Severity: High]
This isn't a bug introduced by this patch, but does this direct assignment
need a memory barrier?

This pointer is published to the globally visible zpci_ibv array.
Concurrently, the asynchronous hardware interrupt handler
zpci_floating_irq_handler() locklessly reads this array using
rcu_dereference(zpci_ibv[si]).

Without an rcu_assign_pointer() or equivalent release barrier, could the
reader observe the aibv pointer before its internal memory regions are
globally visible, leading to an invalid pointer dereference?

[Severity: High]
This is a pre-existing issue, but does zpci_msi_teardown_directed() free
vectors owned by unrelated PCI devices?

When a driver requests fewer IRQs than its maximum capability, __alloc_airq()
dynamically allocates exactly msi_vecs bits. However, the teardown blindly
frees max_msi bits:

arch/s390/pci/pci_irq.c:zpci_msi_teardown_directed() {
    ...
	airq_iv_free(zpci_ibv[0], zdev->msi_first_bit, zdev->max_msi);
    ...
}

Will this cause memory corruption by freeing trailing vectors that might have
been allocated to another device? Should this free zdev->msi_nr_irqs instead?

[Severity: High]
This is a pre-existing issue, but does zpci_msi_clear_airq() double-count
the iteration index and write out of bounds?

In zpci_msi_domain_free(), it iterates over nr_irqs and passes the loop index
i to zpci_msi_clear_airq():

arch/s390/pci/pci_irq.c:zpci_msi_domain_free() {
    ...
	for (i = 0; i < nr_irqs; i++) {
		d = irq_domain_get_irq_data(domain, virq + i);
		zpci_msi_clear_airq(d, i);
    ...
}

Inside zpci_msi_clear_airq(), the hwirq already reflects the correct index.
The bit is calculated, but then i is added again:

arch/s390/pci/pci_irq.c:zpci_msi_clear_airq() {
    ...
	msi_index = zpci_decode_hwirq_msi_index(d->hwirq);
	bit = zdev->msi_first_bit + msi_index;
    ...
		airq_iv_set_ptr(zdev->aibv, bit + i, 0);
    ...
}

If nr_irqs is greater than 1, doesn't this skip the intended elements and
corrupt pointers in unrelated active IRQs while failing to clear the
target vectors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com?part=3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] s390/pci: move MSI affinity flag initialization to boot time
  2026-08-19  8:51 ` [PATCH 7/7] s390/pci: move MSI affinity flag initialization to boot time Tobias Schumacher
@ 2026-08-19  9:06   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-19  9:06 UTC (permalink / raw)
  To: Tobias Schumacher
  Cc: linux-s390, Alexander Gordeev, Christian Borntraeger,
	Vasily Gorbik, Heiko Carstens

> The MSI_FLAG_NO_AFFINITY flag for FLOATING mode is currently set lazily
> in zpci_create_parent_msi_domain(), which may be called multiple times
> when discovering PCI buses. While setting the flag multiple times is a
> no-op due to its idempotent nature, this is inefficient and unclear.
> 
> Since irq_delivery mode is determined once at boot time and never
> changes, move the flag initialization to zpci_irq_init() where it
> belongs. This clarifies intent and avoids redundant repeated operations
> on the shared structure.
> 
> Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com?part=7


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/7] s390/pci: fix MSI directed-mode teardown IRQ bit count
  2026-08-19  8:50 ` [PATCH 4/7] s390/pci: fix MSI directed-mode teardown IRQ bit count Tobias Schumacher
@ 2026-08-19  9:07   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-19  9:07 UTC (permalink / raw)
  To: Tobias Schumacher
  Cc: Alexander Gordeev, linux-s390, Vasily Gorbik,
	Christian Borntraeger, Heiko Carstens

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Double-addition logic error in `zpci_msi_clear_airq()` causes out-of-bounds memory writes and leaves dangling pointers.
- [High] Directed MSI teardown leaks uncleared hardware vector bits on CPUs > 0, causing spurious interrupts.
- [Medium] Missing RCU pointer assignment when publishing `zdev->aibv` to `zpci_ibv` array.
--

--- Patch [4]: [PATCH 4/7] s390/pci: fix MSI directed-mode teardown IRQ bit count ---
commit 061bcdd7920b9b7d42fc840ba0aa403eaf0255a5
Author: Tobias Schumacher <ts@linux.ibm.com>

s390/pci: fix MSI directed-mode teardown IRQ bit count

This commit updates zpci_msi_teardown_directed() to free the actual number of
allocated MSI bits (msi_nr_irqs) instead of the maximum supported by the
platform (max_msi). This prevents freeing bits that were not actually
allocated, which would corrupt the shared IRQ bitmap.

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index 5e934ac990ac1..e9eda846cb2d2 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -349,7 +349,7 @@ static struct airq_struct zpci_airq = {
>  
>  static void zpci_msi_teardown_directed(struct zpci_dev *zdev)
>  {
> -	airq_iv_free(zpci_ibv[0], zdev->msi_first_bit, zdev->max_msi);
> +	airq_iv_free(zpci_ibv[0], zdev->msi_first_bit, zdev->msi_nr_irqs);

[Severity: High]
This is a pre-existing issue, but does this leak hardware vector bits on CPUs
other than 0?

It looks like this only clears the vector bits for zpci_ibv[0]. Since CPUs
greater than 0 have zpci_ibv[cpu] created without AIRQ_IV_ALLOC, their avail
mask is NULL, and airq_iv_free() returns immediately without clearing the bits
in zpci_ibv[cpu]->vector.

If a pending hardware interrupt bit is left set on a CPU greater than 0 after
device teardown, could reallocating that bit index to a new device trigger a
spurious interrupt on that CPU?

>  	zdev->msi_first_bit = -1U;
>  	zdev->msi_nr_irqs = 0;
>  }

[Severity: High]
This is a pre-existing issue, but is there a double-addition logic error in
zpci_msi_clear_airq()?

arch/s390/pci/pci_irq.c:zpci_msi_clear_airq() {
	...
	msi_index = zpci_decode_hwirq_msi_index(d->hwirq);
	bit = zdev->msi_first_bit + msi_index;

	if (irq_delivery == DIRECTED) {
		for_each_possible_cpu(cpu) {
			airq_iv_set_ptr(zpci_ibv[cpu], bit + i, 0);
			airq_iv_set_data(zpci_ibv[cpu], bit + i, 0);
...
}

During allocation, irq_domain_set_info() is passed hwirq + i, so d->hwirq
already incorporates the index i. zpci_decode_hwirq_msi_index(d->hwirq)
decodes this to desc->msi_index + i.

Since bit is calculated as zdev->msi_first_bit + msi_index (which already
includes i), clearing bit + i evaluates to base + 2*i.

Does this skip odd-indexed entries and write out of bounds when clearing,
leaving the intended MSI vector entries dangling?


[Severity: Medium]
This is a pre-existing issue, but is an RCU pointer assignment missing when
publishing zdev->aibv to the zpci_ibv array?

arch/s390/pci/pci_irq.c:__alloc_airq() {
	...
		/* Create adapter interrupt vector */
		zdev->aibv = airq_iv_create(msi_vecs,
					    AIRQ_IV_PTR | AIRQ_IV_DATA | AIRQ_IV_BITLOCK,
					    NULL);
	...
		/* Wire up shortcut pointer */
		zpci_ibv[*bit] = zdev->aibv;
}

The interrupt handler zpci_floating_irq_handler() correctly uses
rcu_dereference() to read zpci_ibv[si]. However, this plain assignment
publishes the pointer to readers without the required RCU compiler barriers.

Could compiler optimizations like LTO reorder the initialization of
zdev->aibv to occur after the pointer is published, allowing a concurrent
interrupt handler to read uninitialized structures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com?part=4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq()
  2026-08-19  8:50 ` [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq() Tobias Schumacher
@ 2026-08-19  9:14   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-19  9:14 UTC (permalink / raw)
  To: Tobias Schumacher
  Cc: Alexander Gordeev, Heiko Carstens, linux-s390,
	Christian Borntraeger, Vasily Gorbik

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The patch introduces dead code by adding NULL checks for impossible scenarios due to a misunderstanding of the MSI irqdomain lifecycle.

Pre-existing issues:
- [Critical] Double addition of the loop index `i` in `zpci_msi_clear_airq()` causes a heap out-of-bounds write during multi-MSI teardown.
- [High] Missing device teardown in `pci_base_init()` error path creates a Use-After-Free condition, which the added NULL checks silently evaluate.
--

--- Patch [5]: [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq() ---
commit a9e3adc4d04b20107c5f8c48da92688566d873e6
Author: Tobias Schumacher <ts@linux.ibm.com>
s390/pci: add NULL check in zpci_msi_clear_airq()

This commit adds null checks for the interrupt vector pointers in
zpci_msi_clear_airq() for both DIRECTED and FLOATING modes. It attempts
to prevent crashes during error cleanup and shutdown paths if the pointers
are already freed.

> In FLOATING mode, zdev->aibv can be NULL if:
> - zpci_msi_prepare() fails after __alloc_airq() but before setting
>   up the device's AIBV, and zpci_msi_domain_free() is called during
>   error cleanup
> - The device is being torn down and zpci_msi_teardown_floating()
>   has already released the AIBV

[Severity: Medium]
Is this description accurate?

If the .msi_prepare callback fails, it returns an error and the MSI core
halts allocation without invoking .alloc. Consequently, the .free callback
is never invoked for this device, so zpci_msi_clear_airq() wouldn't
execute on this error path.

During normal device teardown, the MSI core guarantees that .free is invoked
per-IRQ strictly before the domain is destroyed and .msi_teardown is invoked.
Since zpci_msi_teardown_floating() is the only function that sets zdev->aibv
to null, zdev->aibv should be valid whenever zpci_msi_clear_airq() executes.

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index e9eda846cb2d2..1515d8d7460ea 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -465,12 +465,16 @@ static void zpci_msi_clear_airq(struct irq_data *d, int i)
>  
>  	if (irq_delivery == DIRECTED) {
>  		for_each_possible_cpu(cpu) {
> -			airq_iv_set_ptr(zpci_ibv[cpu], bit + i, 0);
> -			airq_iv_set_data(zpci_ibv[cpu], bit + i, 0);
> +			if (zpci_ibv[cpu]) {

[Severity: High]
This is a pre-existing issue, but does checking zpci_ibv[cpu] here lead to a
use-after-free read?

In pci_base_init(), if zpci_fw_sysfs_init() fails, it jumps to out_find and
calls zpci_irq_exit(). This frees the zpci_ibv array via kfree(), but leaves
active PCI devices registered.

When drivers later bind to these devices and tear them down,
zpci_msi_clear_airq() will execute. Checking the freed zpci_ibv array pointer
will likely evaluate to true, causing a use-after-free read and subsequent
write when it is dereferenced.

> +				airq_iv_set_ptr(zpci_ibv[cpu], bit + i, 0);
> +				airq_iv_set_data(zpci_ibv[cpu], bit + i, 0);

[Severity: Critical]
This is a pre-existing issue, but does this code write out of bounds during a
multi-MSI teardown?

Earlier in zpci_msi_clear_airq(), the local variable bit is computed as
zdev->msi_first_bit + msi_index. The hwirq of the irq_data d was already
initialized as base_hwirq + i during allocation, so msi_index already
includes the offset i.

Passing bit + i to the airq_iv_set_ptr() and airq_iv_set_data() macros means
the index evaluated is zdev->msi_first_bit + desc->msi_index + 2 * i. For
any index greater than zero, this skips the intended target and writes 0 out
of bounds. This same calculation impacts both the zpci_ibv and zdev->aibv
array writes in this function.

> +			}
>  		}
>  	} else {
> -		airq_iv_set_ptr(zdev->aibv, bit + i, 0);
> -		airq_iv_set_data(zdev->aibv, bit + i, 0);
> +		if (zdev->aibv) {
> +			airq_iv_set_ptr(zdev->aibv, bit + i, 0);
> +			airq_iv_set_data(zdev->aibv, bit + i, 0);
> +		}
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com?part=5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/7] s390/pci: fix use-after-free race in zpci floating interrupt cleanup
  2026-08-19  8:50 ` [PATCH 2/7] s390/pci: fix use-after-free race in zpci floating interrupt cleanup Tobias Schumacher
@ 2026-08-19  9:20   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-08-19  9:20 UTC (permalink / raw)
  To: Tobias Schumacher
  Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Teardown order creates a race condition that can overwrite a concurrently allocated IRQ vector pointer with NULL, leading to permanently lost interrupts.
- [Medium] Missing RCU publication mechanism (`rcu_assign_pointer`) for `zpci_ibv` writes creates a potential data race where the IRQ handler could read uninitialized adapter interrupt vectors.
--

--- Patch [2]: [PATCH 2/7] s390/pci: fix use-after-free race in zpci floating interrupt cleanup ---
commit 2b99a8a1009855b33744de14f0b8b1c8afadae39
Author: Tobias Schumacher <ts@linux.ibm.com>

s390/pci: fix use-after-free race in zpci floating interrupt cleanup

The interrupt handler reads zpci_ibv[si] without synchronization while
concurrent teardown can release this memory, creating a race. The fix
protects the array access with RCU synchronization.

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index c9520a16ca753..94b03d16006ba 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -278,8 +278,14 @@ static void zpci_floating_irq_handler(struct airq_struct *airq,
>  			continue;
>  		}
>  
> +		rcu_read_lock();
> +
>  		/* Scan the adapter interrupt vector for this device. */
> -		aibv = zpci_ibv[si];
> +		aibv = rcu_dereference(zpci_ibv[si]);

[Severity: Medium]
I see that rcu_dereference() is used here to read from the array.

Does the write side in __alloc_airq() also need to be updated to use
rcu_assign_pointer()?

arch/s390/pci/pci_irq.c:__alloc_airq() {
    ...
		/* Wire up shortcut pointer */
		zpci_ibv[*bit] = zdev->aibv;
    ...
}

Without rcu_assign_pointer(), the required memory release barrier is missing
when the interrupt vector is published to the RCU-protected array.

> +		if (!aibv) {
> +			rcu_read_unlock();
> +			continue;
> +		}

[ ... ]

> @@ -346,9 +353,12 @@ static void zpci_msi_teardown_directed(struct zpci_dev *zdev)
>  
>  static void zpci_msi_teardown_floating(struct zpci_dev *zdev)
>  {
> +	airq_iv_free_bit(zpci_sbv, zdev->aisb);
> +	zpci_ibv[zdev->aisb] = NULL;
> +	synchronize_rcu();

[Severity: High]
Does freeing the bit before clearing the array pointer create a race condition?

If Thread A (teardown) frees the bit, Thread B (initializing a new device)
could allocate the exact same bit from zpci_sbv. Thread B would then allocate
its vector and assign it to zpci_ibv[bit]. When Thread A continues, it clears
zpci_ibv[zdev->aisb], overwriting Thread B's valid pointer with NULL.

Could this overwrite lead to permanently lost interrupts and a memory leak for
the new device?

> +
>  	airq_iv_release(zdev->aibv);
>  	zdev->aibv = NULL;
> -	airq_iv_free_bit(zpci_sbv, zdev->aisb);
>  	zdev->aisb = -1UL;
>  	zdev->msi_first_bit = -1U;
>  	zdev->msi_nr_irqs = 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com?part=2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration
  2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
                   ` (6 preceding siblings ...)
  2026-08-19  8:51 ` [PATCH 7/7] s390/pci: move MSI affinity flag initialization to boot time Tobias Schumacher
@ 2026-08-19  9:24 ` Niklas Schnelle
  7 siblings, 0 replies; 16+ messages in thread
From: Niklas Schnelle @ 2026-08-19  9:24 UTC (permalink / raw)
  To: Tobias Schumacher, Gerd Bayer, Julian Ruess, Farhan Ali,
	Christian Borntraeger, Halil Pasic
  Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
	linux-s390, linux-kernel, stable

On Wed, 2026-08-19 at 10:50 +0200, Tobias Schumacher wrote:
> Commit f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ
> domain API") introduced several bugs in error handling and cleanup
> paths. This series fixes these issues:
> 
> 1. Null pointer dereference and double-free in MSI cleanup
> 2. Use-after-free race in floating interrupt cleanup
> 3. Resource leak in MSI setup error path
> 4. Wrong number of IRQs freed in directed-mode teardown
> 5. Missing NULL checks in zpci_msi_clear_airq()
> 6. Resource leak in zpci_directed_irq_init() error path
> 7. Inefficient MSI affinity flag initialization
> 
> Patches 1-6 fix critical bugs that can cause crashes, memory
> corruption, or resource exhaustion. Patch 7 is a cleanup that moves
> flag initialization to a more appropriate location.
> 
> Tobias Schumacher (7):
>   s390/pci: fix null pointer dereference and double-free in zpci MSI cleanup
>   s390/pci: fix use-after-free race in zpci floating interrupt cleanup
>   s390/pci: fix resource leak in zpci MSI setup
>   s390/pci: fix MSI directed-mode teardown IRQ bit count
>   s390/pci: add NULL check in zpci_msi_clear_airq()
>   s390/pci: add error cleanup in zpci_directed_irq_init
>   s390/pci: move MSI affinity flag initialization to boot time

Nit: On all patches, s390 uses a capital letter after the "s390/xyz:"
prefix. See here:
https://github.ibm.com/linuxonz/development-wiki/blob/master/wiki/linux-development-guide/kernel.md#Patch-subject

Thanks,
Niklas

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-08-19  9:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19  8:50 [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Tobias Schumacher
2026-08-19  8:50 ` [PATCH 1/7] s390/pci: fix double-free in zpci MSI cleanup Tobias Schumacher
2026-08-19  9:03   ` sashiko-bot
2026-08-19  8:50 ` [PATCH 2/7] s390/pci: fix use-after-free race in zpci floating interrupt cleanup Tobias Schumacher
2026-08-19  9:20   ` sashiko-bot
2026-08-19  8:50 ` [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup Tobias Schumacher
2026-08-19  9:05   ` sashiko-bot
2026-08-19  8:50 ` [PATCH 4/7] s390/pci: fix MSI directed-mode teardown IRQ bit count Tobias Schumacher
2026-08-19  9:07   ` sashiko-bot
2026-08-19  8:50 ` [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq() Tobias Schumacher
2026-08-19  9:14   ` sashiko-bot
2026-08-19  8:50 ` [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init Tobias Schumacher
2026-08-19  9:02   ` sashiko-bot
2026-08-19  8:51 ` [PATCH 7/7] s390/pci: move MSI affinity flag initialization to boot time Tobias Schumacher
2026-08-19  9:06   ` sashiko-bot
2026-08-19  9:24 ` [PATCH 0/7] s390/pci: Fix multiple bugs in IRQ domain migration Niklas Schnelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.