public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Daniel Wagner <wagi@kernel.org>,
	Justin Tee <justin.tee@broadcom.com>,
	Christoph Hellwig <hch@lst.de>, Keith Busch <kbusch@kernel.org>,
	Sasha Levin <sashal@kernel.org>,
	nareshgottumukkala83@gmail.com, paul.ely@broadcom.com,
	sagi@grimberg.me, linux-nvme@lists.infradead.org
Subject: [PATCH AUTOSEL 6.18-5.10] nvme-fc: don't hold rport lock when putting ctrl
Date: Thu, 11 Dec 2025 21:08:54 -0500	[thread overview]
Message-ID: <20251212020903.4153935-2-sashal@kernel.org> (raw)
In-Reply-To: <20251212020903.4153935-1-sashal@kernel.org>

From: Daniel Wagner <wagi@kernel.org>

[ Upstream commit b71cbcf7d170e51148d5467820ae8a72febcb651 ]

nvme_fc_ctrl_put can acquire the rport lock when freeing the
ctrl object:

nvme_fc_ctrl_put
  nvme_fc_ctrl_free
    spin_lock_irqsave(rport->lock)

Thus we can't hold the rport lock when calling nvme_fc_ctrl_put.

Justin suggested use the safe list iterator variant because
nvme_fc_ctrl_put will also modify the rport->list.

Cc: Justin Tee <justin.tee@broadcom.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This shows the affected function was introduced in v5.8-rc1
(`14fd1e98afafc`), meaning this deadlock bug has existed since **Linux
5.8** and affects all stable kernels from 5.8 onwards (5.10.y, 5.15.y,
6.1.y, 6.6.y, etc.).

### SUMMARY

**What the commit fixes:**
A **deadlock bug** in the NVMe-FC (Fibre Channel) driver where
`nvme_fc_match_disconn_ls()` holds `rport->lock` while calling
`nvme_fc_ctrl_put()`. When the reference count hits zero,
`nvme_fc_ctrl_free()` tries to acquire the same lock, causing a
deadlock.

**Stable kernel criteria:**
| Criterion | Assessment |
|-----------|------------|
| Obviously correct | ✅ Uses canonical lock drop/reacquire pattern |
| Fixes real bug | ✅ Deadlock - system hang |
| Important issue | ✅ Deadlocks in storage paths are critical |
| Small and contained | ✅ ~6 lines in one function |
| No new features | ✅ Pure bug fix |
| Expert reviewed | ✅ Christoph Hellwig |

**Risk vs Benefit:**
- **Risk:** LOW - The fix uses a well-established kernel pattern
  (`list_for_each_entry_safe` + lock release/reacquire)
- **Benefit:** HIGH - Prevents deadlock in NVMe-FC storage driver used
  in enterprise environments

**Concerns:**
- No explicit `Cc: stable` tag, but this is not required for obvious bug
  fixes
- No `Fixes:` tag, but we've identified the bug exists since v5.8
- The fix should apply cleanly to any kernel with the affected function
  (5.8+)

### CONCLUSION

This commit fixes a clear deadlock bug in the NVMe-FC driver that has
existed since Linux 5.8. The fix is:
- Small and surgical (only ~6 lines changed)
- Uses well-understood, standard kernel locking patterns
- Has been reviewed by a respected kernel developer (Christoph Hellwig)
- Signed off by the NVMe maintainer (Keith Busch)
- Affects enterprise storage users who rely on NVMe over Fibre Channel

Deadlocks in storage drivers are serious issues that warrant stable
backporting. The minimal scope and established fix pattern make this a
low-risk, high-value backport.

**YES**

 drivers/nvme/host/fc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 2c903729b0b90..8324230c53719 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1468,14 +1468,14 @@ nvme_fc_match_disconn_ls(struct nvme_fc_rport *rport,
 {
 	struct fcnvme_ls_disconnect_assoc_rqst *rqst =
 					&lsop->rqstbuf->rq_dis_assoc;
-	struct nvme_fc_ctrl *ctrl, *ret = NULL;
+	struct nvme_fc_ctrl *ctrl, *tmp, *ret = NULL;
 	struct nvmefc_ls_rcv_op *oldls = NULL;
 	u64 association_id = be64_to_cpu(rqst->associd.association_id);
 	unsigned long flags;
 
 	spin_lock_irqsave(&rport->lock, flags);
 
-	list_for_each_entry(ctrl, &rport->ctrl_list, ctrl_list) {
+	list_for_each_entry_safe(ctrl, tmp, &rport->ctrl_list, ctrl_list) {
 		if (!nvme_fc_ctrl_get(ctrl))
 			continue;
 		spin_lock(&ctrl->lock);
@@ -1488,7 +1488,9 @@ nvme_fc_match_disconn_ls(struct nvme_fc_rport *rport,
 		if (ret)
 			/* leave the ctrl get reference */
 			break;
+		spin_unlock_irqrestore(&rport->lock, flags);
 		nvme_fc_ctrl_put(ctrl);
+		spin_lock_irqsave(&rport->lock, flags);
 	}
 
 	spin_unlock_irqrestore(&rport->lock, flags);
-- 
2.51.0


  reply	other threads:[~2025-12-12  2:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-12  2:08 [PATCH AUTOSEL 6.18-5.15] platform/x86/intel/hid: Add Dell Pro Rugged 10/12 tablet to VGBS DMI quirks Sasha Levin
2025-12-12  2:08 ` Sasha Levin [this message]
2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix double put in emc2305_probe_childs_from_dt Sasha Levin
2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.17] platform/x86: wmi-gamezone: Add Legion Go 2 Quirks Sasha Levin
2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix device node refcount leak in error path Sasha Levin
2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.12] nvme-fabrics: add ENOKEY to no retry criteria for authentication failures Sasha Levin
2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.6] i2c: designware: Disable SMBus interrupts to prevent storms from mis-configured firmware Sasha Levin
2025-12-12  2:09 ` [PATCH AUTOSEL 6.18-6.6] MIPS: ftrace: Fix memory corruption when kernel is located beyond 32 bits Sasha Levin

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=20251212020903.4153935-2-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=hch@lst.de \
    --cc=justin.tee@broadcom.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=nareshgottumukkala83@gmail.com \
    --cc=patches@lists.linux.dev \
    --cc=paul.ely@broadcom.com \
    --cc=sagi@grimberg.me \
    --cc=stable@vger.kernel.org \
    --cc=wagi@kernel.org \
    /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