[PATCH v1] scsi: be2iscsi: Fix MSI-X IRQ cleanup

[PATCH v1] scsi: be2iscsi: Fix MSI-X IRQ cleanup

[Yuho Choi]

beiscsi_init_irqs() uses j to walk previously registered MSI-X vectors
when unwinding an allocation or request_irq() failure, but passes i to
pci_irq_vector(). Consequently, each free_irq() targets the failing
vector instead of the vector associated with be_eq[j].

Use j to release the same vector that was registered. This ensures all
successfully requested IRQs are freed before initialization failure tears
down the HBA.

Fixes: 831488669a33 ("scsi: be2iscsi: switch to pci_alloc_irq_vectors")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
 drivers/scsi/be2iscsi/be_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index fd18d4d3d219..9833ab06b299 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -878,7 +878,7 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
 	return 0;
 free_msix_irqs:
 	for (j = i - 1; j >= 0; j--) {
-		free_irq(pci_irq_vector(pcidev, i), &phwi_context->be_eq[j]);
+		free_irq(pci_irq_vector(pcidev, j), &phwi_context->be_eq[j]);
 		kfree(phba->msi_name[j]);
 	}
 	return ret;
-- 
2.43.0

Re: [PATCH v1] scsi: be2iscsi: Fix MSI-X IRQ cleanup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The logic handling the return value of `pci_alloc_irq_vectors_affinity` in `be2iscsi_enable_msix` is completely inverted, leading to severe resource underutilization on success and guaranteed out-of-bounds array accesses on failure.
--

--- Patch [1]: [PATCH v1] scsi: be2iscsi: Fix MSI-X IRQ cleanup ---
commit 7ef06019dd8a33e5e182606a30c7033f3e41b3de
Author: Yuho Choi <dbgh9129@gmail.com>

scsi: be2iscsi: Fix MSI-X IRQ cleanup
    
This patch fixes the MSI-X IRQ cleanup path in beiscsi_init_irqs(). When
unwinding an allocation failure, it uses the correct index to release the
same vectors that were registered.

> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index fd18d4d3d219b..9833ab06b299a 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -878,7 +878,7 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
>  	return 0;
>  free_msix_irqs:
>  	for (j = i - 1; j >= 0; j--) {
> -		free_irq(pci_irq_vector(pcidev, i), &phwi_context->be_eq[j]);
> +		free_irq(pci_irq_vector(pcidev, j), &phwi_context->be_eq[j]);
>  		kfree(phba->msi_name[j]);
>  	}
>  	return ret;

[Severity: High]
This is a pre-existing issue, but while reviewing the MSI-X initialization in
this driver, I noticed what appears to be inverted error handling logic in
be2iscsi_enable_msix():

drivers/scsi/be2iscsi/be_main.c:be2iscsi_enable_msix() {
    ...
		if (pci_alloc_irq_vectors_affinity(phba->pcidev, 2, nvec,
				PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, &desc) < 0) {
			phba->num_cpus = nvec - 1;
			return;
		}
	}

	phba->num_cpus = 1;
    ...
}

Does this code set phba->num_cpus to a high value on allocation failure, and
limit it to 1 on success?

If pci_alloc_irq_vectors_affinity() fails, phba->num_cpus is set to nvec - 1,
where nvec can be up to 2048. Subsequent functions iterate up to this value:

drivers/scsi/be2iscsi/be_main.c:beiscsi_create_eqs() {
    ...
	for (i = 0; i < (phba->num_cpus + eq_for_mcc); i++) {
		eq = &phwi_context->be_eq[i].q;
    ...
}

Because the be_eq array in struct hwi_context_memory is strictly bounded by
MAX_CPUS (64), can this out-of-bounds access corrupt memory if the MSI-X
allocation fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803014224.614347-1-dbgh9129@gmail.com?part=1