From bb45bc4bd53c95a7bf6f782577b5ede94c0f8aa8 Mon Sep 17 00:00:00 2001 From: Nilesh Javali Date: Thu, 30 Jul 2026 21:28:25 +0530 Subject: [PATCH] scsi: qla2xxx: Avoid double completion in async IOCB timeout qla2x00_async_iocb_timeout() tries to abort a timed-out async IOCB. When qla24xx_async_abort_cmd() fails, both the SRB_LOGIN_CMD path and the SRB_CTRL_VP/default path scan outstanding_cmds[] for the SRB and then call sp->done(sp, QLA_FUNCTION_TIMEOUT) unconditionally, without checking whether the SRB was actually found and removed. If the response ISR completes the same handle first, it removes the SRB under qp_lock_ptr and runs sp->done() -> complete(sp->comp). The submitter qla24xx_control_vp() wakes from wait_for_completion(), clears sp->comp, drops its reference and returns, reclaiming the on-stack completion. The timer reference keeps the SRB alive across the timeout handler, but not the submitter's stack. The timeout then issues a second sp->done() -> qla_ctrlvp_sp_done(), which evaluates "if (sp->comp) complete(sp->comp)"; with the pointer loaded before the submitter's NULL store, complete() writes into the freed stack frame, a use-after-free. Track whether this path removed the SRB from outstanding_cmds and only call sp->done() when it did, so the command is completed exactly once by whichever path owns it. This mirrors the sp_found guard already used in qla24xx_abort_iocb_timeout(). Fixes: f6145e86d21f ("scsi: qla2xxx: Fix race between switch cmd completion and timeout") Cc: stable@vger.kernel.org Reported-by: Sashiko Signed-off-by: Nilesh Javali Link: https://patch.msgid.link/20260730155838.2119230-21-njavali@marvell.com Signed-off-by: Martin K. Petersen (Oracle) --- drivers/scsi/qla2xxx/qla_init.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 5788c7e53d8f..36de0a0bbcc1 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -228,7 +228,7 @@ qla2x00_async_iocb_timeout(void *data) srb_t *sp = data; fc_port_t *fcport = sp->fcport; struct srb_iocb *lio = &sp->u.iocb_cmd; - int rc, h; + int rc, h, found; unsigned long flags; if (fcport) { @@ -251,6 +251,7 @@ qla2x00_async_iocb_timeout(void *data) lio->u.logio.data[1] = lio->u.logio.flags & SRB_LOGIN_RETRIED ? QLA_LOGIO_LOGIN_RETRIED : 0; + found = 0; spin_lock_irqsave(sp->qpair->qp_lock_ptr, flags); for (h = 1; h < sp->qpair->req->num_outstanding_cmds; h++) { @@ -258,11 +259,19 @@ qla2x00_async_iocb_timeout(void *data) sp) { sp->qpair->req->outstanding_cmds[h] = NULL; + found = 1; break; } } spin_unlock_irqrestore(sp->qpair->qp_lock_ptr, flags); - sp->done(sp, QLA_FUNCTION_TIMEOUT); + /* + * Only complete the command if this path removed it + * from outstanding_cmds. Otherwise the ISR already + * completed it and a second sp->done() would race the + * submitter's freeing of the on-stack completion. + */ + if (found) + sp->done(sp, QLA_FUNCTION_TIMEOUT); } break; case SRB_LOGOUT_CMD: @@ -275,6 +284,7 @@ qla2x00_async_iocb_timeout(void *data) default: rc = qla24xx_async_abort_cmd(sp, false); if (rc) { + found = 0; spin_lock_irqsave(sp->qpair->qp_lock_ptr, flags); for (h = 1; h < sp->qpair->req->num_outstanding_cmds; h++) { @@ -282,11 +292,19 @@ qla2x00_async_iocb_timeout(void *data) sp) { sp->qpair->req->outstanding_cmds[h] = NULL; + found = 1; break; } } spin_unlock_irqrestore(sp->qpair->qp_lock_ptr, flags); - sp->done(sp, QLA_FUNCTION_TIMEOUT); + /* + * Only complete the command if this path removed it + * from outstanding_cmds. Otherwise the ISR already + * completed it and a second sp->done() would race the + * submitter's freeing of the on-stack completion. + */ + if (found) + sp->done(sp, QLA_FUNCTION_TIMEOUT); } break; }