Re: [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init

["Tobias Schumacher" <ts@linux.ibm.com>]

On Wed Aug 19, 2026 at 11:02 AM CEST, sashiko-bot wrote:
> 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_directed_irq_init() is reached from zpci_irq_init(), which
pci_base_init() calls before any device exists. A function only starts
generating directed interrupts once it has been registered with
zpci_set_directed_irq(), called from zpci_msi_prepare()->zpci_set_irq().

Tobias