[PATCH v3 0/8] s390/pci: Fix bugs in IRQ domain migration and resource cleanup

[PATCH v3 0/8] s390/pci: Fix bugs in IRQ domain migration and resource cleanup

[Tobias Schumacher]

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. Double-free and NULL dereference in the parent MSI domain cleanup
2. Leak of a zpci_sbv summary bit when AIBV creation fails
3. Directed-mode teardown freeing zdev->max_msi bits instead of the
   zdev->msi_nr_irqs bits that were allocated
4. Use-after-free race between floating IRQ delivery and teardown

Patch 5 is unrelated to the migration. zpci_directed_irq_init() has
leaked its allocations on the -ENOMEM paths since it was added in
e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts").

Patch 6 is a cleanup that removes an unnecessary update of
zpci_msi_parent_ops from the per-bus domain creation path.

Patch 7 is a cleanup that drops the unused index argument of
zpci_msi_clear_airq(). The doubled index it removes never selected a
wrong entry, so it is not a fix.

Patch 8 is a cleanup that unregisters the adapter interrupt first in
zpci_irq_exit().

Patches 1 to 5 carry Cc: stable. Patches 6 to 8 do not; none of them
changes behaviour.

Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
Changes in v3:
- Reverted ordering change in zpci_msi_teardown_floating() (patch 4)
- Patch 5: move zpci_set_irq_ctrl() after allocations in
  zpci_directed_irq_init()
- Patch 5: add Cc: stable
- Added patch 8 which changes teardown ordering in zpci_irq_exit()
- Link to v2: https://lore.kernel.org/r/20261005-s390_irq_domain_fixes-v2-0-d45b824874c0@linux.ibm.com

Changes in v2:
- Capitalize the word after the "s390/pci:" prefix on all subjects
- Replace the "add NULL check in zpci_msi_clear_airq()" patch with a
  cleanup that drops the unused index argument, and move it to the end
  of the series
- Patch 4: clear zpci_ibv[] before the grace period, free the summary
  bit after it, publish with rcu_assign_pointer()
- Patch 5: correct the Fixes: tag, drop Cc: stable
- Link to v1: https://lore.kernel.org/r/20260819-s390_irq_domain_fixes-v1-0-826ff27b6e97@linux.ibm.com

---
Tobias Schumacher (8):
      s390/pci: Fix double-free and NULL deref in zpci MSI domain cleanup
      s390/pci: Fix resource leak in zpci MSI setup
      s390/pci: Fix MSI directed-mode teardown IRQ bit count
      s390/pci: Fix use-after-free race in zpci floating interrupt cleanup
      s390/pci: Add error cleanup in zpci_directed_irq_init()
      s390/pci: Set MSI_FLAG_NO_AFFINITY at IRQ init time
      s390/pci: Drop the unused index argument of zpci_msi_clear_airq()
      s390/pci: Unregister the adapter interrupt first in zpci_irq_exit()

 arch/s390/pci/pci_irq.c | 109 +++++++++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 42 deletions(-)
---
base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
change-id: 20260818-s390_irq_domain_fixes-ad74b3134c51

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

[PATCH v3 1/8] s390/pci: Fix double-free and NULL deref in zpci MSI domain cleanup

[Tobias Schumacher]

zpci_remove_parent_msi_domain() dereferences zbus->msi_parent_domain
unconditionally and does not clear it afterwards.

zpci_bus_create_pci_bus() removes the domain when pci_create_root_bus()
fails, then zpci_bus_release() removes it again on the last kref_put(),
reading ->fwnode from the freed irq_domain and freeing it twice.

If zpci_alloc_domain() or zpci_create_parent_msi_domain() fails, no domain
is created at all; zbus is kzalloc'd, so the same release path dereferences
NULL.

Return early when there is no domain, and clear the pointer after removing
one.

Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
Cc: stable@vger.kernel.org
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
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

[PATCH v3 2/8] s390/pci: Fix resource leak in zpci MSI setup

