Re: [PATCH v5 4/6] scsi: mvumi: reject handshake sizes that overflow the list allocations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: mvumi: ring queue desync when firmware limits shrink across reset
--

commit af2767dc7ac6cb07639493635b81a3c6bd8a53fb
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

scsi: mvumi: reject handshake sizes that overflow the list allocations

This commit adds bounds checking for handshake page sizes to prevent
integer overflows and out-of-bounds writes during allocations. It also
validates that firmware limits do not grow across a host reset.

> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index 9e342db4b8383..e63b3ba7eaabd 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
[ ... ]
> @@ -1054,30 +1067,80 @@ static int mvumi_hs_process_page(struct mvumi_hba *mhba,
>  	}
>  
>  	switch (hs_header->page_code) {
> -	case HS_PAGE_FIRM_CAP:
> +	case HS_PAGE_FIRM_CAP: {
[ ... ]
> +		/* A host reset does not reallocate, so the sizes have to stay
> +		 * the ones the buffers were allocated with.  Only check that
> +		 * the firmware is not asking for more than they hold.
> +		 */
> +		if (mhba->fw_flag & MVUMI_FW_ALLOC) {
> +			if (new_max_io > mhba->max_io ||
> +			    new_ib_max_size > mhba->ib_max_size ||
> +			    new_ob_max_size > mhba->ob_max_size ||
> +			    new_list_num_io > mhba->list_num_io ||
> +			    new_max_target_id > mhba->max_target_id) {
> +				dev_err(&mhba->pdev->dev,
> +					"handshake sizes grew across reset\n");
> +				return -1;
> +			}
> +			break;

[Severity: High]
Does this break statement skip updating the driver's internal limits when the
firmware capacities shrink across a reset?

If a soft reset occurs and the firmware reports smaller capabilities for
list_num_io or max_io, this path is taken because the new values are not
larger than the old ones. However, by breaking out early, the driver misses
updating its internal state to match the smaller firmware limits.

Can this lead to desynchronization of the driver-firmware circular command
rings and overflow of firmware queues?

> +		}
> +
> +		mhba->max_io = new_max_io;
> +		mhba->list_num_io = new_list_num_io;
> +		mhba->max_transfer_size = hs_page1->max_transfer_size;
> +		mhba->max_target_id = new_max_target_id;
> +		mhba->hba_capability = new_capability;
> +		mhba->ib_max_size_setting = hs_page1->cl_in_max_entry_size;
> +		mhba->ib_max_size = new_ib_max_size;
> +		mhba->ob_max_size_setting = hs_page1->cl_out_max_entry_size;
> +		mhba->ob_max_size = new_ob_max_size;
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014151914.3632701-1-yhlee@isslab.korea.ac.kr?part=4