mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kyle Mahlkuch <kmahlkuc@linux.ibm.com>
To: linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
	Paul Ely <paul.ely@broadcom.com>
Cc: msmurthy@linux.ibm.com, mkp@kernel.org, Hannes Reinecke <hare@suse.de>
Subject: [PATCH v4] scsi: lpfc: Fix race conditions in ELS retry handling
Date: Wed, 11 Nov 2026 11:47:41 -0600	[thread overview]
Message-ID: <dcea48dd-f76c-4bb0-9275-c12ef9bd78a2@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.

Moved capture ndlp and clear evt_arg1 under hbalock while dequeuing
LPFC_EVT_ELS_RETRY in lpfc_work_list_done().

Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.ibm.com>
---
Changes since v3:
- More whitespace cleanup. (Paul Ely)

Changes since v2:
- Capture ndlp and clear evt_arg1 under hbalock while dequeuing
  LPFC_EVT_ELS_RETRY in lpfc_work_list_done(). A requeue by
  lpfc_els_retry_delay() could cause a NULL dereference in the worker.
  (AI review)
- Whitespace and line-wrapping cleanup.

Link: https://lore.kernel.org/lkml/29073ebc-b616-475b-bc05-384da09fbb54@linux.ibm.com/T/#u

Changes since v1:
- Leave test_and_clear_bit op and timer_delete_sync at the beginning of
  lpfc_cancel_retry_delay_tmo() outside of lock. (Paul Ely)

Link: https://lore.kernel.org/lkml/1c8a764c-fce1-4ce1-b797-47ac328cf3f2@linux.ibm.com/T/#u
---
 drivers/scsi/lpfc/lpfc_els.c     | 29 +++++++++++++++++++++++------
 drivers/scsi/lpfc/lpfc_hbadisc.c | 11 ++++++++++-
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 6f6394a0047c..98d1824aefe1 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -4477,18 +4477,35 @@ 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..75457f19d4ea 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -840,12 +840,21 @@ lpfc_work_list_done(struct lpfc_hba *phba)
 	while (!list_empty(&phba->work_list)) {
 		list_remove_head((&phba->work_list), evtp, typeof(*evtp),
 				 evt_listp);
+		/*
+		 * The ELS retry event can be requeued by lpfc_els_retry_delay()
+		 * as soon as hbalock is dropped. Consume the payload within the
+		 * hbalock to prevent reuse or double-put.
+		 */
+		if (evtp->evt == LPFC_EVT_ELS_RETRY) {
+			ndlp = (struct lpfc_nodelist *)(evtp->evt_arg1);
+			evtp->evt_arg1 = NULL;
+		}
 		spin_unlock_irq(&phba->hbalock);
 		hba_pci_err = test_bit(HBA_PCI_ERR, &phba->bit_flags);
 		free_evt = 1;
 		switch (evtp->evt) {
 		case LPFC_EVT_ELS_RETRY:
-			ndlp = (struct lpfc_nodelist *) (evtp->evt_arg1);
+			/* ndlp was already captured under hbalock */
 			if (!hba_pci_err) {
 				lpfc_els_retry_delay_handler(ndlp);
 				free_evt = 0; /* evt is part of ndlp */
--
2.55.0

                 reply	other threads:[~2026-11-11 17:47 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dcea48dd-f76c-4bb0-9275-c12ef9bd78a2@linux.ibm.com \
    --to=kmahlkuc@linux.ibm.com \
    --cc=hare@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mkp@kernel.org \
    --cc=msmurthy@linux.ibm.com \
    --cc=paul.ely@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®