[Tobias Schumacher]

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, 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
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
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 c9520a16ca75..134f8f4a5cfa 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -313,8 +313,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

[PATCH v3 3/8] s390/pci: Fix MSI directed-mode teardown IRQ bit count

[Tobias Schumacher]

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
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
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 134f8f4a5cfa..d5763c5feb09 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -342,7 +342,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

[PATCH v3 4/8] s390/pci: Fix use-after-free race in zpci floating interrupt cleanup

[Tobias Schumacher]

zpci_clear_irq() stops the adapter from raising new interrupts for the
function, but a zpci_floating_irq_handler() already running on another CPU
can still be scanning zdev->aibv when zpci_msi_teardown_floating() releases
it.

Clear the zpci_ibv[] entry so no further handler picks the vector up, then
wait for a grace period before releasing it. The handler runs inside the
rcu_read_lock() section that do_airq_interrupt() holds across
airq->handler(), so synchronize_rcu() drains any handler still in flight.
Release the vector and free the summary bit only after the grace period, so
neither can be reused while a reader still holds the old pointer.

zpci_ibv served both delivery modes, indexed by function under
FLOATING and by cpu under DIRECTED. Only the floating vectors are
published to and torn down under the interrupt handler, so split the
directed vectors out into zpci_dibv and annotate zpci_ibv __rcu, which
lets sparse check the accessors above.

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 | 58 +++++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index d5763c5feb09..c5fabad38139 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -22,12 +22,11 @@ static enum {FLOATING, DIRECTED} irq_delivery;
  */
 static struct airq_iv *zpci_sbv;
 
-/*
- * interrupt bit vectors
- * FLOATING - interrupt bit vector per function
- * DIRECTED - interrupt bit vector per cpu
- */
-static struct airq_iv **zpci_ibv;
+/* FLOATING - interrupt bit vector per function */
+static struct airq_iv __rcu **zpci_ibv;
+
+/* DIRECTED - interrupt bit vector per cpu */
+static struct airq_iv **zpci_dibv;
 
 /* Modify PCI: Register floating adapter interruptions */
 static int zpci_set_airq(struct zpci_dev *zdev)
