Re: [PATCH v3 56/88] scsi: qla2xxx: Clamp MSI-X derived queue counts to avoid truncation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch failed to fully resolve the ZERO_SIZE_PTR vulnerability it aimed to fix. A malicious or misconfigured PCI device returning a capability offset that causes ha->msix_count to become 0 still leads to a ZERO_SIZE_PTR allocation and subsequent out-of-bounds write.
--

--- Patch [56]: [PATCH v3 56/88] scsi: qla2xxx: Clamp MSI-X derived queue counts to avoid truncation ---
commit 96603c11bc04fcf9f524adb1b5140a4ef7b1d7f9
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Clamp MSI-X derived queue counts to avoid truncation

This commit adds qla_calc_queue_count() to clamp the derived MSI-X queue
count into a valid range. This prevents an allocation of zero elements
that leads to memory corruption when too many vectors are advertised.

> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 186c6c7a39448..5209dda454596 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -2132,7 +2132,7 @@ qla2x00_iospace_config(struct qla_hw_data *ha)
>  		ha->msix_count = msix + 1;
>  		/* Max queues are bounded by available msix vectors */
>  		/* MB interrupt uses 1 vector */
> -		ha->max_req_queues = ha->msix_count - 1;
> +		ha->max_req_queues = qla_calc_queue_count(ha->msix_count);

[Severity: High]
This isn't a bug introduced by this patch, but it looks like the ZERO_SIZE_PTR
vulnerability this patch aims to fix might still be reachable through a
different path.

If a malicious or misconfigured PCI device returns 0xFFFF for
QLA_PCI_MSIX_CONTROL, could ha->msix_count wrap to 0?

drivers/scsi/qla2xxx/qla_os.c:qla2x00_iospace_config() {
    ...
    pci_read_config_word(ha->pdev, QLA_PCI_MSIX_CONTROL, &msix);
    ha->msix_count = msix + 1;
    ...
}

Later in qla24xx_enable_msix(), if pci_alloc_irq_vectors() successfully
allocates vectors (e.g., returning 2), the condition (ret < ha->msix_count)
evaluates as (2 < 0) due to type promotion, which is false:

drivers/scsi/qla2xxx/qla_isr.c:qla24xx_enable_msix() {
    ...
    } else if (ret < ha->msix_count) {
        ...
        ha->msix_count = ret;
    }
    ...
    ha->msix_entries = kzalloc_objs(struct qla_msix_entry, ha->msix_count);
    ...
}

This leaves ha->msix_count at 0, resulting in kzalloc_objs() returning
ZERO_SIZE_PTR.

When the base vectors are unconditionally initialized shortly after, does
this cause an out-of-bounds write by dereferencing ZERO_SIZE_PTR (0x10)?

drivers/scsi/qla2xxx/qla_isr.c:qla24xx_enable_msix() {
    ...
    /* Enable MSI-X vectors for the base queue */
    for (i = 0; i < QLA_BASE_VECTORS; i++) {
        qentry = &ha->msix_entries[i];
        qentry->handle = rsp;
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707055435.2680300-1-njavali@marvell.com?part=56