Re: [PATCH 03/20] scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] scsi: ibmvfc: double completion and UAF on NVMe H_CLOSED error path
- [High] scsi: ibmvfc: invalid assignment of -EBUSY to unsigned NVMe fcp_req->status
--

commit 09b742c396cef74ab36e606d794a591d97008dc8
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure

When ibmvfc_send_event() returns H_CLOSED the event is freed without
notifying the NVMe-FC transport, which left the transport waiting on a
request that was silently dropped. This commit fixes the hang by explicitly
completing the NVMe FCP request with a busy status before freeing the event.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 6c243b2a84d7a..26a73db662545 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> @@ -1845,6 +1845,10 @@ int ibmvfc_send_event(struct ibmvfc_event *evt,
>  				dev_warn(vhost->dev, "Send warning. Receive queue closed, will retry.\n");
>  			if (evt->cmnd)
>  				scsi_dma_unmap(evt->cmnd);
> +			else if (evt->fcp_req) {
> +				evt->fcp_req->status = -EBUSY;
[Severity: High]
Does assigning -EBUSY to fcp_req->status result in an underflow? Since this
field is an unsigned 32-bit integer, -EBUSY (-16) becomes 0xfffffff0.

When the NVMe-FC completion handler nvme_fc_fcpio_done() checks if this
status is non-zero, it might translate it into a fatal
NVME_SC_HOST_PATH_ERROR instead of recognizing it as a retry request,
potentially causing a permanent I/O failure rather than a seamless retry.

> +				evt->fcp_req->done(evt->fcp_req);
> +			}
>  			ibmvfc_free_event(evt);
>  			return SCSI_MLQUEUE_HOST_BUSY;
[Severity: Critical]
Will this lead to a double completion and use-after-free? When
ibmvfc_send_event() returns SCSI_MLQUEUE_HOST_BUSY (which is a positive
value), the caller ibmvfc_nvme_fcp_io() propagates this non-zero error code
directly back to the NVMe-FC transport.

The NVMe-FC LLDD API contract requires that if the fcp_io callback returns a
non-zero value, it signifies a synchronous queueing failure, and the driver
must not call fcp_req->done().

Because the NVMe-FC core executes its own teardown path that unmaps the SG
list and completes the request when the fcp_io callback returns non-zero,
invoking done() here inline causes it to also be completed asynchronously,
resulting in a fatal use-after-free and double DMA unmap.

Should the driver instead omit calling done() completely and translate the
internal SCSI_MLQUEUE_HOST_BUSY into -EBUSY as the direct return value of
ibmvfc_nvme_fcp_io() to correctly signal a transport retry?

>  		}

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