Re: [PATCH v3 07/88] scsi: qla2xxx: Add flash block read/write BSG support for 29xx

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Region-relative flash offset is incorrectly validated against the total flash size.
--

--- Patch [7]: [PATCH v3 07/88] scsi: qla2xxx: Add flash block read/write BSG support for 29xx ---
commit 31fdf59e3896b2b2cd9a7fe0c5b72367e33df679
Author: Manish Rangankar <mrangankar@marvell.com>

scsi: qla2xxx: Add flash block read/write BSG support for 29xx

Introduce QL_VND_READ_FLASH_BLOCK and QL_VND_WRITE_FLASH_BLOCK
BSG vendor commands so that userspace tools can perform flash
block-level operations on 29xx adapters via the isp_ops
interface.

> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index 5e910b5ca670d..ff7421f3535dc 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c

[ ... ]

> @@ -1431,42 +1422,79 @@ qla2x00_optrom_setup(struct bsg_job *bsg_job, scsi_qla_host_t *vha,
>  	}
>  
>  	ha->optrom_region_start = start;
> +
> +	if (IS_QLA29XX(ha)) {
> +		if (start > ha->optrom_size) {
> +			ql_log(ql_log_warn, vha, 0x7055,
> +			    "start %d > optrom_size %d.\n", start,
> +			    ha->optrom_size);
>  			return -EINVAL;
>  		}
>  
> +		if (is_update) {
> +			ha->optrom_region_size = start +
> +			    bsg_job->request_payload.payload_len >
> +			    ha->optrom_size ?
> +			    ha->optrom_size - start :
> +			    bsg_job->request_payload.payload_len;

[Severity: High]
Does this allow out-of-bounds cross-region flash operations?

For QLA29XX block operations, start is populated with a region-relative
offset, but it is being validated and clamped against the entire adapter
flash size (ha->optrom_size).

If a user specifies a valid region but provides a region-relative offset
and length that exceeds the specific region's actual boundaries, wouldn't
this code bypass region isolation and allow reading or writing outside the
intended region?

[ ... ]

> +static int qla29xx_bsg_flash_block_write(struct bsg_job *bsg_job)
> +{

[ ... ]

> +	bwcmd =
> +	   (struct qla_block_rw *)&bsg_req->rqst_data.h_vendor.vendor_cmd[2];

[ ... ]

> +	mutex_lock(&ha->optrom_mutex);
> +	rval = qla2x00_optrom_setup(bsg_job, vha, bwcmd->region_offset, 1);

[Severity: High]
This is where the user-provided region-relative offset is passed to
qla2x00_optrom_setup().

After the incorrect global size clamping is applied, the offset is dispatched
to the firmware via write_optrom_region() without any further region boundary
checks.

Could this allow users to overwrite critical areas of flash and potentially
damage the hardware adapter?

> +	if (rval) {
> +		mutex_unlock(&ha->optrom_mutex);
> +		return rval;
> +	}
> +
> +	sg_copy_to_buffer(bsg_job->request_payload.sg_list,
> +			bsg_job->request_payload.sg_cnt, ha->optrom_buffer,
> +			ha->optrom_region_size);

[ ... ]

> +	rval = ha->isp_ops->write_optrom_region(vha, bwcmd->region, opts,
> +				ha->optrom_buffer, ha->optrom_region_start,
> +				ha->optrom_region_size);

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