Re: [PATCH v2 15/33] ibmvfc: send NVMe target discovery MAD

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Concurrent SCSI and NVMe target discovery requests race, causing DMA data races and ignored discovery responses.
- [High] NVMe targets are ignored by the host state machine and lifecycle loops, leading to memory leaks and uninitialized targets.
--

--- Patch [15]: [PATCH v2 15/33] ibmvfc: send NVMe target discovery MAD ---
commit 64be8514770ed6154d989f0192b5c3cb746d46d2
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: send NVMe target discovery MAD

This commit extends target discovery to send protocol-specific discover-target
MADs for NVMe/FC. It uses a protocol-aware discovery helper to build and submit
an NVMe discover-target request, processing the returned target count into the
NVMe channel group's discovery buffer.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index a700e2554e692..1d2a88e9fe941 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
[ ... ]
> @@ -5011,19 +5016,30 @@ static void ibmvfc_discover_targets_done(struct ibmvfc_event *evt)
>  {
>  	struct ibmvfc_host *vhost = evt->vhost;
>  	struct ibmvfc_discover_targets *rsp = &evt->xfer_iu->discover_targets;
> +	struct ibmvfc_channels *channels;
>  	u32 mad_status = be16_to_cpu(rsp->common.status);
> +	u32 opcode = be32_to_cpu(rsp->common.opcode);
>  	int level = IBMVFC_DEFAULT_LOG_LEVEL;
>  
> +	if (opcode == IBMVFC_DISC_TARGETS)
> +		channels = &vhost->scsi_scrqs;
> +	else
> +		channels = &vhost->nvme_scrqs;
> +
>  	switch (mad_status) {
>  	case IBMVFC_MAD_SUCCESS:
> -		ibmvfc_dbg(vhost, "Discover Targets succeeded\n");
> -		vhost->scsi_scrqs.num_targets = min_t(u32, be32_to_cpu(rsp->num_written),
> -						      max_targets);
> +		ibmvfc_dbg(vhost, "Discover %s Targets succeeded\n",
> +			   proto_type[channels->protocol]);
> +		channels->num_targets = min_t(u32, be32_to_cpu(rsp->num_written),
> +					      max_targets);
> +		ibmvfc_dbg(vhost, "%d %s targets found\n", channels->num_targets,
> +			   proto_type[channels->protocol]);
>  		ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_ALLOC_TGTS);

[Severity: High]
If SCSI and NVMe target discovery requests are sent concurrently, won't the
first completion trigger ibmvfc_discover_targets_done() and immediately set
the host action to IBMVFC_HOST_ACTION_ALLOC_TGTS?

This state transition wakes the worker thread, which then calls
ibmvfc_alloc_targets().

Because ibmvfc_alloc_targets() unconditionally loops over both the nvme_scrqs
and scsi_scrqs discovery buffers to parse targets, could the worker thread
read from a buffer while it is still being actively modified by the VIOS via
DMA for the slower, in-flight protocol?

Additionally, when the slower protocol finally completes, its transition to
ALLOC_TGTS is ignored because the host is no longer in INIT_WAIT. Does this
cause those discovered targets to be silently dropped?

[Severity: High]
When NVMe targets are successfully discovered here, they are eventually
allocated and appended to vhost->nvme_scrqs.targets by ibmvfc_alloc_target().

However, looking at the host state machine loops in ibmvfc_do_work()
(specifically for IBMVFC_HOST_ACTION_TGT_INIT) and the teardown logic in
ibmvfc_link_down():

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_do_work()
    case IBMVFC_HOST_ACTION_TGT_INIT:
        list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
            if (tgt->action == IBMVFC_TGT_ACTION_INIT) {
                tgt->job_step(tgt);
                break;
            }
        }

These loops appear to only iterate over vhost->scsi_scrqs.targets. 

Will this cause the newly populated NVMe targets to be completely ignored
for initialization?

Does this also mean these NVMe target structures will leak during driver
teardown, since ibmvfc_link_down() also only cleans up the scsi_scrqs list?

>  		break;
>  	case IBMVFC_MAD_FAILED:
[ ... ]

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