* [PATCH v2 01/20] scsi: ibmvfc: initialize evt->tgt for NVMe FCP commands
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:51 ` sashiko-bot
2026-09-19 1:32 ` [PATCH v2 02/20] scsi: ibmvfc: fix trace logging " Tyrel Datwyler
` (18 subsequent siblings)
19 siblings, 1 reply; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
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.
When an FCP request times out the NVMe-FC transport calls
ibmvfc_nvme_fcp_abort(), which in turn calls ibmvfc_init_fcp_abort() to
build the cancel MAD. That function derives the target pointer from
abt_evt->tgt:
struct ibmvfc_target *tgt = abt_evt->tgt;
...
tmf->target_wwpn = cpu_to_be64(tgt->wwpn);
tmf->assoc_id = cpu_to_be64(tgt->assoc_id);
Because abt_evt->tgt was never initialized this is a guaranteed NULL or
stale-pointer dereference on the first NVMe I/O timeout, resulting in a
kernel panic.
Fix this by assigning evt->tgt = rport->private immediately after
ibmvfc_init_event(), consistent with how ibmvfc_nvme_ls_req() already
sets evt->tgt for LS requests. This also has the benefit of making the
associated remote port target visible on in-flight FCP events, which is
useful for live debugging (e.g. reading tgt->wwpn from a hung command
in a crash/kdump context).
Fixes: 4e70b8795ee3 ("scsi: ibmvfc: implement nvme-fc FCP abort callback")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index 9ac2671a3567..9ed6522c2e9e 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;
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 01/20] scsi: ibmvfc: initialize evt->tgt for NVMe FCP commands
2026-09-19 1:32 ` [PATCH v2 01/20] scsi: ibmvfc: initialize evt->tgt for NVMe FCP commands Tyrel Datwyler
@ 2026-09-19 1:51 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-09-19 1:51 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: linux-scsi
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 02/20] scsi: ibmvfc: fix trace logging for NVMe FCP commands
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 01/20] scsi: ibmvfc: initialize evt->tgt for NVMe FCP commands Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:47 ` sashiko-bot
2026-09-19 1:32 ` [PATCH v2 03/20] scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure Tyrel Datwyler
` (17 subsequent siblings)
19 siblings, 1 reply; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_trc_start() and ibmvfc_trc_end() handle IBMVFC_CMD_FORMAT events
by reading SCSI-specific fields via ibmvfc_get_fcp_iu() and
ibmvfc_get_fcp_rsp(). These helpers return a pointer into the v3scsi
union member of struct ibmvfc_cmd, but NVMe FCP commands store their
command IU in the v3nvme union member. Reading SCSI FCP fields out of an
NVMe IU produces garbage trace entries (wrong opcode, lun, tmf_flags,
xfer_len, rsp_code, and scsi_status).
Fix this by gating the SCSI-specific field reads on evt->cmnd within the
IBMVFC_CMD_FORMAT branch. For NVMe FCP events (evt->fcp_req set) record
the NVMe SQE opcode from v3nvme.iu.sqe.common.opcode and the payload
length from vfc_cmd->payload_len. The common fields that are valid for
both protocols (tgt_scsi_id, vfc_cmd->status, vfc_cmd->error) are
logged unconditionally.
Fixes: 73c13e30c56f ("scsi: ibmvfc: implement nvme-fc IO command submission callback")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 31 +++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index aa7ae81df41b..6558ed3c67c4 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -250,6 +250,7 @@ static void ibmvfc_trc_start(struct ibmvfc_event *evt)
int index = atomic_inc_return(&vhost->trace_index) & IBMVFC_TRACE_INDEX_MASK;
entry = &vhost->trace[index];
+ memset(entry, 0, sizeof(*entry));
entry->evt = evt;
entry->time = jiffies;
entry->fmt = evt->crq.format;
@@ -257,11 +258,16 @@ static void ibmvfc_trc_start(struct ibmvfc_event *evt)
switch (entry->fmt) {
case IBMVFC_CMD_FORMAT:
- entry->op_code = iu->cdb[0];
entry->scsi_id = be64_to_cpu(vfc_cmd->tgt_scsi_id);
- entry->lun = scsilun_to_int(&iu->lun);
- entry->tmf_flags = iu->tmf_flags;
- entry->u.start.xfer_len = be32_to_cpu(iu->xfer_len);
+ if (evt->cmnd) {
+ entry->op_code = iu->cdb[0];
+ entry->lun = scsilun_to_int(&iu->lun);
+ entry->tmf_flags = iu->tmf_flags;
+ entry->u.start.xfer_len = be32_to_cpu(iu->xfer_len);
+ } else if (evt->fcp_req) {
+ entry->op_code = vfc_cmd->v3nvme.iu.sqe.common.opcode;
+ entry->u.start.xfer_len = evt->fcp_req->payload_length;
+ }
break;
case IBMVFC_MAD_FORMAT:
entry->op_code = be32_to_cpu(mad->opcode);
@@ -287,6 +293,7 @@ static void ibmvfc_trc_end(struct ibmvfc_event *evt)
int index = atomic_inc_return(&vhost->trace_index) & IBMVFC_TRACE_INDEX_MASK;
entry = &vhost->trace[index];
+ memset(entry, 0, sizeof(*entry));
entry->evt = evt;
entry->time = jiffies;
entry->fmt = evt->crq.format;
@@ -294,15 +301,19 @@ static void ibmvfc_trc_end(struct ibmvfc_event *evt)
switch (entry->fmt) {
case IBMVFC_CMD_FORMAT:
- entry->op_code = iu->cdb[0];
entry->scsi_id = be64_to_cpu(vfc_cmd->tgt_scsi_id);
- entry->lun = scsilun_to_int(&iu->lun);
- entry->tmf_flags = iu->tmf_flags;
entry->u.end.status = be16_to_cpu(vfc_cmd->status);
entry->u.end.error = be16_to_cpu(vfc_cmd->error);
- entry->u.end.fcp_rsp_flags = rsp->flags;
- entry->u.end.rsp_code = rsp->data.info.rsp_code;
- entry->u.end.scsi_status = rsp->scsi_status;
+ if (evt->cmnd) {
+ entry->op_code = iu->cdb[0];
+ entry->lun = scsilun_to_int(&iu->lun);
+ entry->tmf_flags = iu->tmf_flags;
+ entry->u.end.fcp_rsp_flags = rsp->flags;
+ entry->u.end.rsp_code = rsp->data.info.rsp_code;
+ entry->u.end.scsi_status = rsp->scsi_status;
+ } else if (evt->fcp_req) {
+ entry->op_code = vfc_cmd->v3nvme.iu.sqe.common.opcode;
+ }
break;
case IBMVFC_MAD_FORMAT:
entry->op_code = be32_to_cpu(mad->opcode);
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 02/20] scsi: ibmvfc: fix trace logging for NVMe FCP commands
2026-09-19 1:32 ` [PATCH v2 02/20] scsi: ibmvfc: fix trace logging " Tyrel Datwyler
@ 2026-09-19 1:47 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-09-19 1:47 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: ibmvfc: UAF of nvmefc_fcp_req in ibmvfc_trc_start()
- [Medium] scsi: ibmvfc: trace data dropped for SCSI TMFs
--
commit d84c85cc97fa932f9084a1571f9d4d47d52e914d
Author: Tyrel Datwyler <tyreld@linux.ibm.com>
scsi: ibmvfc: fix trace logging for NVMe FCP commands
This commit fixes trace logging for NVMe FCP commands by gating SCSI-specific
field reads on evt->cmnd. It records the NVMe SQE opcode and payload length
from the v3nvme union member for NVMe FCP events.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index aa7ae81df41b1..6558ed3c67c4b 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> @@ -257,11 +258,16 @@ static void ibmvfc_trc_start(struct ibmvfc_event *evt)
>
> switch (entry->fmt) {
> case IBMVFC_CMD_FORMAT:
> - entry->op_code = iu->cdb[0];
> entry->scsi_id = be64_to_cpu(vfc_cmd->tgt_scsi_id);
> - entry->lun = scsilun_to_int(&iu->lun);
> - entry->tmf_flags = iu->tmf_flags;
> - entry->u.start.xfer_len = be32_to_cpu(iu->xfer_len);
> + if (evt->cmnd) {
> + entry->op_code = iu->cdb[0];
> + entry->lun = scsilun_to_int(&iu->lun);
> + entry->tmf_flags = iu->tmf_flags;
> + entry->u.start.xfer_len = be32_to_cpu(iu->xfer_len);
[Severity: Medium]
Will this drop trace data for SCSI Task Management Functions?
When an internally generated SCSI TMF is processed, evt->cmnd is initialized
to NULL. Since the extraction of LUN and TMF flags is now strictly gated on
evt->cmnd, TMFs will be skipped entirely leaving these trace fields zeroed out.
> + } else if (evt->fcp_req) {
> + entry->op_code = vfc_cmd->v3nvme.iu.sqe.common.opcode;
> + entry->u.start.xfer_len = evt->fcp_req->payload_length;
[Severity: High]
Can this result in a use-after-free on the NVMe FCP request?
The command has already been submitted to the hardware at this point. In
ibmvfc_send_event(), the queue lock is dropped before calling
ibmvfc_trc_start():
drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_send_event() {
...
spin_unlock_irqrestore(&evt->queue->l_lock, flags);
ibmvfc_trc_start(evt);
}
Because the lock is dropped, the hardware completion can race and process
the response, calling evt->done(). This hands the NVMe FCP request back to
the block layer where it is completed and freed. Meanwhile, this thread will
locklessly dereference evt->fcp_req->payload_length here, accessing the freed
memory.
> + }
> break;
> case IBMVFC_MAD_FORMAT:
[ ... ]
> @@ -294,15 +301,19 @@ static void ibmvfc_trc_end(struct ibmvfc_event *evt)
>
> switch (entry->fmt) {
> case IBMVFC_CMD_FORMAT:
> - entry->op_code = iu->cdb[0];
> entry->scsi_id = be64_to_cpu(vfc_cmd->tgt_scsi_id);
> - entry->lun = scsilun_to_int(&iu->lun);
> - entry->tmf_flags = iu->tmf_flags;
> entry->u.end.status = be16_to_cpu(vfc_cmd->status);
> entry->u.end.error = be16_to_cpu(vfc_cmd->error);
> - entry->u.end.fcp_rsp_flags = rsp->flags;
> - entry->u.end.rsp_code = rsp->data.info.rsp_code;
> - entry->u.end.scsi_status = rsp->scsi_status;
> + if (evt->cmnd) {
> + entry->op_code = iu->cdb[0];
> + entry->lun = scsilun_to_int(&iu->lun);
> + entry->tmf_flags = iu->tmf_flags;
> + entry->u.end.fcp_rsp_flags = rsp->flags;
> + entry->u.end.rsp_code = rsp->data.info.rsp_code;
> + entry->u.end.scsi_status = rsp->scsi_status;
[Severity: Medium]
Will this also drop trace data in the end trace for SCSI TMFs?
Similarly to the start trace, since evt->cmnd is NULL for SCSI TMFs, the
response code, FCP response flags, and SCSI status will be left unrecorded
for these events.
> + } else if (evt->fcp_req) {
> + entry->op_code = vfc_cmd->v3nvme.iu.sqe.common.opcode;
> + }
> break;
> case IBMVFC_MAD_FORMAT:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260919013306.2948028-1-tyreld@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 03/20] scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 01/20] scsi: ibmvfc: initialize evt->tgt for NVMe FCP commands Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 02/20] scsi: ibmvfc: fix trace logging " Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:44 ` sashiko-bot
2026-09-19 1:32 ` [PATCH v2 04/20] scsi: ibmvfc: defer NVMe local port registration out of atomic context Tyrel Datwyler
` (16 subsequent siblings)
19 siblings, 1 reply; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
When ibmvfc_send_event() returns H_CLOSED the event is freed without
notifying the NVMe-FC transport. The caller ibmvfc_nvme_fcp_io() returns
SCSI_MLQUEUE_HOST_BUSY back to the transport, which is a SCSI midlayer
concept the NVMe-FC transport does not interpret. The result is that the
NVMe-FC transport is left waiting on a request that was silently dropped,
causing the I/O to hang permanently.
Fix this by calling fcp_req->done() with a -EBUSY status before freeing
the event in the H_CLOSED path, consistent with how other FC-NVMe LLDDs
(e.g. lpfc) signal a retryable transport-level failure to the NVMe-FC
transport.
Fixes: 73c13e30c56f ("scsi: ibmvfc: implement nvme-fc IO command submission callback")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index 9ed6522c2e9e..08dd897b8e64 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
@@ -367,8 +367,12 @@ static int ibmvfc_nvme_fcp_io(struct nvme_fc_local_port *lport,
vfc_cmd->correlation = cpu_to_be64((u64)evt);
- if (likely(!(rc = ibmvfc_nvme_map_sg_data(fcp_req, evt, vfc_cmd))))
- return ibmvfc_send_event(evt, vhost, 0);
+ if (likely(!(rc = ibmvfc_nvme_map_sg_data(fcp_req, evt, vfc_cmd)))) {
+ rc = ibmvfc_send_event(evt, vhost, 0);
+ if (rc)
+ return -EBUSY;
+ return 0;
+ }
ibmvfc_free_event(evt);
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 03/20] scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure
2026-09-19 1:32 ` [PATCH v2 03/20] scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure Tyrel Datwyler
@ 2026-09-19 1:44 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-09-19 1:44 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: ibmvfc: misleading commit message contradicts safe implementation
--
commit a9d59d0a40253b9e7dff6fac485fc0f17bf38172
Author: Tyrel Datwyler <tyreld@linux.ibm.com>
scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure
This commit addresses a hang caused by silently dropped NVMe-FC requests
when ibmvfc_send_event() returns H_CLOSED. It changes the code to correctly
signal a retryable transport-level failure to the NVMe-FC transport.
> Fix this by calling fcp_req->done() with a -EBUSY status before freeing
> the event in the H_CLOSED path, consistent with how other FC-NVMe LLDDs
> (e.g. lpfc) signal a retryable transport-level failure to the NVMe-FC
> transport.
[ ... ]
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> index 9ed6522c2e9e3..08dd897b8e64e 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> @@ -367,8 +367,12 @@ static int ibmvfc_nvme_fcp_io(struct nvme_fc_local_port *lport,
>
> vfc_cmd->correlation = cpu_to_be64((u64)evt);
>
> - if (likely(!(rc = ibmvfc_nvme_map_sg_data(fcp_req, evt, vfc_cmd))))
> - return ibmvfc_send_event(evt, vhost, 0);
> + if (likely(!(rc = ibmvfc_nvme_map_sg_data(fcp_req, evt, vfc_cmd)))) {
> + rc = ibmvfc_send_event(evt, vhost, 0);
> + if (rc)
> + return -EBUSY;
[Severity: Medium]
Does the commit message accurately describe the implementation here?
The commit message explicitly claims the fix is implemented by "calling
fcp_req->done() with a -EBUSY status before freeing the event". However,
the code safely returns -EBUSY directly to the NVMe-FC transport without
invoking the done() callback.
If the implementation actually matched the commit message, it would have
caused a double-completion bug by calling done() while also failing the
submission API. The code appears correct by returning -EBUSY, but the
commit message misrepresents the implemented transport interaction.
Could the commit message be updated to match the code?
> + return 0;
> + }
>
> ibmvfc_free_event(evt);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260919013306.2948028-1-tyreld@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 04/20] scsi: ibmvfc: defer NVMe local port registration out of atomic context
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (2 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 03/20] scsi: ibmvfc: complete NVMe FCP requests on H_CLOSED send failure Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 05/20] scsi: ibmvfc: fix uninitialized _done dereference for TMF events on send failure Tyrel Datwyler
` (15 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_fabric_login_nvme_done() is invoked under vhost->host->host_lock
via the ibmvfc_locked_done() wrapper, which acquires the spinlock with
IRQs disabled before calling the MAD completion handler.
On the IBMVFC_MAD_SUCCESS path the handler called ibmvfc_nvme_register()
directly. That function calls nvme_fc_register_localport(), which
performs GFP_KERNEL memory allocations and may sleep — both illegal
inside a spinlock-held (atomic) context — resulting in a 'scheduling
while atomic' kernel panic during NVMe/FC fabric login.
Fix this by replacing the direct call with a new do_nvme_register flag
that mirrors the existing do_nvme_login pattern already used in the
driver. The flag is consumed in ibmvfc_do_work() under the
IBMVFC_HOST_ACTION_QUERY case, which runs in the kernel thread context
(ibmvfc_work) where sleeping allocations are safe. The lock is dropped
before calling ibmvfc_nvme_register() and re-acquired on return so the
rest of the QUERY case proceeds normally.
Fixes: 3831863f9f56 ("ibmvfc: register local nvme fc port after fabric login")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 12 +++++++++++-
drivers/scsi/ibmvscsi/ibmvfc.h | 1 +
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 6558ed3c67c4..728529155ded 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -991,6 +991,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
vhost->using_channels = 0;
vhost->do_scsi_login = 0;
vhost->do_nvme_login = 0;
+ vhost->do_nvme_register = 0;
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(&vhost->host->host_lock, flags);
@@ -1032,6 +1033,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
vhost->using_channels = 0;
vhost->do_scsi_login = 0;
vhost->do_nvme_login = 0;
+ vhost->do_nvme_register = 0;
/* Clean out the queue */
memset(crq->msgs.crq, 0, PAGE_SIZE);
@@ -5243,7 +5245,7 @@ static void ibmvfc_fabric_login_nvme_done(struct ibmvfc_event *evt)
switch (mad_status) {
case IBMVFC_MAD_SUCCESS:
fc_host_port_id(vhost->host) = be64_to_cpu(rsp->nport_id);
- ibmvfc_nvme_register(vhost);
+ vhost->do_nvme_register = 1;
ibmvfc_dbg(vhost, "NVMe fabric login succeeded\n");
break;
case IBMVFC_MAD_FAILED:
@@ -6070,6 +6072,14 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
vhost->job_step(vhost);
break;
case IBMVFC_HOST_ACTION_QUERY:
+ if (vhost->do_nvme_register) {
+ vhost->do_nvme_register = 0;
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ ibmvfc_nvme_register(vhost);
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ if (vhost->action != IBMVFC_HOST_ACTION_QUERY)
+ break;
+ }
list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue)
ibmvfc_init_tgt(tgt, ibmvfc_tgt_query_target);
list_for_each_entry(tgt, &vhost->nvme_scrqs.targets, queue)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index ca80ceffe53a..df0775183d72 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -1005,6 +1005,7 @@ struct ibmvfc_host {
unsigned int nvme_enabled:1;
unsigned int do_scsi_login:1;
unsigned int do_nvme_login:1;
+ unsigned int do_nvme_register:1;
unsigned int aborting_passthru:1;
unsigned int scan_complete:1;
int scan_timeout;
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 05/20] scsi: ibmvfc: fix uninitialized _done dereference for TMF events on send failure
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (3 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 04/20] scsi: ibmvfc: defer NVMe local port registration out of atomic context Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 06/20] scsi: ibmvfc: fix uninitialized shwqs in ibmvfc_purge_requests() Tyrel Datwyler
` (14 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
In ibmvfc_send_event(), the non-H_CLOSED error path uses a bare else
clause to handle the case where evt->cmnd is NULL, assuming the event
must be a MAD and reassigning evt->done = evt->_done before calling it.
However, SCSI Task Management Function (TMF) events created by
ibmvfc_init_tmf() are initialised with IBMVFC_CMD_FORMAT, not
IBMVFC_MAD_FORMAT. ibmvfc_init_event() only populates evt->_done for
IBMVFC_MAD_FORMAT events; for IBMVFC_CMD_FORMAT events evt->_done is
never set. Since TMF events also have evt->cmnd == NULL (cleared by
ibmvfc_init_event()), they fall through to the bare else branch,
copying the uninitialised evt->_done into evt->done and immediately
calling it — a wild function-pointer dereference that results in a
kernel panic during SCSI error recovery under SAN error conditions.
Fix this by replacing the bare else with
'else if (evt->crq.format == IBMVFC_MAD_FORMAT)', gating the _done
reassignment strictly on the MAD format where evt->_done is guaranteed
to be valid. TMF events (IBMVFC_CMD_FORMAT, cmnd==NULL) no longer reach
this branch; their evt->done (ibmvfc_locked_done wrapping
ibmvfc_sync_completion) remains correct as initialised, allowing the
waiting ibmvfc_cancel_all_sq/mq paths to receive the completion
normally.
Fixes: 848c70852e3f ("ibmvfc: split NVMe support into separate source file and add transport stubs")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 728529155ded..e4a21ca6815d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -1859,9 +1859,12 @@ int ibmvfc_send_event(struct ibmvfc_event *evt,
evt->done = ibmvfc_vfc_eh_done;
} else if (evt->fcp_req || evt->ls_req) {
evt->done = ibmvfc_vfc_eh_done;
- } else {
+ } else if (evt->crq.format == IBMVFC_MAD_FORMAT) {
evt->xfer_iu->mad_common.status = cpu_to_be16(IBMVFC_MAD_CRQ_ERROR);
evt->done = evt->_done;
+ } else {
+ evt->xfer_iu->cmd.status = cpu_to_be16(IBMVFC_VIOS_FAILURE);
+ evt->xfer_iu->cmd.error = cpu_to_be16(IBMVFC_CRQ_FAILURE);
}
evt->done(evt);
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 06/20] scsi: ibmvfc: fix uninitialized shwqs in ibmvfc_purge_requests()
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (4 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 05/20] scsi: ibmvfc: fix uninitialized _done dereference for TMF events on send failure Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 07/20] scsi: ibmvfc: fix uninitialized status logged on LS abort send failure Tyrel Datwyler
` (13 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
In ibmvfc_purge_requests() the variables shwqs and nhwqs are declared as:
int shwqs, nhwqs = 0;
This initialises only nhwqs to zero; shwqs is left uninitialised. Both
are assigned inside the 'if (vhost->using_channels)' block, so when
using_channels is false the block is skipped and shwqs retains its
garbage stack value. The subsequent loop
for (i = 0; i < shwqs; i++)
then iterates an arbitrary number of times over scsi_scrqs.scrqs[], which
may be NULL on the non-channel path, resulting in out-of-bounds heap
accesses and a kernel crash during any adapter reset or shutdown that
occurs before channels are established.
Fix by initialising shwqs to 0 in the declaration so that both loop
bounds are zero when using_channels is false and neither sub-queue loop
executes.
Fixes: 4857949b58cd ("ibmvfc: fail nvme-fc fcp-io and ls requests during transport reset")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index e4a21ca6815d..5ba8058991ef 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -1192,7 +1192,7 @@ static void ibmvfc_purge_requests(struct ibmvfc_host *vhost, int error_code)
struct ibmvfc_queue *scsi_q = vhost->scsi_scrqs.scrqs;
struct ibmvfc_queue *nvme_q = vhost->nvme_scrqs.scrqs;
unsigned long flags;
- int shwqs, nhwqs = 0;
+ int shwqs = 0, nhwqs = 0;
int i;
if (vhost->using_channels) {
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 07/20] scsi: ibmvfc: fix uninitialized status logged on LS abort send failure
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (5 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 06/20] scsi: ibmvfc: fix uninitialized shwqs in ibmvfc_purge_requests() Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 08/20] scsi: ibmvfc: fix inverted suppress-ABTS capability check in NVMe TMF path Tyrel Datwyler
` (12 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
In ibmvfc_nvme_ls_abort(), 'status' is declared without an initialiser.
It is only assigned at line 223 on the ibmvfc_send_event() success path:
wait_for_completion(&evt->comp);
status = be16_to_cpu(rsp.mad_common.status);
When ibmvfc_send_event() fails the code jumps via 'goto out', skipping
the assignment entirely. The ibmvfc_dbg() call that immediately follows
the out: label then reads uninitialised stack memory and logs a
meaningless status value.
Fix by initialising status to IBMVFC_MAD_CRQ_ERROR at its declaration.
This sentinel accurately reflects that the underlying CRQ send failed
and produces a meaningful log message on the error path.
Fixes: 20bec08f0208 ("ibmvfc: implement nvme-fc LS abort handling callback")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index 08dd897b8e64..0391fdb4642d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
@@ -202,7 +202,7 @@ static void ibmvfc_nvme_ls_abort(struct nvme_fc_local_port *lport,
struct ibmvfc_event *evt;
union ibmvfc_iu rsp;
unsigned long flags;
- u16 status;
+ u16 status = IBMVFC_MAD_CRQ_ERROR;
evt = ibmvfc_get_event(&vhost->crq);
if (!vhost->logged_in || !evt)
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 08/20] scsi: ibmvfc: fix inverted suppress-ABTS capability check in NVMe TMF path
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (6 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 07/20] scsi: ibmvfc: fix uninitialized status logged on LS abort send failure Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 09/20] scsi: ibmvfc: fix infinite reset loop on NULL evt in implicit logout path Tyrel Datwyler
` (11 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_nvme_setup_fcp_abort() sets the IBMVFC_TMF_SUPPRESS_ABTS flag
only when the adapter does NOT have the IBMVFC_CAN_SUPPRESS_ABTS
capability, which is the exact opposite of the intended behaviour.
Remove the erroneous '!' negation so the flag is set when the adapter
actually reports the capability, consistent with how the SCSI TMF path
in ibmvfc_cancel_all() handles the same check.
Fixes: 20bec08f0208 ("ibmvfc: implement nvme-fc LS abort handling callback")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index 0391fdb4642d..58e3e50d0c5c 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
@@ -184,7 +184,7 @@ static void ibmvfc_init_ls_abort(struct ibmvfc_event *evt, struct nvmefc_ls_req
tmf->common.opcode = cpu_to_be32(IBMVFC_NVMF_TMF_MAD);
tmf->common.length = cpu_to_be16(sizeof(*tmf));
if (vhost->state != IBMVFC_ACTIVE)
- if (!ibmvfc_check_caps(vhost, IBMVFC_CAN_SUPPRESS_ABTS))
+ if (ibmvfc_check_caps(vhost, IBMVFC_CAN_SUPPRESS_ABTS))
tmf->flags = cpu_to_be32(IBMVFC_TMF_SUPPRESS_ABTS);
tmf->cancel_key = cpu_to_be32((u64)abt_evt);
tmf->my_cancel_key = cpu_to_be32((u64)evt);
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 09/20] scsi: ibmvfc: fix infinite reset loop on NULL evt in implicit logout path
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (7 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 08/20] scsi: ibmvfc: fix inverted suppress-ABTS capability check in NVMe TMF path Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 10/20] scsi: ibmvfc: fix u16 overflow of max_cmds in ibmvfc_set_login_info() Tyrel Datwyler
` (10 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_tgt_implicit_logout_and_del() calls ibmvfc_set_tgt_action() with
IBMVFC_TGT_ACTION_NONE in the NULL evt error path. However the state
machine in ibmvfc_set_tgt_action() explicitly handles LOGOUT_RPORT and
only permits transitions to LOGOUT_RPORT_WAIT or DEL_RPORT from that
state; NONE is silently rejected and returns -EINVAL, leaving the target
stuck in LOGOUT_RPORT.
With the target permanently in LOGOUT_RPORT, ibmvfc_dev_logo_to_do()
keeps returning 1, causing the host to spin in an infinite reset loop.
Use IBMVFC_TGT_ACTION_DEL_RPORT instead, which is the same transition
taken by the !vhost->logged_in early-exit path in the same function and
is the correct valid transition to unblock target cleanup.
Fixes: 0122eab60b15 ("ibmvfc: check for NULL evt in implicit LOGO and target delete path")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 5ba8058991ef..70771f6420d9 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -4545,7 +4545,7 @@ static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *tgt)
if (!evt) {
vhost->discovery_threads--;
- ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);
+ ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_DEL_RPORT);
kref_put(&tgt->kref, ibmvfc_release_tgt);
__ibmvfc_reset_host(vhost);
return;
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 10/20] scsi: ibmvfc: fix u16 overflow of max_cmds in ibmvfc_set_login_info()
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (8 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 09/20] scsi: ibmvfc: fix infinite reset loop on NULL evt in implicit logout path Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 11/20] scsi: ibmvfc: fix UAF and hang in ibmvfc_cancel_all_mq() on send failure Tyrel Datwyler
` (9 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
max_cmds is declared as u16 but the calculation:
scsi_qdepth + IBMVFC_NUM_INTERNAL_REQ +
(scsi_qdepth + IBMVFC_NUM_INTERNAL_SUBQ_REQ) *
(scsi_scrqs.desired_queues + nvme_scrqs.desired_queues)
can exceed 65535 at non-extreme configurations. With scsi_qdepth=2048
and 32 total desired queues (16 SCSI + 16 NVMe maximum), the subqueue
term alone evaluates to 2052 * 32 = 65664, which silently wraps to 128
in a u16, causing the NPIV login MAD to advertise a badly corrupted
command slot count to the VIOS.
The wire field login_info->max_cmds is already __be32, so widen the
local variable to u32 to match, making the arithmetic safe across all
supported module parameter combinations.
Fixes: ecc03d958e37 ("ibmvfc: add logic for protocol specific fabric logins")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 70771f6420d9..623fff503040 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -1566,7 +1566,7 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost)
struct ibmvfc_queue *async_crq = &vhost->async_crq;
struct device_node *of_node = vhost->dev->of_node;
const char *location;
- u16 max_cmds;
+ u32 max_cmds;
max_cmds = scsi_qdepth + IBMVFC_NUM_INTERNAL_REQ;
if (mq_enabled)
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 11/20] scsi: ibmvfc: fix UAF and hang in ibmvfc_cancel_all_mq() on send failure
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (9 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 10/20] scsi: ibmvfc: fix u16 overflow of max_cmds in ibmvfc_set_login_info() Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 12/20] scsi: ibmvfc: fix data race on tgt->nvme_remote_port Tyrel Datwyler
` (8 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_cancel_all_mq() discards the return value of ibmvfc_send_event().
When ibmvfc_send_event() fails due to H_CLOSED it internally frees the
event before returning SCSI_MLQUEUE_HOST_BUSY. The caller then performs
list_add_tail() on the freed event — a use-after-free — and subsequently
blocks indefinitely on wait_for_completion(&evt->comp) for a completion
that will never arrive.
Fix by capturing the return value. On failure, drop the locks, log the
error (consistent with ibmvfc_cancel_all_sq()), drain and free any
cancel events already queued from earlier loop iterations, then return 0.
Returning 0 on send failure is correct: when the adapter closes the CRQ
the firmware delivers a transport event through ibmvfc_handle_crq() which
completes all outstanding commands, so the error recovery caller will
naturally see them return.
Fixes: 9c2aa65000f6 ("ibmvfc: don't call locked done variant for MADs on send failure")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 623fff503040..080312ff0a93 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -2696,6 +2696,7 @@ static int ibmvfc_cancel_all_mq(struct scsi_device *sdev, int type)
unsigned long flags;
int num_hwq, i;
int fail = 0;
+ int rc;
LIST_HEAD(cancelq);
u16 status;
@@ -2722,7 +2723,18 @@ static int ibmvfc_cancel_all_mq(struct scsi_device *sdev, int type)
return -ENOMEM;
}
evt->sync_iu = &queues[i].cancel_rsp;
- ibmvfc_send_event(evt, vhost, default_timeout);
+ rc = ibmvfc_send_event(evt, vhost, default_timeout);
+ if (rc) {
+ spin_unlock(queues[i].q_lock);
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ sdev_printk(KERN_ERR, sdev, "Failed to send cancel event. rc=%d\n", rc);
+ list_for_each_entry_safe(evt, temp, &cancelq, cancel) {
+ wait_for_completion(&evt->comp);
+ list_del(&evt->cancel);
+ ibmvfc_free_event(evt);
+ }
+ return 0;
+ }
list_add_tail(&evt->cancel, &cancelq);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 12/20] scsi: ibmvfc: fix data race on tgt->nvme_remote_port
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (10 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 11/20] scsi: ibmvfc: fix UAF and hang in ibmvfc_cancel_all_mq() on send failure Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:32 ` [PATCH v2 13/20] scsi: ibmvfc: make NVMe FCP abort callback asynchronous Tyrel Datwyler
` (7 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
tgt->nvme_remote_port is written in two places without holding host_lock:
1. ibmvfc_nvme_register_remoteport() calls nvme_fc_register_remoteport()
and stores the result directly into tgt->nvme_remote_port with no lock
held, racing against locked readers in ibmvfc_tgt_add_nvme_rport() and
ibmvfc_do_work().
2. The ibmvfc_nvme_remoteport_delete() callback writes
tgt->nvme_remote_port = NULL from the NVMe core's thread context with
no lock held, again racing against every locked reader of the field.
All readers of tgt->nvme_remote_port acquire host_lock before reading the
field, so both writers must do the same.
For ibmvfc_nvme_register_remoteport(): nvme_fc_register_remoteport() may
sleep so it must be called before acquiring the spinlock. Use a local
pointer to capture the result, then take host_lock and store the pointer
into tgt->nvme_remote_port and set ->private under the lock.
For ibmvfc_nvme_remoteport_delete(): acquire host_lock around the
tgt->nvme_remote_port = NULL store. The callback is always invoked after
ibmvfc_nvme_unregister_remoteport() drops host_lock before waiting for
completion, so taking the lock here is safe.
Fixes: 696d1cc2aaa2 ("scsi: ibmvfc: process NVMe/FC rports in work thread")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index 58e3e50d0c5c..a6fc9e8a35f6 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
@@ -26,8 +26,11 @@ static void ibmvfc_nvme_localport_delete(struct nvme_fc_local_port *lport)
static void ibmvfc_nvme_remoteport_delete(struct nvme_fc_remote_port *rport)
{
struct ibmvfc_target *tgt = rport->private;
+ unsigned long flags;
+ spin_lock_irqsave(&tgt->vhost->host->host_lock, flags);
tgt->nvme_remote_port = NULL;
+ spin_unlock_irqrestore(&tgt->vhost->host->host_lock, flags);
complete(&tgt->nvme_delete_done);
}
@@ -473,7 +476,9 @@ static struct nvme_fc_port_template ibmvfc_nvme_fc_transport = {
int ibmvfc_nvme_register_remoteport(struct ibmvfc_target *tgt)
{
struct ibmvfc_host *vhost = tgt->vhost;
+ struct nvme_fc_remote_port *rport;
struct nvme_fc_port_info pinfo;
+ unsigned long flags;
int rc;
if (!IS_ENABLED(CONFIG_NVME_FC))
@@ -490,14 +495,16 @@ int ibmvfc_nvme_register_remoteport(struct ibmvfc_target *tgt)
pinfo.port_id = tgt->ids.port_id;
pinfo.port_role = FC_PORT_ROLE_NVME_TARGET;
- rc = nvme_fc_register_remoteport(vhost->nvme_local_port, &pinfo,
- &tgt->nvme_remote_port);
+ rc = nvme_fc_register_remoteport(vhost->nvme_local_port, &pinfo, &rport);
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
if (!rc) {
ibmvfc_log(vhost, 2, "register_remoteport: traddr=nn-0x%llx:pn-0x%llx PortID:%x\n",
pinfo.node_name, pinfo.port_name, pinfo.port_id);
- tgt->nvme_remote_port->private = tgt;
+ rport->private = tgt;
+ tgt->nvme_remote_port = rport;
}
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
return rc;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 13/20] scsi: ibmvfc: make NVMe FCP abort callback asynchronous
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (11 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 12/20] scsi: ibmvfc: fix data race on tgt->nvme_remote_port Tyrel Datwyler
@ 2026-09-19 1:32 ` Tyrel Datwyler
2026-09-19 1:33 ` [PATCH v2 14/20] scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback Tyrel Datwyler
` (6 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:32 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
The nvme_fc_port_template fcp_abort callback is called by the NVMe-FC
transport from the block layer timeout workqueue. The current
implementation submits a cancel MAD and then blocks on
wait_for_completion() until the VIOS response arrives. This stalls the
timeout workqueue for the full MAD round-trip, preventing any other I/O
timeout from being processed in the meantime.
The nvme-fc-driver.h API contract is explicit: the LLDD must initiate
the abort and return immediately; the ABTS does not need to be complete
on return. The transport waits for the original exchange to complete
independently via fcp_req->done().
Replace the synchronous ibmvfc_sync_nvme_completion callback with a
dedicated ibmvfc_nvme_fcp_abort_done() that runs asynchronously when
the cancel MAD response arrives. The new callback logs any non-zero MAD
status, drops the target kref, and frees the event. ibmvfc_send_event()
guarantees the done callback is invoked on both success and failure
paths, so event ownership is fully transferred and ibmvfc_nvme_fcp_abort()
returns immediately after ibmvfc_send_event().
Drop the now-unnecessary init_completion() from ibmvfc_init_fcp_abort()
and the sync_iu / wait_for_completion / second lock-cycle from the abort
function itself.
Fixes: 4e70b8795ee3 ("scsi: ibmvfc: implement nvme-fc FCP abort callback")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 34 ++++++++++++-----------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index a6fc9e8a35f6..d23e5f31f8b5 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
@@ -400,8 +400,18 @@ static void ibmvfc_init_fcp_abort(struct ibmvfc_event *evt,
tmf->target_wwpn = cpu_to_be64(tgt->wwpn);
tmf->assoc_id = cpu_to_be64(tgt->assoc_id);
tmf->task_tag = cpu_to_be64((u64)abt_evt);
+}
- init_completion(&evt->comp);
+static void ibmvfc_nvme_fcp_abort_done(struct ibmvfc_event *evt)
+{
+ u16 status = be16_to_cpu(evt->xfer_iu->mad_common.status);
+
+ if (status)
+ ibmvfc_dbg(evt->vhost, "fcp_abort: cancel MAD failed with rc=%x\n",
+ status);
+
+ kref_put(&evt->tgt->kref, ibmvfc_release_tgt);
+ ibmvfc_free_event(evt);
}
static void ibmvfc_nvme_fcp_abort(struct nvme_fc_local_port *lport,
@@ -413,9 +423,7 @@ static void ibmvfc_nvme_fcp_abort(struct nvme_fc_local_port *lport,
struct ibmvfc_target *tgt = rport->private;
struct ibmvfc_event *evt, *abt_evt = abort_req->private;
struct ibmvfc_queue *queue;
- union ibmvfc_iu rsp;
unsigned long flags;
- u16 status = 0;
if (!abt_evt)
return;
@@ -430,27 +438,13 @@ static void ibmvfc_nvme_fcp_abort(struct nvme_fc_local_port *lport,
spin_lock_irqsave(queue->q_lock, flags);
kref_get(&tgt->kref);
- ibmvfc_init_event(evt, ibmvfc_sync_nvme_completion, IBMVFC_MAD_FORMAT);
+ ibmvfc_init_event(evt, ibmvfc_nvme_fcp_abort_done, IBMVFC_MAD_FORMAT);
ibmvfc_init_fcp_abort(evt, abort_req);
- evt->sync_iu = &rsp;
+ evt->tgt = tgt;
if (ibmvfc_send_event(evt, vhost, default_timeout))
- goto out;
-
- spin_unlock_irqrestore(queue->q_lock, flags);
-
- wait_for_completion(&evt->comp);
- status = be16_to_cpu(rsp.mad_common.status);
-
- spin_lock_irqsave(queue->q_lock, flags);
- ibmvfc_free_event(evt);
-out:
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
spin_unlock_irqrestore(queue->q_lock, flags);
-
- if (status)
- ibmvfc_dbg(vhost, "fcp_abort: cancel failed with rc=%x\n", status);
-
- kref_put(&tgt->kref, ibmvfc_release_tgt);
}
static struct nvme_fc_port_template ibmvfc_nvme_fc_transport = {
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 14/20] scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (12 preceding siblings ...)
2026-09-19 1:32 ` [PATCH v2 13/20] scsi: ibmvfc: make NVMe FCP abort callback asynchronous Tyrel Datwyler
@ 2026-09-19 1:33 ` Tyrel Datwyler
2026-09-19 1:53 ` sashiko-bot
2026-09-19 1:33 ` [PATCH v2 15/20] scsi: ibmvfc: unregister NVMe local port on adapter removal Tyrel Datwyler
` (5 subsequent siblings)
19 siblings, 1 reply; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:33 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
Two problems in ibmvfc_nvme_ls_abort():
1. Use-after-free / stale pointer dereference.
ibmvfc_init_ls_abort() reads abt_evt = ls_abort->private and
immediately dereferences abt_evt->tgt. ibmvfc_ls_req_done() calls
ibmvfc_free_event() under host_lock, which returns the event slot to
the pool. If the LS completes naturally just before ls_abort is
called, abt_evt points to a freed (and potentially reused) event,
making the dereference a UAF.
Fix by taking host_lock before reading ls_abort->private and checking
evt->free (set to 1 by ibmvfc_free_event() under host_lock) to detect
whether the original LS has already completed. If so, there is
nothing to cancel and we return early. ibmvfc_get_event() is also
moved inside the lock so the validity check and event allocation are
atomic with respect to the completion path.
2. Blocking wait on timeout workqueue (same class as the FCP abort fix).
The original code called wait_for_completion() from ls_abort, which
is invoked by the NVMe-FC transport from a context that must not
block.
Fix by replacing ibmvfc_sync_nvme_completion with a dedicated async
callback ibmvfc_nvme_ls_abort_done() that logs any non-zero MAD
status, drops the target kref, and frees the event.
ibmvfc_send_event() guarantees the callback fires on both success and
failure paths, so ibmvfc_nvme_ls_abort() returns immediately after
ibmvfc_send_event().
ibmvfc_sync_nvme_completion is now unused and is removed.
ibmvfc_init_ls_abort() is updated to take the validated abt_evt pointer
directly instead of deriving it from ls_abort->private.
Fixes: 20bec08f0208 ("scsi: ibmvfc: implement nvme-fc LS abort handling callback")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 68 +++++++++++++++--------------
1 file changed, 36 insertions(+), 32 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index d23e5f31f8b5..475177cda103 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
@@ -103,6 +103,7 @@ static void ibmvfc_ls_req_done(struct ibmvfc_event *evt)
rc = -EIO;
evt->ls_req->done(evt->ls_req, rc);
+ evt->ls_req = NULL;
kref_put(&tgt->kref, ibmvfc_release_tgt);
ibmvfc_free_event(evt);
@@ -164,21 +165,12 @@ static int ibmvfc_nvme_ls_req(struct nvme_fc_local_port *lport,
return 0;
}
-static void ibmvfc_sync_nvme_completion(struct ibmvfc_event *evt)
+static void ibmvfc_init_ls_abort(struct ibmvfc_event *evt,
+ struct ibmvfc_event *abt_evt)
{
- /* copy the response back */
- if (evt->sync_iu)
- *evt->sync_iu = *evt->xfer_iu;
-
- complete(&evt->comp);
-}
-
-static void ibmvfc_init_ls_abort(struct ibmvfc_event *evt, struct nvmefc_ls_req *ls_abort)
-{
- struct ibmvfc_tmf *tmf;
- struct ibmvfc_event *abt_evt = ls_abort->private;
struct ibmvfc_target *tgt = abt_evt->tgt;
struct ibmvfc_host *vhost = evt->vhost;
+ struct ibmvfc_tmf *tmf;
tmf = &evt->iu.tmf;
memset(tmf, 0, sizeof(*tmf));
@@ -192,8 +184,18 @@ static void ibmvfc_init_ls_abort(struct ibmvfc_event *evt, struct nvmefc_ls_req
tmf->cancel_key = cpu_to_be32((u64)abt_evt);
tmf->my_cancel_key = cpu_to_be32((u64)evt);
tmf->assoc_id = cpu_to_be64(tgt->assoc_id);
+}
+
+static void ibmvfc_nvme_ls_abort_done(struct ibmvfc_event *evt)
+{
+ u16 status = be16_to_cpu(evt->xfer_iu->mad_common.status);
+
+ if (status)
+ ibmvfc_dbg(evt->vhost, "ls_abort: cancel MAD failed with rc=%x\n",
+ status);
- init_completion(&evt->comp);
+ kref_put(&evt->tgt->kref, ibmvfc_release_tgt);
+ ibmvfc_free_event(evt);
}
static void ibmvfc_nvme_ls_abort(struct nvme_fc_local_port *lport,
@@ -202,34 +204,36 @@ static void ibmvfc_nvme_ls_abort(struct nvme_fc_local_port *lport,
{
struct ibmvfc_host *vhost = lport->private;
struct ibmvfc_target *tgt = rport->private;
- struct ibmvfc_event *evt;
- union ibmvfc_iu rsp;
+ struct ibmvfc_event *evt, *abt_evt;
unsigned long flags;
- u16 status = IBMVFC_MAD_CRQ_ERROR;
+
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+
+ /*
+ * If the original LS has already completed naturally, abt_evt will
+ * have been freed back to the pool (evt->free set to 1 under
+ * host_lock by ibmvfc_free_event()). Nothing left to cancel.
+ */
+ abt_evt = ls_abort->private;
+ if (!abt_evt || atomic_read(&abt_evt->free) || abt_evt->ls_req != ls_abort) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ return;
+ }
evt = ibmvfc_get_event(&vhost->crq);
- if (!vhost->logged_in || !evt)
+ if (!vhost->logged_in || !evt) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
return;
+ }
- spin_lock_irqsave(&vhost->host->host_lock, flags);
kref_get(&tgt->kref);
- ibmvfc_init_event(evt, ibmvfc_sync_nvme_completion, IBMVFC_MAD_FORMAT);
- ibmvfc_init_ls_abort(evt, ls_abort);
- evt->sync_iu = &rsp;
+ ibmvfc_init_event(evt, ibmvfc_nvme_ls_abort_done, IBMVFC_MAD_FORMAT);
+ ibmvfc_init_ls_abort(evt, abt_evt);
+ evt->tgt = tgt;
if (ibmvfc_send_event(evt, vhost, default_timeout))
- goto out;
-
- spin_unlock_irqrestore(&vhost->host->host_lock, flags);
-
- wait_for_completion(&evt->comp);
- status = be16_to_cpu(rsp.mad_common.status);
- spin_lock_irqsave(&vhost->host->host_lock, flags);
- ibmvfc_free_event(evt);
-out:
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
spin_unlock_irqrestore(&vhost->host->host_lock, flags);
- ibmvfc_dbg(vhost, "ls_abort: cancel failed with rc=%x\n", status);
- kref_put(&tgt->kref, ibmvfc_release_tgt);
}
static void ibmvfc_nvme_done(struct ibmvfc_event *evt)
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 14/20] scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback
2026-09-19 1:33 ` [PATCH v2 14/20] scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback Tyrel Datwyler
@ 2026-09-19 1:53 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-09-19 1:53 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: ls_abort validity check defeated by stale ls_req
--
commit 0d745a9ce0a68c1aa95c0283c60b12feb6ac43d9
Author: Tyrel Datwyler <tyreld@linux.ibm.com>
scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback
This commit fixes a use-after-free and a blocking wait in the NVMe LS abort
callback. It moves event validation inside the host_lock and replaces the
synchronous wait with an asynchronous callback.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> index d23e5f31f8b51..475177cda103a 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
[ ... ]
> @@ -202,34 +204,36 @@ static void ibmvfc_nvme_ls_abort(struct nvme_fc_local_port *lport,
> {
> struct ibmvfc_host *vhost = lport->private;
> struct ibmvfc_target *tgt = rport->private;
> - struct ibmvfc_event *evt;
> - union ibmvfc_iu rsp;
> + struct ibmvfc_event *evt, *abt_evt;
> unsigned long flags;
> - u16 status = IBMVFC_MAD_CRQ_ERROR;
> +
> + spin_lock_irqsave(&vhost->host->host_lock, flags);
> +
> + /*
> + * If the original LS has already completed naturally, abt_evt will
> + * have been freed back to the pool (evt->free set to 1 under
> + * host_lock by ibmvfc_free_event()). Nothing left to cancel.
> + */
> + abt_evt = ls_abort->private;
> + if (!abt_evt || atomic_read(&abt_evt->free) || abt_evt->ls_req != ls_abort) {
[Severity: High]
If an LS request fails during send, ibmvfc_vfc_eh_done() frees the event without
clearing evt->ls_req:
drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_vfc_eh_done() {
...
} else if (ls_req) {
ls_req->done(ls_req, -EIO);
kref_put(&evt->tgt->kref, ibmvfc_release_tgt);
}
ibmvfc_free_event(evt);
}
If this event is subsequently reallocated for a new command, evt->free
becomes 0 but evt->ls_req remains stale.
If the NVMe core then calls ibmvfc_nvme_ls_abort() concurrently
(before the new command calls ibmvfc_init_event()), wouldn't this
validation bypass the check since abt_evt->ls_req still matches
ls_abort?
If the check is bypassed, the abort path will dereference abt_evt->tgt
which was already released by the error path, potentially causing a
use-after-free or sending an unintended abort MAD for an active command.
> + spin_unlock_irqrestore(&vhost->host->host_lock, flags);
> + return;
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260919013306.2948028-1-tyreld@linux.ibm.com?part=14
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 15/20] scsi: ibmvfc: unregister NVMe local port on adapter removal
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (13 preceding siblings ...)
2026-09-19 1:33 ` [PATCH v2 14/20] scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback Tyrel Datwyler
@ 2026-09-19 1:33 ` Tyrel Datwyler
2026-09-19 1:58 ` sashiko-bot
2026-09-19 1:33 ` [PATCH v2 16/20] scsi: ibmvfc: fix NVMe local port leak on fabric link bounce Tyrel Datwyler
` (4 subsequent siblings)
19 siblings, 1 reply; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:33 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_remove() tears down the CRQ and frees all host memory but never
calls ibmvfc_nvme_unregister(). The NVMe-FC transport therefore retains
a live reference to the local port whose private pointer points into the
now-freed ibmvfc_host, leading to a use-after-free whenever the transport
subsequently touches the port (queue creation, port scanning, etc.).
ibmvfc_nvme_unregister() is already called from the NPIV logout and host
reset paths during normal operation; driver removal simply missed it.
Add the call after ibmvfc_release_crq_queue() — at which point the CRQ
is torn down so no further completions can arrive — and before
ibmvfc_free_mem(), so host memory is still valid while
nvme_fc_unregister_localport() runs and waits for the
localport_delete callback to complete.
Fixes: 86e495358096 ("scsi: ibmvfc: implement LLDD callbacks for mapping nvme-fc queues")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 080312ff0a93..e68ed5e79e58 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -6976,6 +6976,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
static void ibmvfc_remove(struct vio_dev *vdev)
{
struct ibmvfc_host *vhost = dev_get_drvdata(&vdev->dev);
+ struct ibmvfc_target *tgt, *tgt_tmp;
LIST_HEAD(purge);
unsigned long flags;
@@ -6992,6 +6993,22 @@ static void ibmvfc_remove(struct vio_dev *vdev)
fc_remove_host(vhost->host);
scsi_remove_host(vhost->host);
+ list_for_each_entry_safe(tgt, tgt_tmp, &vhost->nvme_scrqs.targets, queue) {
+ if (tgt->nvme_remote_port)
+ ibmvfc_nvme_unregister_remoteport(tgt);
+ list_del(&tgt->queue);
+ timer_delete_sync(&tgt->timer);
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
+ }
+
+ list_for_each_entry_safe(tgt, tgt_tmp, &vhost->scsi_scrqs.targets, queue) {
+ if (tgt->rport)
+ fc_remote_port_delete(tgt->rport);
+ list_del(&tgt->queue);
+ timer_delete_sync(&tgt->timer);
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
+ }
+
spin_lock_irqsave(&vhost->host->host_lock, flags);
ibmvfc_purge_requests(vhost, DID_ERROR);
list_splice_init(&vhost->purge, &purge);
@@ -7000,6 +7017,8 @@ static void ibmvfc_remove(struct vio_dev *vdev)
ibmvfc_release_sub_crqs(vhost);
ibmvfc_release_crq_queue(vhost);
+ ibmvfc_nvme_unregister(vhost);
+
ibmvfc_free_mem(vhost);
spin_lock(&ibmvfc_driver_lock);
list_del(&vhost->queue);
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 15/20] scsi: ibmvfc: unregister NVMe local port on adapter removal
2026-09-19 1:33 ` [PATCH v2 15/20] scsi: ibmvfc: unregister NVMe local port on adapter removal Tyrel Datwyler
@ 2026-09-19 1:58 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-09-19 1:58 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: ibmvfc: lockless list modification in ibmvfc_remove() races with tasklet
- [High] scsi: ibmvfc: use-after-free on rport in ibmvfc_remove()
--
commit 64ca41cc44b400a863417867ee490f75b432ff92
Author: Tyrel Datwyler <tyreld@linux.ibm.com>
scsi: ibmvfc: unregister NVMe local port on adapter removal
This commit ensures the NVMe local port is unregistered during adapter removal
to prevent a use-after-free when the transport subsequently touches the port.
It does this by adding teardown calls for targets and the local port before
host memory is freed.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 080312ff0a93d..e68ed5e79e58c 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> @@ -6976,6 +6976,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> static void ibmvfc_remove(struct vio_dev *vdev)
> {
> struct ibmvfc_host *vhost = dev_get_drvdata(&vdev->dev);
> + struct ibmvfc_target *tgt, *tgt_tmp;
> LIST_HEAD(purge);
> unsigned long flags;
>
[ ... ]
> @@ -6992,6 +6993,22 @@ static void ibmvfc_remove(struct vio_dev *vdev)
> fc_remove_host(vhost->host);
> scsi_remove_host(vhost->host);
>
> + list_for_each_entry_safe(tgt, tgt_tmp, &vhost->nvme_scrqs.targets, queue) {
> + if (tgt->nvme_remote_port)
> + ibmvfc_nvme_unregister_remoteport(tgt);
> + list_del(&tgt->queue);
[Severity: High]
Does this list_del() race with concurrent list traversals in the tasklet?
Because this loop modifies the list locklessly without holding vhost->host_lock,
and the CRQ is not torn down until ibmvfc_release_crq_queue() is called later
in this function, an incoming asynchronous event (like IBMVFC_AE_ELS_LOGO)
could trigger the ibmvfc_tasklet.
The tasklet handles the event by locking host_lock and traversing the same list:
drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_handle_async() {
...
list_for_each_entry(tgt, &vhost->nvme_scrqs.targets, queue) {
...
}
Will this concurrent lockless modification lead to list corruption and a kernel
panic during the traversal?
> + timer_delete_sync(&tgt->timer);
> + kref_put(&tgt->kref, ibmvfc_release_tgt);
> + }
> +
> + list_for_each_entry_safe(tgt, tgt_tmp, &vhost->scsi_scrqs.targets, queue) {
> + if (tgt->rport)
> + fc_remote_port_delete(tgt->rport);
[Severity: High]
Is it possible this dereferences a freed rport?
Right before this loop, fc_remove_host(vhost->host) and
scsi_remove_host(vhost->host) are called. fc_remove_host() synchronously
deletes all remote ports, and scsi_remove_host() removes all SCSI targets,
dropping their final references to the rport structures and causing them to
be freed.
Will dereferencing tgt->rport here (such as reading rport->dev_loss_tmo inside
fc_remote_port_delete) result in a use-after-free?
> + list_del(&tgt->queue);
> + timer_delete_sync(&tgt->timer);
> + kref_put(&tgt->kref, ibmvfc_release_tgt);
> + }
> +
> spin_lock_irqsave(&vhost->host->host_lock, flags);
> ibmvfc_purge_requests(vhost, DID_ERROR);
> list_splice_init(&vhost->purge, &purge);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260919013306.2948028-1-tyreld@linux.ibm.com?part=15
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 16/20] scsi: ibmvfc: fix NVMe local port leak on fabric link bounce
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (14 preceding siblings ...)
2026-09-19 1:33 ` [PATCH v2 15/20] scsi: ibmvfc: unregister NVMe local port on adapter removal Tyrel Datwyler
@ 2026-09-19 1:33 ` Tyrel Datwyler
2026-09-19 1:33 ` [PATCH v2 17/20] scsi: ibmvfc: fix TOCTOU race in ibmvfc_nvme_create_queue() on adapter removal Tyrel Datwyler
` (3 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:33 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
When the fabric link goes down, ibmvfc_link_down() does not call
ibmvfc_nvme_unregister(), so vhost->nvme_local_port remains set across
the link bounce. When the adapter re-initializes and completes fabric
login, ibmvfc_nvme_register() is called again unconditionally.
nvme_fc_register_localport() finds the still-live port in the transport's
list (FC_OBJSTATE_ONLINE) and returns -EEXIST, writing NULL into
*portptr. The original local port is now orphaned in the transport with
no way to reach it, while vhost->nvme_local_port is NULL, leaving NVMe
functionality permanently broken for the lifetime of the adapter.
Fix by unregistering any existing local port at the top of
ibmvfc_nvme_register() before attempting to register a new one.
ibmvfc_nvme_register() is called from ibmvfc_do_work() with host_lock
dropped, so ibmvfc_nvme_unregister()'s wait_for_completion() is safe
there. This makes ibmvfc_nvme_register() idempotent across link bounces
without requiring changes to the link-down path.
Fixes: 3831863f9f56 ("scsi: ibmvfc: register local nvme fc port after fabric login")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-nvme.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
index 475177cda103..89ad7680beb4 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
@@ -537,6 +537,10 @@ int ibmvfc_nvme_register(struct ibmvfc_host *vhost)
if (!IS_ENABLED(CONFIG_NVME_FC))
return 0;
+ /* Unregister any stale local port left from a previous link-up */
+ if (vhost->nvme_local_port)
+ ibmvfc_nvme_unregister(vhost);
+
pinfo.node_name = fc_host_node_name(vhost->host);
pinfo.port_name = fc_host_port_name(vhost->host);
pinfo.port_id = fc_host_port_id(vhost->host);
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 17/20] scsi: ibmvfc: fix TOCTOU race in ibmvfc_nvme_create_queue() on adapter removal
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (15 preceding siblings ...)
2026-09-19 1:33 ` [PATCH v2 16/20] scsi: ibmvfc: fix NVMe local port leak on fabric link bounce Tyrel Datwyler
@ 2026-09-19 1:33 ` Tyrel Datwyler
2026-09-19 1:33 ` [PATCH v2 18/20] scsi: ibmvfc: fix NVMe sub-queue registration failure disabling SCSI multiqueue Tyrel Datwyler
` (2 subsequent siblings)
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:33 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_remove() called ibmvfc_release_sub_crqs() before
ibmvfc_nvme_unregister(). ibmvfc_release_sub_crqs() frees the
nvme_scrqs.scrqs array and sets active_queues to 0, but the nvme-fc
transport is still live at that point and can call back into
ibmvfc_nvme_create_queue(). That callback reads active_queues and
dereferences scrqs without any lock, so the window between the guard
check and the actual use can race with the teardown, leading to:
- divide-by-zero in '(qidx - 1) % vhost->nvme_scrqs.active_queues'
when active_queues has been zeroed concurrently, and
- NULL pointer dereference on 'vhost->nvme_scrqs.scrqs[qhandle->index]'
when scrqs has been freed and set to NULL concurrently.
ibmvfc_nvme_unregister() already calls nvme_fc_unregister_localport()
followed by wait_for_completion(), which blocks until the transport has
fully quiesced all queue operations. It just needs to happen before the
sub-CRQ memory is released.
Move ibmvfc_nvme_unregister() before ibmvfc_release_sub_crqs() in
ibmvfc_remove() so the transport is quiesced before the sub-CRQ arrays
are freed. The reset path (ibmvfc_reset_crq) is unaffected: it only
deregisters sub-CRQs at the hypervisor level and never frees them, so
active_queues and scrqs remain valid across a reset.
Fixes: 86e495358096 ("scsi: ibmvfc: implement LLDD callbacks for mapping nvme-fc queues")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index e68ed5e79e58..701a9ebf2b7f 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -7014,11 +7014,10 @@ static void ibmvfc_remove(struct vio_dev *vdev)
list_splice_init(&vhost->purge, &purge);
spin_unlock_irqrestore(&vhost->host->host_lock, flags);
ibmvfc_complete_purge(&purge);
+ ibmvfc_nvme_unregister(vhost);
ibmvfc_release_sub_crqs(vhost);
ibmvfc_release_crq_queue(vhost);
- ibmvfc_nvme_unregister(vhost);
-
ibmvfc_free_mem(vhost);
spin_lock(&ibmvfc_driver_lock);
list_del(&vhost->queue);
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 18/20] scsi: ibmvfc: fix NVMe sub-queue registration failure disabling SCSI multiqueue
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (16 preceding siblings ...)
2026-09-19 1:33 ` [PATCH v2 17/20] scsi: ibmvfc: fix TOCTOU race in ibmvfc_nvme_create_queue() on adapter removal Tyrel Datwyler
@ 2026-09-19 1:33 ` Tyrel Datwyler
2026-09-19 1:33 ` [PATCH v2 19/20] scsi: ibmvfc: fix nr_nvme_hw_queues module parameter ignored for NVMe queue sizing Tyrel Datwyler
2026-09-19 1:33 ` [PATCH v2 20/20] scsi: ibmvfc: fix concurrent SCSI and NVMe discover-targets race dropping targets Tyrel Datwyler
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:33 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
ibmvfc_reg_sub_crqs() clears vhost->do_enquiry unconditionally when any
channel registration fails. The function is called for SCSI channels
first and then for NVMe channels (in ibmvfc_init_sub_crqs(),
ibmvfc_reset_crq(), and ibmvfc_reenable_crq_queue()). If the NVMe
channel registration fails, do_enquiry is cleared even though SCSI
channels registered successfully, preventing the IBMVFC_CHANNEL_ENQUIRY
MAD from being sent and silently disabling multiqueue support for SCSI.
ibmvfc_init_sub_crqs() already handles NVMe *allocation* failure
correctly — it only clears nvme_enabled without touching do_enquiry.
The same logic needs to apply when NVMe *registration* fails.
Fix ibmvfc_reg_sub_crqs() to check channels->protocol in the failure
path: clear do_enquiry only for a SCSI channel failure (preserving
existing behaviour), and clear nvme_enabled instead for an NVMe channel
failure. This covers all three call sites in one place.
Fixes: 319f6545a2d4 ("scsi: ibmvfc: alloc/dealloc sub-queues for nvme channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 701a9ebf2b7f..914dafd576a0 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -992,6 +992,8 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
vhost->do_scsi_login = 0;
vhost->do_nvme_login = 0;
vhost->do_nvme_register = 0;
+ if (vhost->nvme_scrqs.scrqs)
+ vhost->nvme_enabled = 1;
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(&vhost->host->host_lock, flags);
@@ -1034,6 +1036,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
vhost->do_scsi_login = 0;
vhost->do_nvme_login = 0;
vhost->do_nvme_register = 0;
+ if (vhost->nvme_scrqs.scrqs)
+ vhost->nvme_enabled = 1;
/* Clean out the queue */
memset(crq->msgs.crq, 0, PAGE_SIZE);
@@ -6532,7 +6536,10 @@ static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
if (ibmvfc_register_channel(vhost, channels, i)) {
for (j = i; j > 0; j--)
ibmvfc_deregister_channel(vhost, channels, j - 1);
- vhost->do_enquiry = 0;
+ if (channels->protocol == IBMVFC_PROTO_SCSI)
+ vhost->do_enquiry = 0;
+ else
+ vhost->nvme_enabled = 0;
return;
}
}
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 19/20] scsi: ibmvfc: fix nr_nvme_hw_queues module parameter ignored for NVMe queue sizing
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (17 preceding siblings ...)
2026-09-19 1:33 ` [PATCH v2 18/20] scsi: ibmvfc: fix NVMe sub-queue registration failure disabling SCSI multiqueue Tyrel Datwyler
@ 2026-09-19 1:33 ` Tyrel Datwyler
2026-09-19 1:33 ` [PATCH v2 20/20] scsi: ibmvfc: fix concurrent SCSI and NVMe discover-targets race dropping targets Tyrel Datwyler
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:33 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
max_nvme_queues was computed as min(IBMVFC_MAX_NVME_QUEUES, online_cpus),
silently ignoring the nr_nvme_hw_queues module parameter. The SCSI path
correctly caps shost->nr_hw_queues with nr_scsi_hw_queues, but the NVMe
equivalent was never applied, making the 'nvme_host_queues' parameter a
no-op.
Apply the same pattern as the SCSI side: incorporate nr_nvme_hw_queues
into the max_nvme_queues calculation at probe time so that
nvme_scrqs.max_queues and nvme_scrqs.desired_queues both respect the
administrator-provided limit.
Fixes: 018fc3965291 ("scsi: ibmvfc: initialize NVMe channel configuration during driver probe")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 914dafd576a0..ca9a0437fc0c 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -6871,7 +6871,9 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
int rc = -ENOMEM;
unsigned int online_cpus = num_online_cpus();
unsigned int max_scsi_queues = min_t(unsigned int, IBMVFC_MAX_SCSI_QUEUES, online_cpus);
- unsigned int max_nvme_queues = min_t(unsigned int, IBMVFC_MAX_NVME_QUEUES, online_cpus);
+ unsigned int max_nvme_queues = min_t(unsigned int,
+ min(IBMVFC_MAX_NVME_QUEUES, nr_nvme_hw_queues),
+ online_cpus);
ENTER;
shost = scsi_host_alloc(&driver_template, sizeof(*vhost));
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 20/20] scsi: ibmvfc: fix concurrent SCSI and NVMe discover-targets race dropping targets
2026-09-19 1:32 [PATCH v2 00/20] scsi: ibmvfc: Fixes and cleanup for NVMe/FC support Tyrel Datwyler
` (18 preceding siblings ...)
2026-09-19 1:33 ` [PATCH v2 19/20] scsi: ibmvfc: fix nr_nvme_hw_queues module parameter ignored for NVMe queue sizing Tyrel Datwyler
@ 2026-09-19 1:33 ` Tyrel Datwyler
19 siblings, 0 replies; 26+ messages in thread
From: Tyrel Datwyler @ 2026-09-19 1:33 UTC (permalink / raw)
To: james.bottomley, martin.petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, brking, davemarq,
Tyrel Datwyler
Commit 5e9dd03726ad ("scsi: ibmvfc: send NVMe target discovery MAD")
introduced concurrent SCSI and NVMe discover-target MADs.
ibmvfc_discover_targets_done() unconditionally set
IBMVFC_HOST_ACTION_ALLOC_TGTS on the first completion to arrive.
ibmvfc_alloc_targets() ran immediately and read both channels'
disc_buf and num_targets fields while the slower protocol's MAD was
still in flight. The slower protocol's num_targets remained zero from
the previous cycle, silently dropping all targets for that protocol.
The second completion attempted to transition to ALLOC_TGTS again but
the state machine had already advanced, so the transition was a no-op
and ibmvfc_alloc_targets() was never re-run.
Fix this by serializing discovery: send the SCSI discover-targets MAD
first and wait for its completion before sending the NVMe MAD.
ibmvfc_discover_scsi_targets_done() advances the job_step to
ibmvfc_discover_nvme_targets() when NVMe is active, or transitions
directly to IBMVFC_HOST_ACTION_ALLOC_TGTS when it is not.
ibmvfc_discover_nvme_targets_done() then transitions to ALLOC_TGTS
after the NVMe buffer is fully written. This guarantees that
ibmvfc_alloc_targets() is only called after both discovery buffers
are complete with no shared counter and no concurrent MADs in flight.
Fixes: 5e9dd03726ad ("scsi: ibmvfc: send NVMe target discovery MAD")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 115 +++++++++++++++++++---------
1 file changed, 79 insertions(+), 36 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index ca9a0437fc0c..46d6b94cb96d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -184,6 +184,7 @@ static void ibmvfc_tgt_send_prli(struct ibmvfc_target *);
static void ibmvfc_tgt_send_plogi(struct ibmvfc_target *);
static void ibmvfc_tgt_query_target(struct ibmvfc_target *);
static void ibmvfc_npiv_logout(struct ibmvfc_host *);
+static void ibmvfc_discover_nvme_targets(struct ibmvfc_host *);
static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
@@ -5137,45 +5138,80 @@ static int ibmvfc_alloc_targets(struct ibmvfc_host *vhost)
}
/**
- * ibmvfc_discover_targets_done - Completion handler for discover targets MAD
+ * ibmvfc_discover_scsi_targets_done - Completion handler for SCSI discover targets MAD
* @evt: ibmvfc event struct
*
**/
-static void ibmvfc_discover_targets_done(struct ibmvfc_event *evt)
+static void ibmvfc_discover_scsi_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 SCSI Targets succeeded\n");
+ vhost->scsi_scrqs.num_targets = min_t(u32, be32_to_cpu(rsp->num_written),
+ max_targets);
+ ibmvfc_dbg(vhost, "%d SCSI targets found\n",
+ vhost->scsi_scrqs.num_targets);
+ if (ibmvfc_nvme_active(vhost)) {
+ ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT);
+ vhost->job_step = ibmvfc_discover_nvme_targets;
+ } else {
+ ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_ALLOC_TGTS);
+ }
+ break;
+ case IBMVFC_MAD_FAILED:
+ level += ibmvfc_retry_host_init(vhost);
+ ibmvfc_log(vhost, level, "Discover SCSI Targets failed: %s (%x:%x)\n",
+ ibmvfc_get_cmd_error(be16_to_cpu(rsp->status), be16_to_cpu(rsp->error)),
+ be16_to_cpu(rsp->status), be16_to_cpu(rsp->error));
+ break;
+ case IBMVFC_MAD_DRIVER_FAILED:
+ break;
+ default:
+ dev_err(vhost->dev, "Invalid Discover SCSI Targets response: 0x%x\n", mad_status);
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+ break;
+ }
+
+ ibmvfc_free_event(evt);
+ wake_up(&vhost->work_wait_q);
+}
+
+/**
+ * ibmvfc_discover_nvme_targets_done - Completion handler for NVMe discover targets MAD
+ * @evt: ibmvfc event struct
+ *
+ **/
+static void ibmvfc_discover_nvme_targets_done(struct ibmvfc_event *evt)
+{
+ struct ibmvfc_host *vhost = evt->vhost;
+ struct ibmvfc_discover_targets *rsp = &evt->xfer_iu->discover_targets;
+ u32 mad_status = be16_to_cpu(rsp->common.status);
+ int level = IBMVFC_DEFAULT_LOG_LEVEL;
switch (mad_status) {
case IBMVFC_MAD_SUCCESS:
- 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_dbg(vhost, "Discover NVMe Targets succeeded\n");
+ vhost->nvme_scrqs.num_targets = min_t(u32, be32_to_cpu(rsp->num_written),
+ max_targets);
+ ibmvfc_dbg(vhost, "%d NVMe targets found\n",
+ vhost->nvme_scrqs.num_targets);
ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_ALLOC_TGTS);
break;
case IBMVFC_MAD_FAILED:
level += ibmvfc_retry_host_init(vhost);
- ibmvfc_log(vhost, level, "Discover %s Targets failed: %s (%x:%x)\n",
- proto_type[channels->protocol],
+ ibmvfc_log(vhost, level, "Discover NVMe Targets failed: %s (%x:%x)\n",
ibmvfc_get_cmd_error(be16_to_cpu(rsp->status), be16_to_cpu(rsp->error)),
be16_to_cpu(rsp->status), be16_to_cpu(rsp->error));
break;
case IBMVFC_MAD_DRIVER_FAILED:
break;
default:
- dev_err(vhost->dev, "Invalid Discover Targets response: 0x%x\n", mad_status);
+ dev_err(vhost->dev, "Invalid Discover NVMe Targets response: 0x%x\n", mad_status);
ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
break;
}
@@ -5184,7 +5220,8 @@ static void ibmvfc_discover_targets_done(struct ibmvfc_event *evt)
wake_up(&vhost->work_wait_q);
}
-static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channels)
+static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channels,
+ void (*done)(struct ibmvfc_event *))
{
struct ibmvfc_discover_targets *mad;
struct ibmvfc_host *vhost = ibmvfc_channels_to_vhost(channels);
@@ -5193,7 +5230,7 @@ static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channe
if (!evt)
return NULL;
- ibmvfc_init_event(evt, ibmvfc_discover_targets_done, IBMVFC_MAD_FORMAT);
+ ibmvfc_init_event(evt, done, IBMVFC_MAD_FORMAT);
mad = &evt->iu.discover_targets;
memset(mad, 0, sizeof(*mad));
mad->common.version = cpu_to_be32(1);
@@ -5211,17 +5248,18 @@ static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channe
}
/**
- * ibmvfc_discover_targets - Send Discover Targets MAD
+ * ibmvfc_discover_nvme_targets - Send Discover NVMe Targets MAD
* @vhost: ibmvfc host struct
*
**/
-static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
+static void ibmvfc_discover_nvme_targets(struct ibmvfc_host *vhost)
{
- struct ibmvfc_event *evt = ibmvfc_get_disc_event(&vhost->scsi_scrqs);
+ struct ibmvfc_event *evt = ibmvfc_get_disc_event(&vhost->nvme_scrqs,
+ ibmvfc_discover_nvme_targets_done);
int level = IBMVFC_DEFAULT_LOG_LEVEL;
if (!evt) {
- ibmvfc_log(vhost, level, "Discover SCSI Targets failed: no available events\n");
+ ibmvfc_log(vhost, level, "Discover NVMe Targets failed: no available events\n");
ibmvfc_hard_reset_host(vhost);
return;
}
@@ -5229,29 +5267,34 @@ static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
if (!ibmvfc_send_event(evt, vhost, default_timeout))
- ibmvfc_dbg(vhost, "Sent discover SCSI targets\n");
+ ibmvfc_dbg(vhost, "Sent discover NVMe targets\n");
else
- goto link_down;
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+}
- if (!ibmvfc_nvme_active(vhost))
- return;
+/**
+ * ibmvfc_discover_targets - Send Discover SCSI Targets MAD
+ * @vhost: ibmvfc host struct
+ *
+ **/
+static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
+{
+ struct ibmvfc_event *evt = ibmvfc_get_disc_event(&vhost->scsi_scrqs,
+ ibmvfc_discover_scsi_targets_done);
+ int level = IBMVFC_DEFAULT_LOG_LEVEL;
- evt = ibmvfc_get_disc_event(&vhost->nvme_scrqs);
if (!evt) {
- ibmvfc_log(vhost, level, "Discover NVMe Targets failed: no available events\n");
+ ibmvfc_log(vhost, level, "Discover SCSI Targets failed: no available events\n");
ibmvfc_hard_reset_host(vhost);
return;
}
+ ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
+
if (!ibmvfc_send_event(evt, vhost, default_timeout))
- ibmvfc_dbg(vhost, "Sent discover NVMe targets\n");
+ ibmvfc_dbg(vhost, "Sent discover SCSI targets\n");
else
- goto link_down;
-
- return;
-
-link_down:
- ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
}
static void ibmvfc_fabric_login_nvme_done(struct ibmvfc_event *evt)
--
2.55.0
^ permalink raw reply related [flat|nested] 26+ messages in thread