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

[sashiko-bot@kernel.org]

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

New issues:
- [High] Missing `IS_QLA29XX` support in the `EDIF_CAP` macro causes `vp_map` allocation failure for QLA29XX when EDIF is enabled, leading to a `BUG_ON` kernel panic.

Pre-existing issues:
- [High] The IO error paths for `qla82xx_start_scsi` and `qla_start_scsi_type6` erroneously free a DMA pool allocation into a slab-backed mempool, causing SLUB corruption and kernel panics.
--

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

scsi: qla2xxx: Add support for QLA29XX in memory allocation

Enhance the qla2x00_mem_alloc function to include checks for
QLA29XX adapters.  This modification updates the conditions for
memory allocation and cleanup, ensuring 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
> @@ -4285,7 +4285,8 @@ qla2x00_mem_alloc(struct qla_hw_data *ha, uint16_t req_len, uint16_t rsp_len,
>  	if (!ha->srb_mempool)
>  		goto fail_free_gid_list;
>  
> -	if (IS_P3P_TYPE(ha) || IS_QLA27XX(ha) || (ql2xsecenable && IS_QLA28XX(ha))) {
> +	if (IS_P3P_TYPE(ha) || IS_QLA27XX(ha) ||
> +	    (ql2xsecenable && (IS_QLA28XX(ha) || IS_QLA29XX(ha)))) {
>  		/* Allocate cache for CT6 Ctx. */
>  		if (!ctx_cachep) {
>  			ctx_cachep = kmem_cache_create("qla2xxx_ctx",

[Severity: High]
This is a pre-existing issue, but do the IO error paths for qla82xx_start_scsi 
and qla_start_scsi_type6 incorrectly free a DMA pool allocation into a 
slab-backed mempool?

The crc_ctx object is allocated from dl_dma_pool via dma_pool_zalloc(), e.g. 
in qla24xx_build_scsi_crc_2_iocbs(), but the error paths in 
qla82xx_start_scsi() and qla_start_scsi_type6() do this:

drivers/scsi/qla2xxx/qla_iocb.c:qla82xx_start_scsi() {
    ...
    if (sp->u.scmd.crc_ctx) {
        mempool_free(sp->u.scmd.crc_ctx, ha->ctx_mempool);
        sp->u.scmd.crc_ctx = NULL;
    }
}

Since ctx_mempool is created here as a slab pool, freeing DMA-coherent memory 
back into it causes SLUB corruption or kernel panics when queueing fails under 
load.

> @@ -4319,7 +4320,8 @@ qla2x00_mem_alloc(struct qla_hw_data *ha, uint16_t req_len, uint16_t rsp_len,
>  	    "init_cb=%p gid_list=%p, srb_mempool=%p s_dma_pool=%p.\n",
>  	    ha->init_cb, ha->gid_list, ha->srb_mempool, ha->s_dma_pool);
>  
> -	if (IS_P3P_TYPE(ha) || ql2xenabledif || (IS_QLA28XX(ha) && ql2xsecenable)) {
> +	if (IS_P3P_TYPE(ha) || ql2xenabledif ||
> +	    ((IS_QLA28XX(ha) || IS_QLA29XX(ha)) && ql2xsecenable)) {

[Severity: High]
By adding QLA29XX to the ql2xsecenable condition here, we enable EDIF 
allocations for the new adapters. However, does this miss updating the 
EDIF_CAP macro?

The EDIF_CAP macro in drivers/scsi/qla2xxx/qla_edif.h remains hardcoded to 
IS_QLA28XX:

drivers/scsi/qla2xxx/qla_edif.h:
#define EDIF_CAP(_ha) (ql2xsecenable && IS_QLA28XX(_ha))

Earlier in qla2x00_mem_alloc(), vp_map is conditionally allocated using 
this macro:

drivers/scsi/qla2xxx/qla_os.c:qla2x00_mem_alloc() {
    if (QLA_TGT_MODE_ENABLED() || EDIF_CAP(ha)) {
        ha->vp_map = kzalloc_objs(struct qla_vp_map, MAX_MULTI_ID_FABRIC);
    }
}

If QLA29XX adapters operate in initiator mode with EDIF enabled, vp_map 
will be NULL. When EDIF receives ELS/Auth packets, it routes through 
target-mode infrastructure and calls qla_find_host_by_vp_idx(), which 
will trigger a BUG_ON(ha->vp_map == NULL) panic.

[ ... ]

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