Re: [PATCH v5 6/6] scsi: mvumi: widen the flush cache loop counter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] scsi: mvumi: target ID and LUN overlap causes command misrouting
--

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

scsi: mvumi: widen the flush cache loop counter

mvumi_flush_cache() walks the targets with an unsigned char counter while
the bound it compares against, mhba->max_target_id, comes from the u16
max_devices_support field of the firmware handshake page. A controller
reporting 256 or more devices makes the counter wrap back to zero before
it ever reaches the bound, so the loop never ends. This patch gives the
counter the type of the bound to prevent this wedge.

> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index 07cfac9276e7b..090ae79ec8c3a 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
> @@ -769,8 +769,9 @@ static unsigned char mvumi_flush_cache(struct mvumi_hba *mhba)
>  {
>  	struct mvumi_cmd *cmd;
>  	struct mvumi_msg_frame *frame;
> -	unsigned char device_id, retry = 0;
> +	unsigned char retry = 0;
>  	unsigned char bitcount = sizeof(unsigned char) * 8;
> +	unsigned short device_id;
>  
>  	for (device_id = 0; device_id < mhba->max_target_id; device_id++) {
>  		if (!(mhba->target_map[device_id / bitcount] &

[Severity: Critical]
By widening the device_id counter to unsigned short, this code now allows
target IDs of 256 and higher to be processed. Does this safely interact with
the I/O submission path?

Looking at mvumi_build_frame(), the target ID and LUN are packed together
into the device_id field using a bitwise OR:

drivers/scsi/mvumi.c:mvumi_build_frame() {
    ...
	pframe->device_id = ((unsigned short) scmd->device->id) |
				(((unsigned short) scmd->device->lun) << 8);
    ...
}

If a target ID is 256 or greater (e.g., 0x0100), its bits will bleed into the
LUN space. For example, an I/O to Target 256 LUN 0 would collide exactly with
an I/O to Target 0 LUN 1. 

Could this lead to silent command misrouting and data corruption for
high-numbered targets newly unblocked by this patch?

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