Re: [PATCH 5/7] s390/pci: add NULL check in zpci_msi_clear_airq()

[sashiko-bot@kernel.org]

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