* [PATCH v2] scsi: lpfc: Fix race conditions in ELS retry
@ 2026-10-29 18:50 Kyle Mahlkuch
0 siblings, 0 replies; 2+ messages in thread
From: Kyle Mahlkuch @ 2026-10-29 18:50 UTC (permalink / raw)
To: linux-scsi, linux-kernel, Paul Ely; +Cc: hare, msmurthy
This patch addresses a race condition in the lpfc driver's ELS retry
event handling that can lead to a use-after-free in the
lpfc_cancel_retry_delay_tmo(), where the NLP_DELAY_TMO flag is cleared
before acquiring the lock to check if the retry event is queued. This
creates a window where the timer can be rescheduled and fire, causing
both the cancel path and the worker thread to release the same
reference, resulting in a double-put and use-after-free.
Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.ibm.com>
Signed-off-by: Paul Ely <paul.ely@broadcom.com>
Tested-by: Maram Srimannarayana Murthy <msmurthy@linux.ibm.com>
---
drivers/scsi/lpfc/lpfc_els.c | 32 +++++++++++++++++++++++++-------
drivers/scsi/lpfc/lpfc_hbadisc.c | 5 +++++
2 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 6f6394a0047c..ffd61ac46646 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -4477,18 +4477,36 @@ lpfc_issue_els_edc(struct lpfc_vport *vport,
uint8_t retry)
void
lpfc_cancel_retry_delay_tmo(struct lpfc_vport *vport, struct
lpfc_nodelist *nlp)
{
- struct lpfc_work_evt *evtp;
+ struct lpfc_hba *phba = vport->phba;
+ struct lpfc_work_evt *evtp = &nlp->els_retry_evt;
+ struct lpfc_nodelist *arg_ndlp = NULL;
+ unsigned long flags;
+
if (!test_and_clear_bit(NLP_DELAY_TMO, &nlp->nlp_flag))
return;
timer_delete_sync(&nlp->nlp_delayfunc);
nlp->nlp_last_elscmd = 0;
- if (!list_empty(&nlp->els_retry_evt.evt_listp)) {
- list_del_init(&nlp->els_retry_evt.evt_listp);
- /* Decrement nlp reference count held for the delayed retry */
- evtp = &nlp->els_retry_evt;
- lpfc_nlp_put((struct lpfc_nodelist *)evtp->evt_arg1);
- }
+
+ /*
+ * If retry event is queued, remove it under lock and consume
+ * its payload to prevent double-put. The event holds a reference
+ * to the nodelist that must be released exactly once.
+ */
+ spin_lock_irqsave(&phba->hbalock, flags);
+ if (!list_empty(&evtp->evt_listp)) {
+ list_del_init(&evtp->evt_listp);
+ /* Take local copy and clear to prevent reuse */
+ arg_ndlp = (struct lpfc_nodelist *)evtp->evt_arg1;
+ evtp->evt_arg1 = NULL;
+ smp_wmb(); /* Ensure NULL visible before unlock */
+ }
+ spin_unlock_irqrestore(&phba->hbalock, flags);
+
+ /* Drop the event-held reference outside the lock */
+ if (arg_ndlp)
+ lpfc_nlp_put(arg_ndlp);
+
if (test_and_clear_bit(NLP_NPR_2B_DISC, &nlp->nlp_flag)) {
if (vport->num_disc_nodes) {
if (vport->port_state < LPFC_VPORT_READY) {
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c
b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 4c673dffa671..3eb142cf4455 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -846,6 +846,11 @@ lpfc_work_list_done(struct lpfc_hba *phba)
switch (evtp->evt) {
case LPFC_EVT_ELS_RETRY:
ndlp = (struct lpfc_nodelist *) (evtp->evt_arg1);
+ /*
+ * Consume the payload to prevent reuse or double-put.
+ * evt_arg1 was populated when event was queued.
+ */
+ evtp->evt_arg1 = NULL;
if (!hba_pci_err) {
lpfc_els_retry_delay_handler(ndlp);
free_evt = 0; /* evt is part of ndlp */
--
2.55.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* [PATCH v2] scsi: lpfc: Fix race conditions in ELS retry
@ 2026-10-30 13:41 Paul Ely
0 siblings, 0 replies; 2+ messages in thread
From: Paul Ely @ 2026-10-30 13:41 UTC (permalink / raw)
To: Kyle Mahlkuch, linux-scsi, linux-kernel, Hannes Reinecke,
msmurthy
[-- Attachment #1: Type: text/plain, Size: 3601 bytes --]
From: Kyle Mahlkuch <kmahlkuc@linux.ibm.com>
To: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
Paul Ely <paul.ely@broadcom.com>
Cc: hare@suse.de, msmurthy@linux.ibm.com
Subject: [PATCH v2] scsi: lpfc: Fix race conditions in ELS retry
Date: Thu, 29 Oct 2026 13:50:45 -0500 [thread overview]
Message-ID: <29073ebc-b616-475b-bc05-384da09fbb54@linux.ibm.com> (raw)
This patch addresses a race condition in the lpfc driver's ELS retry
event handling that can lead to a use-after-free in the
lpfc_cancel_retry_delay_tmo(), where the NLP_DELAY_TMO flag is cleared
before acquiring the lock to check if the retry event is queued. This
creates a window where the timer can be rescheduled and fire, causing
both the cancel path and the worker thread to release the same
reference, resulting in a double-put and use-after-free.
Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.ibm.com>
Signed-off-by: Paul Ely <paul.ely@broadcom.com>
Tested-by: Maram Srimannarayana Murthy <msmurthy@linux.ibm.com>
---
drivers/scsi/lpfc/lpfc_els.c | 32 +++++++++++++++++++++++++-------
drivers/scsi/lpfc/lpfc_hbadisc.c | 5 +++++
2 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 6f6394a0047c..ffd61ac46646 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -4477,18 +4477,36 @@ lpfc_issue_els_edc(struct lpfc_vport *vport,
uint8_t retry)
void
lpfc_cancel_retry_delay_tmo(struct lpfc_vport *vport, struct
lpfc_nodelist *nlp)
{
- struct lpfc_work_evt *evtp;
+ struct lpfc_hba *phba = vport->phba;
+ struct lpfc_work_evt *evtp = &nlp->els_retry_evt;
+ struct lpfc_nodelist *arg_ndlp = NULL;
+ unsigned long flags;
+
if (!test_and_clear_bit(NLP_DELAY_TMO, &nlp->nlp_flag))
return;
timer_delete_sync(&nlp->nlp_delayfunc);
nlp->nlp_last_elscmd = 0;
- if (!list_empty(&nlp->els_retry_evt.evt_listp)) {
- list_del_init(&nlp->els_retry_evt.evt_listp);
- /* Decrement nlp reference count held for the delayed retry */
- evtp = &nlp->els_retry_evt;
- lpfc_nlp_put((struct lpfc_nodelist *)evtp->evt_arg1);
- }
+
+ /*
+ * If retry event is queued, remove it under lock and consume
+ * its payload to prevent double-put. The event holds a reference
+ * to the nodelist that must be released exactly once.
+ */
+ spin_lock_irqsave(&phba->hbalock, flags);
+ if (!list_empty(&evtp->evt_listp)) {
+ list_del_init(&evtp->evt_listp);
+ /* Take local copy and clear to prevent reuse */
+ arg_ndlp = (struct lpfc_nodelist *)evtp->evt_arg1;
+ evtp->evt_arg1 = NULL;
+ smp_wmb(); /* Ensure NULL visible before unlock */
+ }
+ spin_unlock_irqrestore(&phba->hbalock, flags);
+
+ /* Drop the event-held reference outside the lock */
+ if (arg_ndlp)
+ lpfc_nlp_put(arg_ndlp);
+
if (test_and_clear_bit(NLP_NPR_2B_DISC, &nlp->nlp_flag)) {
if (vport->num_disc_nodes) {
if (vport->port_state < LPFC_VPORT_READY) {
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c
b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 4c673dffa671..3eb142cf4455 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -846,6 +846,11 @@ lpfc_work_list_done(struct lpfc_hba *phba)
switch (evtp->evt) {
case LPFC_EVT_ELS_RETRY:
ndlp = (struct lpfc_nodelist *) (evtp->evt_arg1);
+ /*
+ * Consume the payload to prevent reuse or double-put.
+ * evt_arg1 was populated when event was queued.
+ */
+ evtp->evt_arg1 = NULL;
if (!hba_pci_err) {
lpfc_els_retry_delay_handler(ndlp);
free_evt = 0; /* evt is part of ndlp */
--
Reviewed-by: Paul Ely <paul.ely@broadcom.com>
Looks good Kyle - thanks!
Paul Ely
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5453 bytes --]
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-30 13:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 18:50 [PATCH v2] scsi: lpfc: Fix race conditions in ELS retry Kyle Mahlkuch
-- strict thread matches above, loose matches on Subject: below --
2026-10-30 13:41 Paul Ely
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.