Re: [PATCH 25/29] ibmvfc: implement nvme-fc LS submission transport callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver accesses NVMe-FC response payloads (`evt->ls_req->rspaddr` and `fcp_req->rspaddr`) from CPU memory while they are still mapped for DMA, before the NVMe-FC core unmaps or syncs them. On architectures using SWIOTLB bounce buffers (like PowerPC Secure Execution), this causes the driver to read stale data and its manual payload writes to be clobbered by the subsequent DMA sync.
--

commit 1915a13751880ed8af4917b3092d7cf5b63c79eb
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: implement nvme-fc LS submission transport callback
    
NVMe FC Link Service commands are required to use the ibmvfc_passthru
MAD. Initialize a pssthru mad for the target port including the DMA
addresses for the FC4_LS request and response as well as the max length
of each IU as provided in the nvmefc_ls_req struct. FC4_LS commands are
sent via the primary CRQ. Further, store the assoc_id during a create
association request as this is a required field in our vfc_cmd struct
for nvme_fcp_io commands.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> index 1108d11d6b2db..506135c1a34e3 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c

[ ... ]

> @@ -62,10 +62,99 @@ static void ibmvfc_nvme_delete_queue(struct nvme_fc_local_port *lport, unsigned
>  	kfree(handle);
>  }
>  
> +static void ibmvfc_ls_req_done(struct ibmvfc_event *evt)
> +{
> +	struct ibmvfc_target *tgt = evt->tgt;
> +	struct ibmvfc_passthru_mad *mad = &evt->xfer_iu->passthru;
> +	struct fcnvme_ls_rqst_w0 *ls_rqst;
> +	struct fcnvme_ls_cr_assoc_acc *ls_resp;
> +	u32 status = be16_to_cpu(mad->common.status);
> +	int rc = 0;
> +
> +	ls_rqst = (struct fcnvme_ls_rqst_w0 *)evt->ls_req->rqstaddr;
> +	ls_resp = (struct fcnvme_ls_cr_assoc_acc *)evt->ls_req->rspaddr;
> +
> +	switch (status) {
> +	case IBMVFC_MAD_SUCCESS:
> +		tgt_dbg(tgt, "ls_req succeeded\n");
> +		if ((ls_rqst->ls_cmd == FCNVME_LS_CREATE_ASSOCIATION) &&
> +		    (ls_resp->hdr.w0.ls_cmd == FCNVME_LS_ACC)) {
> +			tgt->assoc_id = be64_to_cpu(ls_resp->associd.association_id);

[Severity: High]
Does this code read stale data from the response buffer when bounce buffers
are active?

Since the response payload at evt->ls_req->rspaddr is mapped for DMA, and the
NVMe-FC core hasn't unmapped or synced it yet, reading the association_id
here without a prior dma_sync_single_for_cpu() could result in reading stale
memory on architectures using SWIOTLB bounce buffers (like PowerPC Secure
Execution).

Additionally, does a similar issue occur in ibmvfc_nvme_done()? If the driver
reads ersp->xfrd_len from rspaddr and writes to cqe->command_id before
calling fcp_req->done(), the subsequent DMA sync inside fcp_req->done()
could overwrite the manually written command_id with unmodified bounce buffer
memory, causing the I/O to fail.

> +			tgt_dbg(tgt, "assoc_id 0x%llx\n", tgt->assoc_id);
> +		}
> +		break;
> +	case IBMVFC_MAD_DRIVER_FAILED:
> +		break;

[ ... ]

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