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

[PATCH v2 0/7] 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.

Patches 1 to 4 carry Cc: stable. Patches 5 to 7 do not: patch 5 only
affects a boot-time allocation failure, and patches 6 and 7 change no
behaviour.

Signed-off-by: Tobias Schumacher <ts@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 (7):
      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()

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

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

[PATCH v2 1/7] 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
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 v2 2/7] 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
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 v2 3/7] 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
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 v2 4/7] 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.
Free the summary bit only after the grace period, so it cannot be handed to
another device while a reader still holds the old pointer.

zpci_ibv served both delivery modes, indexed by summary bit 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 | 60 +++++++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index d5763c5feb09..81a27bf756a3 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,16 +343,19 @@ 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_free_bit(zpci_sbv, zdev->aisb);
+
 	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;
@@ -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 v2 5/7] 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.

Fixes: e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts")
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 81a27bf756a3..682bceb6525e 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -576,7 +576,7 @@ static int __init zpci_directed_irq_init(void)
 
 	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 +589,25 @@ 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;
 	}
 	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 v2 6/7] 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.

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 682bceb6525e..661867a1ffad 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 v2 7/7] 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.

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 661867a1ffad..984bf9c96768 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

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

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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
> 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)

Thanks for the fix!

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

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

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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
> 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;

Thank you for fixing and sorry for not catching it in the review!

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

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

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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 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;
>  }

Good catch and fix looks good to me.

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

Re: [PATCH v2 5/7] s390/pci: Add error cleanup in zpci_directed_irq_init

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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: e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts")
> 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 81a27bf756a3..682bceb6525e 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -576,7 +576,7 @@ static int __init zpci_directed_irq_init(void)
>  
>  	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 +589,25 @@ 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;
>  	}
>  	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;

As Sashiko notes this doesn't undo the
zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib) and via the iib leaves
the zpci_sbv->vector that gets freed here set in hardware. While
Sashiko also notes this is an __init function and these allocations are
extremely unlikely to fail I think the rollback is still just nice
form. I think we could simply move the zpci_set_irq_ctrl() to after the
allocations but before the cpu_enable_directed_irq(). It also seems
more logical to set the interrupt mode only after memory has been set
up.

Thanks,
Niklas

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

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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.
> Free the summary bit only after the grace period, so it cannot be handed to
> another device while a reader still holds the old pointer.
> 
> zpci_ibv served both delivery modes, indexed by summary bit under

Nit: Maybe more precisely and matching the comment: "…, indexed by
function under FLOATING and …"?

> 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 | 60 +++++++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
> index d5763c5feb09..81a27bf756a3 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;
--- snip ---
>  
>  static void zpci_msi_teardown_floating(struct zpci_dev *zdev)
>  {
> +	rcu_assign_pointer(zpci_ibv[zdev->aisb], NULL);
> +	synchronize_rcu();
> +	airq_iv_free_bit(zpci_sbv, zdev->aisb);
> +
>  	airq_iv_release(zdev->aibv);
>  	zdev->aibv = NULL;
> -	airq_iv_free_bit(zpci_sbv, zdev->aisb);

Not sure why the aisb free moved to before the aibv release? The commit
message only explains why it is after the synchronize_rcu(). This way
it's also not in the opposite order of the allocation.

>  	zdev->aisb = -1UL;
>  	zdev->msi_first_bit = -1U;
>  	zdev->msi_nr_irqs = 0;
> @@ -428,9 +432,9 @@ static int zpci_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
>  
--- snip ---

Overall the change and the use of RCU looks good to me.

Thanks,
Niklas

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

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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.
> 
> Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>
> ---

Good catch and a gppd explanation. Great work.

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

Thanks,
Niklas

Re: [PATCH v2 6/7] s390/pci: Set MSI_FLAG_NO_AFFINITY at IRQ init time

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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.
> 
> 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 682bceb6525e..661867a1ffad 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;
>  

Looks good to me and I agree the race is benign as everyone races to
write the same value but this is certainly cleaner. Thanks.

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

Re: [PATCH v2 5/7] s390/pci: Add error cleanup in zpci_directed_irq_init

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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: e979ce7bced2 ("s390/pci: provide support for CPU directed interrupts")
> Signed-off-by: Tobias Schumacher <ts@linux.ibm.com>

Had my bot review my reviews and it noticed that this is missing Cc
stable despite having a Fixes tag.

Thanks,
Niklas

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

[Niklas Schnelle]

On Mon, 2026-10-05 at 14:03 +0200, Tobias Schumacher wrote:
> 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.
> Free the summary bit only after the grace period, so it cannot be handed to
> another device while a reader still holds the old pointer.
> 
> zpci_ibv served both delivery modes, indexed by summary bit 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 | 60 +++++++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
--- snip ---
> @@ -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);

This is pre-existing but my additional LLM review noticed it. It
seems like a theoretical race and against the reverse cleanup vs setup
rule, that kfree(zpci_ibv) is done before
unregister_adapter_interrupt(). 

Thanks,
Niklas