Re: [PATCH 17/29] ibmvfc: delete NVMe/FC targets as well as SCSI

[sashiko-bot@kernel.org]

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

New issues:
- [High] Fabric ELS events (LOGO, PRLO, PLOGI) fail to process NVMe targets, violating the commit's claim regarding consistent fabric event handling.

Pre-existing issues:
- [High] An integer overflow in the calculation of `sense_len + rsp_len` bypasses bounds checking, leading to an out-of-bounds read and write in `memcpy()`.
--

commit 56df0a05e56764e1456bdc6176dcf26a3a1bc6b0
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: delete NVMe/FC targets as well as SCSI

This commit extends target deletion paths to process NVMe targets alongside
SCSI targets. It updates link-down and host reinitialization flows to walk
both lists to maintain consistent target state. It also renames
ibmvfc_relogin to ibmvfc_scsi_relogin.

>     Update link-down, and host reinitialization flows to walk both the SCSI
>     and NVMe target lists when marking targets for deletion. This ensures
>     that protocol-specific target state stays consistent across adapter
>     resets and fabric events.

[Severity: High]
Does the current patch fully handle fabric events for NVMe targets?

The commit message mentions maintaining consistent target state across fabric
events, but it appears ibmvfc_handle_async in
drivers/scsi/ibmvscsi/ibmvfc-core.c was not updated to process the
nvme_scrqs.targets list.

When an NVMe target logs out of the fabric and the adapter sends an async
IBMVFC_AE_ELS_LOGO event, the ELS event handler loop only searches
scsi_scrqs.targets. Can this cause stale NVMe targets to remain in memory
indefinitely and break the target state machine?

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 4b95e43449473..13e513bfd0a86 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c

[ ... ]

> @@ -1920,7 +1928,7 @@ static void ibmvfc_scsi_done(struct ibmvfc_event *evt)
>  				memcpy(cmnd->sense_buffer, rsp->data.sense + rsp_len, sense_len);

[Severity: High]
This is a pre-existing issue, but can an integer overflow occur here when
calculating sense_len + rsp_len in ibmvfc_scsi_done?

If a buggy or compromised virtual I/O server returns a large fcp_sense_len
like 0xFFFFFFFE and a small fcp_rsp_len like 4, their sum could wrap around
to 2.

This wrapped value would bypass the greater than SCSI_SENSE_BUFFERSIZE
bounds check. Since rsp_len <= 8 would still be true, could the memcpy call
attempt to copy nearly 4GB of data and cause memory corruption?

>  			if ((be16_to_cpu(vfc_cmd->status) & IBMVFC_VIOS_FAILURE) &&
>  			    (be16_to_cpu(vfc_cmd->error) == IBMVFC_PLOGI_REQUIRED))
> -				ibmvfc_relogin(cmnd->device);
> +				ibmvfc_scsi_relogin(cmnd->device);
>  
>  			if (!cmnd->result && (!scsi_get_resid(cmnd) || (rsp->flags & FCP_RESID_OVER)))
>  				cmnd->result = (DID_ERROR << 16);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623013035.3436640-1-tyreld@linux.ibm.com?part=17