Re: [PATCH v5 24/56] scsi: qla2xxx: Add support for QLA29XX in memory allocation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unconditional calls to `dma_pool_free()` with conditionally allocated (and potentially NULL) pointers cause a kernel panic during error recovery.
--

--- Patch [24]: [PATCH v5 24/56] scsi: qla2xxx: Add support for QLA29XX in memory allocation ---
commit d2e304ac1a70602773864240ceed6148180e105c
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Add support for QLA29XX in memory allocation

This patch enhances the qla2x00_mem_alloc function to include checks for
QLA29XX adapters. This ensures proper handling of the new adapter series
alongside existing QLA27XX and QLA28XX checks.

> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 5450c40259bf9..3be59179a0245 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
[ ... ]
> @@ -4681,12 +4683,12 @@ qla2x00_mem_alloc(struct qla_hw_data *ha, uint16_t req_len, uint16_t rsp_len,
>  	}
>  
>  fail_dif_bundl_dma_pool:
> -	if (IS_QLA82XX(ha) || ql2xenabledif) {
> +	if (IS_QLA82XX(ha) || IS_QLA29XX(ha) || ql2xenabledif) {
>  		dma_pool_destroy(ha->fcp_cmnd_dma_pool);
>  		ha->fcp_cmnd_dma_pool = NULL;
>  	}
>  fail_dl_dma_pool:
> -	if (IS_QLA82XX(ha) || ql2xenabledif) {
> +	if (IS_QLA82XX(ha) || IS_QLA29XX(ha) || ql2xenabledif) {
>  		dma_pool_destroy(ha->dl_dma_pool);
>  		ha->dl_dma_pool = NULL;
>  	}

[Severity: High]
This is a pre-existing issue, but while reviewing the error cleanup paths here,
I noticed a potential issue slightly higher up in the same function.

Could the unconditional calls to dma_pool_free() higher up in the cleanup
sequence cause a kernel panic?

In drivers/scsi/qla2xxx/qla_os.c:qla2x00_mem_alloc(), several DMA pool
allocations are conditional based on adapter capabilities. For example,
ha->ex_init_cb and ha->sf_init_cb are allocated for CNA/newer adapters,
ha->async_pd for non-FWI2, and ha->ms_iocb for non-2100/2200.

If a subsequent memory allocation fails, the error recovery path jumps
to labels that unconditionally call dma_pool_free():

fail_loop_id_map:
	dma_pool_free(ha->s_dma_pool, ha->async_pd, ha->async_pd_dma);
fail_async_pd:
	dma_pool_free(ha->s_dma_pool, ha->sf_init_cb, ha->sf_init_cb_dma);
fail_sf_init_cb:
	dma_pool_free(ha->s_dma_pool, ha->ex_init_cb, ha->ex_init_cb_dma);

And later:

fail_free_ms_iocb:
	dma_pool_free(ha->s_dma_pool, ha->ms_iocb, ha->ms_iocb_dma);

If the pointers were never allocated, they remain NULL. Since
dma_pool_free() does not safely handle NULL pointers in its fast path,
calling it with a NULL pointer could trigger a kernel panic during error
recovery. Should these calls be guarded with NULL checks to prevent
crashing when handling an allocation failure?

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