Re: [PATCH v2 01/20] scsi: ibmvfc: initialize evt->tgt for NVMe FCP commands

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: data race on fcp_req->private assignment can cause stale pointer dereference
--

commit 76065f9b570dd8183dec90d4771dd155a1cacca2
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

scsi: ibmvfc: initialize evt->tgt for NVMe FCP commands

ibmvfc_nvme_fcp_io() submits NVMe FCP requests without setting evt->tgt
on the allocated event. ibmvfc_init_event() does not initialize this
field, so it retains whatever value was left in the pool-allocated event
structure. Fix this by assigning evt->tgt = rport->private.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> index 9ac2671a3567d..9ed6522c2e9e3 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> @@ -359,6 +359,7 @@ static int ibmvfc_nvme_fcp_io(struct nvme_fc_local_port *lport,
>  	ibmvfc_dbg(vhost, "vfc-nvme-mq-%d\n", evt->hwq);
>  
>  	ibmvfc_init_event(evt, ibmvfc_nvme_done, IBMVFC_CMD_FORMAT);
> +	evt->tgt = rport->private;
>  	evt->fcp_req = fcp_req;
>  	fcp_req->private = evt;

[Severity: High]
Can a concurrent timeout read an uninitialized or stale tgt pointer here?

The assignment fcp_req->private = evt is performed locklessly and without
memory barriers. On weakly ordered architectures like POWER, this allows the
new evt pointer to be visible to other CPUs before the initialization
evt->tgt = rport->private is committed to memory.

The block layer starts the timeout timer before invoking the driver's
.queue_rq callback, meaning the timer can fire concurrently with submission.
If a timeout occurs, the concurrent ibmvfc_nvme_fcp_abort() handler reads
abort_req->private locklessly and accesses abt_evt->tgt to build the TMF
payload.

Observing an uninitialized or stale tgt pointer could result in a NULL
pointer dereference or use-after-free.

Would it be better to use smp_store_release() for fcp_req->private = evt
and smp_load_acquire() in the abort path to ensure the handler sees the fully
initialized event object?

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