@@ -169,7 +168,7 @@ static struct irq_chip zpci_irq_chip = {
 
 static void zpci_handle_cpu_local_irq(bool rescan)
 {
-	struct airq_iv *dibv = zpci_ibv[smp_processor_id()];
+	struct airq_iv *dibv = zpci_dibv[smp_processor_id()];
 	union zpci_sic_iib iib = {{0}};
 	struct irq_domain *msi_domain;
 	irq_hw_number_t hwirq;
@@ -279,7 +278,9 @@ static void zpci_floating_irq_handler(struct airq_struct *airq,
 		}
 
 		/* Scan the adapter interrupt vector for this device. */
-		aibv = zpci_ibv[si];
+		aibv = rcu_dereference(zpci_ibv[si]);
+		if (!aibv)
+			continue;
 		for (ai = 0;;) {
 			ai = airq_iv_scan(aibv, ai, airq_iv_end(aibv));
 			if (ai == -1UL)
@@ -299,7 +300,7 @@ static int __alloc_airq(struct zpci_dev *zdev, int msi_vecs,
 {
 	if (irq_delivery == DIRECTED) {
 		/* Allocate cpu vector bits */
-		*bit = airq_iv_alloc(zpci_ibv[0], msi_vecs);
+		*bit = airq_iv_alloc(zpci_dibv[0], msi_vecs);
 		if (*bit == -1UL)
 			return -EIO;
 	} else {
@@ -320,7 +321,7 @@ static int __alloc_airq(struct zpci_dev *zdev, int msi_vecs,
 		}
 
 		/* Wire up shortcut pointer */
-		zpci_ibv[*bit] = zdev->aibv;
+		rcu_assign_pointer(zpci_ibv[*bit], zdev->aibv);
 		/* Each function has its own interrupt vector */
 		*bit = 0;
 	}
@@ -342,13 +343,16 @@ 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->msi_nr_irqs);
+	airq_iv_free(zpci_dibv[0], zdev->msi_first_bit, zdev->msi_nr_irqs);
 	zdev->msi_first_bit = -1U;
 	zdev->msi_nr_irqs = 0;
 }
 
 static void zpci_msi_teardown_floating(struct zpci_dev *zdev)
 {
+	rcu_assign_pointer(zpci_ibv[zdev->aisb], NULL);
+	synchronize_rcu();
+
 	airq_iv_release(zdev->aibv);
 	zdev->aibv = NULL;
 	airq_iv_free_bit(zpci_sbv, zdev->aisb);
@@ -428,9 +432,9 @@ static int zpci_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 
 		if (irq_delivery == DIRECTED) {
 			for_each_possible_cpu(cpu) {
-				airq_iv_set_ptr(zpci_ibv[cpu], bit + i,
+				airq_iv_set_ptr(zpci_dibv[cpu], bit + i,
 						(unsigned long)zbus->msi_parent_domain);
-				airq_iv_set_data(zpci_ibv[cpu], bit + i, hwirq + i);
+				airq_iv_set_data(zpci_dibv[cpu], bit + i, hwirq + i);
 			}
 		} else {
 			airq_iv_set_ptr(zdev->aibv, bit + i,
@@ -455,8 +459,8 @@ 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);
+			airq_iv_set_ptr(zpci_dibv[cpu], bit + i, 0);
+			airq_iv_set_data(zpci_dibv[cpu], bit + i, 0);
 		}
 	} else {
 		airq_iv_set_ptr(zdev->aibv, bit + i, 0);
@@ -550,7 +554,7 @@ static void __init cpu_enable_directed_irq(void *unused)
 	union zpci_sic_iib iib = {{0}};
 	union zpci_sic_iib ziib = {{0}};
 
-	iib.cdiib.dibv_addr = virt_to_phys(zpci_ibv[smp_processor_id()]->vector);
+	iib.cdiib.dibv_addr = virt_to_phys(zpci_dibv[smp_processor_id()]->vector);
 
 	zpci_set_irq_ctrl(SIC_IRQ_MODE_SET_CPU, 0, &iib);
 	zpci_set_irq_ctrl(SIC_IRQ_MODE_D_SINGLE, PCI_ISC, &ziib);
@@ -570,8 +574,8 @@ static int __init zpci_directed_irq_init(void)
 	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)
+	zpci_dibv = kzalloc_objs(*zpci_dibv, num_possible_cpus());
+	if (!zpci_dibv)
 		return -ENOMEM;
 
 	for_each_possible_cpu(cpu) {
@@ -579,12 +583,12 @@ static int __init zpci_directed_irq_init(void)
 		 * Per CPU IRQ vectors look the same but bit-allocation
 		 * is only done on the first vector.
 		 */
-		zpci_ibv[cpu] = airq_iv_create(cache_line_size() * BITS_PER_BYTE,
-					       AIRQ_IV_PTR |
-					       AIRQ_IV_DATA |
-					       AIRQ_IV_CACHELINE |
-					       (!cpu ? AIRQ_IV_ALLOC : 0), NULL);
-		if (!zpci_ibv[cpu])
+		zpci_dibv[cpu] = airq_iv_create(cache_line_size() * BITS_PER_BYTE,
+						AIRQ_IV_PTR |
+						AIRQ_IV_DATA |
+						AIRQ_IV_CACHELINE |
+						(!cpu ? AIRQ_IV_ALLOC : 0), NULL);
+		if (!zpci_dibv[cpu])
 			return -ENOMEM;
 	}
 	on_each_cpu(cpu_enable_directed_irq, NULL, 1);
@@ -660,10 +664,12 @@ void __init zpci_irq_exit(void)
 
 	if (irq_delivery == DIRECTED) {
 		for_each_possible_cpu(cpu) {
-			airq_iv_release(zpci_ibv[cpu]);
+			airq_iv_release(zpci_dibv[cpu]);
 		}
+		kfree(zpci_dibv);
+	} else {
+		kfree(zpci_ibv);
 	}
-	kfree(zpci_ibv);
 	if (zpci_sbv)
 		airq_iv_release(zpci_sbv);
 	unregister_adapter_interrupt(&zpci_airq);

-- 
2.53.0

[PATCH v3 5/8] s390/pci: Add error cleanup in zpci_directed_irq_init()

[Tobias Schumacher]

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.

The error path frees zpci_sbv, but SIC_IRQ_MODE_DIRECT has already
programmed its address into the hardware. Set the directed interrupt
mode only after all allocations have succeeded, so a failed init leaves
no freed memory registered with the hardware.

Fixes: e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts")
Cc: stable@vger.kernel.org
Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
---
 arch/s390/pci/pci_irq.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index c5fabad38139..bcf11585df8d 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -569,14 +569,9 @@ static int __init zpci_directed_irq_init(void)
 	if (!zpci_sbv)
 		return -ENOMEM;
 
-	iib.diib.isc = PCI_ISC;
-	iib.diib.nr_cpus = num_possible_cpus();
-	iib.diib.disb_addr = virt_to_phys(zpci_sbv->vector);
-	zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib);
-
 	zpci_dibv = kzalloc_objs(*zpci_dibv, num_possible_cpus());
 	if (!zpci_dibv)
-		return -ENOMEM;
+		goto out_free_sbv;
 
 	for_each_possible_cpu(cpu) {
 		/*
@@ -589,13 +584,30 @@ static int __init zpci_directed_irq_init(void)
 						AIRQ_IV_CACHELINE |
 						(!cpu ? AIRQ_IV_ALLOC : 0), NULL);
 		if (!zpci_dibv[cpu])
-			return -ENOMEM;
+			goto out_free_dibv;
 	}
+
+	iib.diib.isc = PCI_ISC;
+	iib.diib.nr_cpus = num_possible_cpus();
+	iib.diib.disb_addr = virt_to_phys(zpci_sbv->vector);
+	zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib);
 	on_each_cpu(cpu_enable_directed_irq, NULL, 1);
 
 	zpci_irq_chip.irq_set_affinity = zpci_set_irq_affinity;
 
 	return 0;
+
+out_free_dibv:
+	for_each_possible_cpu(cpu) {
+		if (zpci_dibv[cpu])
+			airq_iv_release(zpci_dibv[cpu]);
+	}
+	kfree(zpci_dibv);
+	zpci_dibv = 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

[PATCH v3 6/8] s390/pci: Set MSI_FLAG_NO_AFFINITY at IRQ init time

[Tobias Schumacher]

MSI_FLAG_NO_AFFINITY is added to zpci_msi_parent_ops.required_flags from
zpci_create_parent_msi_domain(), which runs for every new PCI bus,
including buses created at runtime from a hotplug availability event.

That is a non-atomic read-modify-write on a field which
msi_lib_init_dev_msi_info() reads without a common lock while setting up
MSI for a device on an already existing bus:

      required_flags = pops->required_flags;

The stored value is always the same, so no caller observes a change, but
the race need not exist: irq_delivery is decided once in zpci_irq_init()
and never changes afterwards.

Set the flag there instead, before any parent domain exists.

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
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 bcf11585df8d..b890b55be873 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -523,9 +523,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);
@@ -636,6 +633,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

[PATCH v3 7/8] s390/pci: Drop the unused index argument of zpci_msi_clear_airq()

[Tobias Schumacher]

zpci_msi_domain_free() passes its loop index to zpci_msi_clear_airq(),
which adds it to an offset that already accounts for it.
zpci_msi_domain_alloc() stores hwirq + i for each vector, so
zpci_decode_hwirq_msi_index() hands back msi_index + i and bit is
already zdev->msi_first_bit + msi_index + i.

The doubled index never selected a wrong entry. An irq domain's free()
callback is only ever invoked from irq_domain_free_irqs_hierarchy(),
which walks the range itself and passes a count of one. So, the loop in
zpci_msi_domain_free() runs once with an index of zero.

Drop the parameter and the addition.

No functional change.

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

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index b890b55be873..4b8ebb987580 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -446,7 +446,7 @@ static int zpci_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 	return 0;
 }
 
-static void zpci_msi_clear_airq(struct irq_data *d, int i)
+static void zpci_msi_clear_airq(struct irq_data *d)
 {
 	struct msi_desc *desc = irq_data_get_msi_desc(d);
 	struct zpci_dev *zdev = to_zpci_dev(desc->dev);
@@ -459,12 +459,12 @@ 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_dibv[cpu], bit + i, 0);
-			airq_iv_set_data(zpci_dibv[cpu], bit + i, 0);
+			airq_iv_set_ptr(zpci_dibv[cpu], bit, 0);
+			airq_iv_set_data(zpci_dibv[cpu], bit, 0);
 		}
 	} else {
-		airq_iv_set_ptr(zdev->aibv, bit + i, 0);
-		airq_iv_set_data(zdev->aibv, bit + i, 0);
+		airq_iv_set_ptr(zdev->aibv, bit, 0);
+		airq_iv_set_data(zdev->aibv, bit, 0);
 	}
 }
 
@@ -476,7 +476,7 @@ static void zpci_msi_domain_free(struct irq_domain *domain, unsigned int virq,
 
 	for (i = 0; i < nr_irqs; i++) {
 		d = irq_domain_get_irq_data(domain, virq + i);
-		zpci_msi_clear_airq(d, i);
+		zpci_msi_clear_airq(d);
 		irq_domain_reset_irq_data(d);
 	}
 }

-- 
2.53.0

[PATCH v3 8/8] s390/pci: Unregister the adapter interrupt first in zpci_irq_exit()

[Tobias Schumacher]

zpci_irq_exit() frees the interrupt bit vectors before unregistering the
adapter interrupt whose handler uses them. This is only called from the
pci_base_init() error path, where no handler is expected to run, but
unregister first anyway so the vectors are never freed under a live
handler.

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 4b8ebb987580..16ae80ed2d13 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -674,6 +674,7 @@ void __init zpci_irq_exit(void)
 {
 	unsigned int cpu;
 
+	unregister_adapter_interrupt(&zpci_airq);
 	if (irq_delivery == DIRECTED) {
 		for_each_possible_cpu(cpu) {
 			airq_iv_release(zpci_dibv[cpu]);
@@ -684,5 +685,4 @@ void __init zpci_irq_exit(void)
 	}
 	if (zpci_sbv)
 		airq_iv_release(zpci_sbv);
-	unregister_adapter_interrupt(&zpci_airq);
 }

-- 
2.53.0

Re: [PATCH v3 7/8] s390/pci: Drop the unused index argument of zpci_msi_clear_airq()

[sashiko-bot]

> zpci_msi_domain_free() passes its loop index to zpci_msi_clear_airq(),
> which adds it to an offset that already accounts for it.
> zpci_msi_domain_alloc() stores hwirq + i for each vector, so
> zpci_decode_hwirq_msi_index() hands back msi_index + i and bit is
> already zdev->msi_first_bit + msi_index + i.
> 
> The doubled index never selected a wrong entry. An irq domain's free()
> callback is only ever invoked from irq_domain_free_irqs_hierarchy(),
> which walks the range itself and passes a count of one. So, the loop in
> zpci_msi_domain_free() runs once with an index of zero.
> 
> Drop the parameter and the addition.
> 
> No functional change.
> 
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 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/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=7

Re: [PATCH v3 5/8] s390/pci: Add error cleanup in zpci_directed_irq_init()

[sashiko-bot]

> 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.
> 
> The error path frees zpci_sbv, but SIC_IRQ_MODE_DIRECT has already
> programmed its address into the hardware. Set the directed interrupt
> mode only after all allocations have succeeded, so a failed init leaves
> no freed memory registered with the hardware.
> 
> Fixes: e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts")
> Cc: stable@vger.kernel.org
> 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/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=5

Re: [PATCH v3 6/8] s390/pci: Set MSI_FLAG_NO_AFFINITY at IRQ init time

[sashiko-bot]

> MSI_FLAG_NO_AFFINITY is added to zpci_msi_parent_ops.required_flags from
> zpci_create_parent_msi_domain(), which runs for every new PCI bus,
> including buses created at runtime from a hotplug availability event.
> 
> That is a non-atomic read-modify-write on a field which
> msi_lib_init_dev_msi_info() reads without a common lock while setting up
> MSI for a device on an already existing bus:
> 
>       required_flags = pops->required_flags;
> 
> The stored value is always the same, so no caller observes a change, but
> the race need not exist: irq_delivery is decided once in zpci_irq_init()
> and never changes afterwards.
> 
> Set the flag there instead, before any parent domain exists.
> 
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 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/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=6

Re: [PATCH v3 1/8] s390/pci: Fix double-free and NULL deref in zpci MSI domain cleanup

[sashiko-bot]

> zpci_remove_parent_msi_domain() dereferences zbus->msi_parent_domain
> unconditionally and does not clear it afterwards.
> 
> zpci_bus_create_pci_bus() removes the domain when pci_create_root_bus()
> fails, then zpci_bus_release() removes it again on the last kref_put(),
> reading ->fwnode from the freed irq_domain and freeing it twice.
> 
> If zpci_alloc_domain() or zpci_create_parent_msi_domain() fails, no domain
> is created at all; zbus is kzalloc'd, so the same release path dereferences
> NULL.
> 
> Return early when there is no domain, and clear the pointer after removing
> one.
> 
> Fixes: f770950a4709 ("s390/pci: Migrate s390 IRQ logic to IRQ domain API")
> Cc: stable@vger.kernel.org
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 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/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=1

Re: [PATCH v3 2/8] s390/pci: Fix resource leak in zpci MSI setup

[sashiko-bot]

> 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, 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
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 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/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=2

Re: [PATCH v3 8/8] s390/pci: Unregister the adapter interrupt first in zpci_irq_exit()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/pci: UAF memory corruption in MSI setup due to missing device teardown
--

commit 4f6e192e648a7af5752afbd15f3be9f0172b9594
Author: Tobias Schumacher <ts@linux.ibm.com>

s390/pci: Unregister the adapter interrupt first in zpci_irq_exit()

This commit modifies zpci_irq_exit() to unregister the adapter interrupt before
freeing the interrupt bit vectors. This aims to prevent vectors from being
freed under a live handler during the pci_base_init() error path.

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index 4b8ebb987580b..16ae80ed2d139 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -674,6 +674,7 @@ void __init zpci_irq_exit(void)
>  {
>  	unsigned int cpu;
>  
> +	unregister_adapter_interrupt(&zpci_airq);
>  	if (irq_delivery == DIRECTED) {
>  		for_each_possible_cpu(cpu) {
>  			airq_iv_release(zpci_dibv[cpu]);
> @@ -684,5 +685,4 @@ void __init zpci_irq_exit(void)
>  	}
>  	if (zpci_sbv)
>  		airq_iv_release(zpci_sbv);

[Severity: High]
Does this change fully resolve the use-after-free risk in the pci_base_init()
error path?

While moving unregister_adapter_interrupt() prevents the interrupt handler
from accessing the vectors while they are being freed, it appears the devices
registered earlier by zpci_scan_devices() are never unregistered when
pci_base_init() fails during zpci_fw_sysfs_init().

If a driver later binds to one of these still-registered devices and calls
pci_alloc_irq_vectors(), the PCI MSI setup accesses the freed zpci_sbv
and zpci_ibv structures:

arch/s390/pci/pci_irq.c:__alloc_airq() {
    ...
    /* Allocate adapter summary indicator bit */
    *bit = airq_iv_alloc_bit(zpci_sbv);
    ...
    /* Wire up shortcut pointer */
    rcu_assign_pointer(zpci_ibv[*bit], zdev->aibv);
    ...
}

Could this lead to memory corruption when a driver attempts to set up MSI on
a device that was not properly torn down?

> -	unregister_adapter_interrupt(&zpci_airq);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=8

Re: [PATCH v3 4/8] s390/pci: Fix use-after-free race in zpci floating interrupt cleanup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/pci: pending directed MSIs cause spurious interrupts on reallocation
--

commit 125383b5f8790de7f1cbeff916db7b71f9226ee6
Author: Tobias Schumacher <ts@linux.ibm.com>

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

This commit splits directed vectors from floating vectors into a separate
zpci_dibv array and adds RCU synchronization to zpci_msi_teardown_floating()
to prevent use-after-free issues. It also annotates zpci_ibv with __rcu to
ensure safe access to floating vectors during teardown.

> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index d5763c5feb097..c5fabad381395 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
[ ... ]
> @@ -342,13 +343,16 @@ 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->msi_nr_irqs);
> +	airq_iv_free(zpci_dibv[0], zdev->msi_first_bit, zdev->msi_nr_irqs);

[Severity: High]
Does this code leave pending DMA bits on remote CPUs intact?

Only CPU 0's vector is passed to airq_iv_free() here, which clears the
physical vector bits on CPU 0 but leaves them untouched on remote CPUs.

If a hardware adapter interrupt was pending on a remote CPU in
zpci_dibv[N]->vector, the bit remains set. Furthermore, there is no
synchronization (such as synchronize_rcu() or draining) to ensure remote
CPUs have finished processing the old bit before it is freed.

When a new device allocates the same MSI bit, it will overwrite the ptr and
data fields across all CPUs without memory barriers. If a remote CPU then
processes its delayed interrupt, it will read the newly populated ptr and
data, resulting in a spurious interrupt delivered to the wrong device or a
potential kernel panic from an inconsistent domain/hwirq pair.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=4

Re: [PATCH v3 3/8] s390/pci: Fix MSI directed-mode teardown IRQ bit count

[sashiko-bot]

> 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
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 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/20261009-s390_irq_domain_fixes-v3-0-ced19028da3c@linux.ibm.com?part=3

Re: [PATCH v3 8/8] s390/pci: Unregister the adapter interrupt first in zpci_irq_exit()

[Tobias Schumacher]

On Fri Oct 9, 2026 at 8:29 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] s390/pci: UAF memory corruption in MSI setup due to missing device teardown
> --
>
> commit 4f6e192e648a7af5752afbd15f3be9f0172b9594
> Author: Tobias Schumacher <ts@linux.ibm.com>
>
> s390/pci: Unregister the adapter interrupt first in zpci_irq_exit()
>
> This commit modifies zpci_irq_exit() to unregister the adapter interrupt before
> freeing the interrupt bit vectors. This aims to prevent vectors from being
> freed under a live handler during the pci_base_init() error path.
>
>> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
>> index 4b8ebb987580b..16ae80ed2d139 100644
>> --- a/arch/s390/pci/pci_irq.c
>> +++ b/arch/s390/pci/pci_irq.c
>> @@ -674,6 +674,7 @@ void __init zpci_irq_exit(void)
>>  {
>>  	unsigned int cpu;
>>
>> +	unregister_adapter_interrupt(&zpci_airq);
>>  	if (irq_delivery == DIRECTED) {
>>  		for_each_possible_cpu(cpu) {
>>  			airq_iv_release(zpci_dibv[cpu]);
>> @@ -684,5 +685,4 @@ void __init zpci_irq_exit(void)
>>  	}
>>  	if (zpci_sbv)
>>  		airq_iv_release(zpci_sbv);
>
> [Severity: High]
> Does this change fully resolve the use-after-free risk in the pci_base_init()
> error path?
>
> While moving unregister_adapter_interrupt() prevents the interrupt handler
> from accessing the vectors while they are being freed, it appears the devices
> registered earlier by zpci_scan_devices() are never unregistered when
> pci_base_init() fails during zpci_fw_sysfs_init().
>
> If a driver later binds to one of these still-registered devices and calls
> pci_alloc_irq_vectors(), the PCI MSI setup accesses the freed zpci_sbv
> and zpci_ibv structures:
>
> arch/s390/pci/pci_irq.c:__alloc_airq() {
>     ...
>     /* Allocate adapter summary indicator bit */
>     *bit = airq_iv_alloc_bit(zpci_sbv);
>     ...
>     /* Wire up shortcut pointer */
>     rcu_assign_pointer(zpci_ibv[*bit], zdev->aibv);
>     ...
> }
>
> Could this lead to memory corruption when a driver attempts to set up MSI on
> a device that was not properly torn down?

Right, this issue exists but is not addressed by this patch. This series
fixes and cleans the s390 PCI IRQ code, so I'll fix that in a separate patch
outside this series.

Thanks
Tobias

Re: [PATCH v3 4/8] s390/pci: Fix use-after-free race in zpci floating interrupt cleanup

[Tobias Schumacher]

On Fri Oct 9, 2026 at 8:32 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] s390/pci: pending directed MSIs cause spurious interrupts on reallocation
> --
>
> commit 125383b5f8790de7f1cbeff916db7b71f9226ee6
> Author: Tobias Schumacher <ts@linux.ibm.com>
>
> s390/pci: Fix use-after-free race in zpci floating interrupt cleanup
>
> This commit splits directed vectors from floating vectors into a separate
> zpci_dibv array and adds RCU synchronization to zpci_msi_teardown_floating()
> to prevent use-after-free issues. It also annotates zpci_ibv with __rcu to
> ensure safe access to floating vectors during teardown.
>
>> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
>> index d5763c5feb097..c5fabad381395 100644
>> --- a/arch/s390/pci/pci_irq.c
>> +++ b/arch/s390/pci/pci_irq.c
> [ ... ]
>> @@ -342,13 +343,16 @@ 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->msi_nr_irqs);
>> +	airq_iv_free(zpci_dibv[0], zdev->msi_first_bit, zdev->msi_nr_irqs);
>
> [Severity: High]
> Does this code leave pending DMA bits on remote CPUs intact?
>
> Only CPU 0's vector is passed to airq_iv_free() here, which clears the
> physical vector bits on CPU 0 but leaves them untouched on remote CPUs.
>
> If a hardware adapter interrupt was pending on a remote CPU in
> zpci_dibv[N]->vector, the bit remains set. Furthermore, there is no
> synchronization (such as synchronize_rcu() or draining) to ensure remote
> CPUs have finished processing the old bit before it is freed.
>
> When a new device allocates the same MSI bit, it will overwrite the ptr and
> data fields across all CPUs without memory barriers. If a remote CPU then
> processes its delayed interrupt, it will read the newly populated ptr and
> data, resulting in a spurious interrupt delivered to the wrong device or a
> potential kernel panic from an inconsistent domain/hwirq pair.

The only change this patch makes to zpci_msi_teardown_directed() is the
rename from zpci_ibv to zpci_dibv. Freeing the directed bits only in
CPU 0's vector goes back to the original commit introducing directed
interrupts. A stale bit on a remote CPU could in theory end up as a
spurious interrupt for a device that later gets the same bit, but until
the bit is reused, the cleared pointer makes the lookup fail harmlessly.

I'll have a look at clearing the bits on all CPUs separately from this
series.

Thanks
Tobias