Re: [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup

On Wed Aug 19, 2026 at 11:05 AM CEST, sashiko-bot wrote:

-- snip --

> --- Patch [3]: [PATCH 3/7] s390/pci: fix resource leak in zpci MSI setup ---
> commit ac232ab92e1500cdc9f0a1a221f03f9be96fffe4
> Author: Tobias Schumacher <[email protected]>
>
> 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?

Duplicate finding already answered for Patch 2, yes, will fix this.

> [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?

This is what Patch 4 addresses.

> [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?

Good catch, will fix this.

Tobias