Re: [PATCH v3 86/88] scsi: qla2xxx: Zero SFP DMA buffer in FRU/I2C bsg handlers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Heap buffer overflow in qla_edif_process_els() due to unbounded user payload lengths.
--

--- Patch [86]: [PATCH v3 86/88] scsi: qla2xxx: Zero SFP DMA buffer in FRU/I2C bsg handlers ---
commit 5b2a349a69efbe478558db07cf4b96e8fb11e62b
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Zero SFP DMA buffer in FRU/I2C bsg handlers

The FRU and I2C bsg handlers stage their transfer in a DMA_POOL_SIZE
bounce buffer obtained from dma_pool_alloc(), which does not zero the
allocation. This commit changes these handlers to use dma_pool_zalloc()
to prevent writing stale heap memory to the device flash.

> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index e671c3de8c058..51ab638b4fc09 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -1994,7 +1994,7 @@ qla2x00_update_fru_versions(struct bsg_job *bsg_job)
>  	struct qla_image_version *image;
>  	uint32_t count;
>  	dma_addr_t sfp_dma;
> -	void *sfp = dma_pool_alloc(ha->s_dma_pool, GFP_KERNEL, &sfp_dma);
> +	void *sfp = dma_pool_zalloc(ha->s_dma_pool, GFP_KERNEL, &sfp_dma);
>  
>  	if (!sfp) {
>  		bsg_reply->reply_data.vendor_reply.vendor_rsp[0] =

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the DMA pool
usage in BSG handlers, I noticed a potential buffer overflow in
qla_edif_process_els() located in drivers/scsi/qla2xxx/qla_edif.c.

Does this code allow an unbounded copy into the fixed-size purex_dma_pool?

drivers/scsi/qla2xxx/qla_edif.c:qla_edif_process_els() {
        ...
        sp->remap.req.len = bsg_job->request_payload.payload_len;
        sp->remap.req.buf = dma_pool_alloc(ha->purex_dma_pool, GFP_KERNEL,
                                           &sp->remap.req.dma);
        ...
        sg_copy_to_buffer(bsg_job->request_payload.sg_list,
            bsg_job->request_payload.sg_cnt, sp->remap.req.buf,
            sp->remap.req.len);
        ...
        sp->remap.rsp.len = bsg_job->reply_payload.payload_len;
        sp->remap.rsp.buf = dma_pool_alloc(ha->purex_dma_pool, GFP_KERNEL,
                                           &sp->remap.rsp.dma);
        ...
}

Since purex_dma_pool has a fixed object size (2112 bytes), can a user submit
a BSG request with a request_payload.payload_len or
reply_payload.payload_len greater than 2112 bytes?

If so, could this cause an out-of-bounds write on the DMA pool memory during
the sg_copy_to_buffer() call or during the firmware DMA transfer?

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