Re: [PATCH v2 2/3] scsi: mvumi: quarantine a timed-out internal command's buffers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] scsi: mvumi: stranded mhba_dev_list entries when
  mvumi_probe_devices() aborts
--

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

scsi: mvumi: quarantine a timed-out internal command's buffers

This commit moves the DMA buffers of timed-out internal commands onto
mhba->res_list instead of freeing them immediately, and stops the target
scan loop in mvumi_probe_devices() when an inquiry command times out.

> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index 8d22d370d8608..48652640afc18 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
[ ... ]
> @@ -1552,6 +1584,7 @@ static int mvumi_probe_devices(struct mvumi_hba *mhba)
>  	struct mvumi_device *mv_dev = NULL;
>  	struct mvumi_cmd *cmd = NULL;
>  	int found = 0;
> +	int ret;
>  
>  	cmd = mvumi_create_internal_cmd(mhba, 64);
>  	if (!cmd)
> @@ -1564,6 +1597,11 @@ static int mvumi_probe_devices(struct mvumi_hba *mhba)
>  
>  	for (id = 0; id < maxid; id++) {
>  		wwid = mvumi_inquiry(mhba, id, cmd);
> +		/* the controller still owns this command's buffers */
> +		if (cmd->timed_out) {
> +			ret = -1;
> +			goto out;
> +		}

[Severity: Medium]
Does aborting the loop here strand struct mvumi_device entries allocated
for earlier target IDs on mhba->mhba_dev_list?

When an earlier iteration finds a new target, mvumi_probe_devices()
removes any prior entry from mhba->shost_dev_list and appends a new
struct mvumi_device to mhba->mhba_dev_list.  If a later target ID times
out and returns -1, mvumi_rescan_bus() skips draining mhba->mhba_dev_list:

drivers/scsi/mvumi.c:mvumi_rescan_bus() {
	...
		ret = mvumi_probe_devices(mhba);
		if (!ret) {
			list_for_each_entry_safe(mv_dev, dev_next,
						 &mhba->mhba_dev_list, list) {
	...
}

Because mvumi_match_devices() and mvumi_remove_devices() only inspect
mhba->shost_dev_list, a later rescan will allocate duplicate entries on
mhba->mhba_dev_list for those earlier targets or bring stale entries
online if a target disappeared in the meantime.

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