[PATCH AUTOSEL 7.0-6.18] scsi: lpfc: Remove unnecessary ndlp kref get in lpfc_check_nlp_post_devloss

Sasha Levin
2026-04-28T10:42:12Z

From: Justin Tee <justin.tee@broadcom.com>

[ Upstream commit f6bfb8d149336661bb80e62980da9a45b920403c ]

When NLP_IN_RECOV_POST_DEV_LOSS is set, the initial node reference
remains held while recovery is in progress. Taking a reference when
NLP_IN_RECOV_POST_DEV_LOSS is cleared results in an additional reference
being held. This causes an extra reference when cleaning up lpfc_vport
instances. Thus, remove the extraneous ndlp kref get in
lpfc_check_nlp_post_devloss.

Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://patch.msgid.link/20260212213008.149873-7-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Based on my thorough investigation, I can now complete my analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: Parse the Subject Line**
- Record: Subsystem `scsi: lpfc:`, action verb `Remove`, summary:
removes an unnecessary `ndlp` kref get in
`lpfc_check_nlp_post_devloss` that was leaving an extra reference
after device-loss recovery.

**Step 1.2: Parse All Commit Message Tags**
- Record: `Signed-off-by: Justin Tee <justin.tee@broadcom.com>` (author
is a Broadcom lpfc maintainer). `Link: https://patch.msgid.link/202602
12213008.149873-7-justintee8345@gmail.com`. `Signed-off-by: Martin K.
Petersen <martin.petersen@oracle.com>` (SCSI maintainer). NO `Fixes:`
tag, NO `Reported-by:` tag, NO `Cc: stable`, NO `Reviewed-by`.

**Step 1.3: Analyze the Commit Body Text**
- Record: The commit describes a refcount leak - when
`NLP_IN_RECOV_POST_DEV_LOSS` is set, the initial ndlp reference is
*still held* (because current `lpfc_dev_loss_tmo_handler` does *not*
put in the recovering path anymore). Taking another reference via
`lpfc_nlp_get` in the "reverse" path therefore *adds* an extra
reference. Symptom: "extra reference when cleaning up lpfc_vport
instances". No crash/stack trace given.

**Step 1.4: Detect Hidden Bug Fixes**
- Record: Subject begins with "Remove unnecessary" (cleanup wording) but
commit body explicitly says "extra reference when cleaning up
lpfc_vport instances" - this IS a real bug fix for a ref leak (pattern
7: reference counting bug).

## PHASE 2: DIFF ANALYSIS

**Step 2.1: Inventory the Changes**
- Record: One file changed: `drivers/scsi/lpfc/lpfc_hbadisc.c`. One line
removed (`lpfc_nlp_get(ndlp);`). Scope: single-function surgical fix.

**Step 2.2: Understand the Code Flow Change**
- Record: Before - `lpfc_check_nlp_post_devloss` cleared the
`NLP_IN_RECOV_POST_DEV_LOSS` flag, cleared `NLP_DROPPED`, then called
`lpfc_nlp_get(ndlp)` to "restore" a put supposedly performed in
`lpfc_dev_loss_tmo_handler`. After - the get is gone; the flag bits
are still cleared; no refcount change is performed.

**Step 2.3: Identify the Bug Mechanism**
- Record: Category (c) reference counting fix - removing an extra
`lpfc_nlp_get()`. The reason the get is wrong: commit `d1a2ef63fc8b3`
("scsi: lpfc: Fix kref imbalance on fabric ndlps from dev_loss_tmo
handler", merged in v6.12) added an early `return fcf_inuse;` in the
`recovering` branch, so `lpfc_nlp_put(ndlp)` is no longer executed
when `NLP_IN_RECOV_POST_DEV_LOSS` is set. The matching `lpfc_nlp_get`
in `lpfc_check_nlp_post_devloss` was left behind and now grabs an
extra reference every time a fabric ndlp transiently hits dev_loss and
recovers.

**Step 2.4: Assess the Fix Quality**
- Record: Minimal 1-line removal. Obvious correctness - just removes the
stale counterpart of a no-longer-happening put. No new regression
risk: removing an unbalanced get can only reduce references, it cannot
cause a UAF (the ndlp reference held before dev_loss remains held
because the put never happens in the recovering path).

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: Blame the Changed Lines**
- Record: The `lpfc_nlp_get(ndlp);` being removed was originally
introduced by `af984c87293b1` (Oct 2021, v5.17) as part of "scsi:
lpfc: Allow fabric node recovery if recovery is in progress before
devloss". At that time it was *correct* (paired with a put in the
handler). The bug was introduced by `d1a2ef63fc8b3` (Sep 2024, v6.12)
"scsi: lpfc: Fix kref imbalance on fabric ndlps from dev_loss_tmo
handler", which added the early `return fcf_inuse;` that skips the
`lpfc_nlp_put` in the recovering path but left the `lpfc_nlp_get` in
`lpfc_check_nlp_post_devloss` dangling.

**Step 3.2: Follow the Fixes: Tag**
- Record: No `Fixes:` tag given. Based on code analysis, the actual
Fixes: target is `d1a2ef63fc8b3`, present in v6.12 and forward.

**Step 3.3: Check File History for Related Changes**
- Record: Related recent commits: `07caedc6a3887` (Nov 2025) added the
`clear_bit(NLP_DROPPED, ...);` to `lpfc_check_nlp_post_devloss`;
`3f8f9f16f844a` converted save_flags to bitmask; `e07ac2d2aa5fc`
removed unnecessary relocking. This is a standalone patch, part of a
13-patch lpfc-14.4.0.14 update series (PATCH 6/13).

**Step 3.4: Check the Author's Other Commits**
- Record: Justin Tee (Broadcom) is the primary lpfc
maintainer/contributor. Wrote `d1a2ef63fc8b3` (which introduced this
bug) and has 30+ lpfc commits recently. Subject-matter expert.

**Step 3.5: Check for Dependent/Prerequisite Commits**
- Record: This fix is completely standalone for trees that contain
`d1a2ef63fc8b3` (v6.12+). The only dependency is that the buggy commit
is present. Can apply standalone.

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1: Find the Original Patch Discussion**
- Record: `b4 am` located the patch series at lore.kernel.org. Patch is
"PATCH 6/13 lpfc: Remove unnecessary ndlp kref get in
lpfc_check_nlp_post_devloss" from series "Update lpfc to revision
14.4.0.14" posted 2026-02-12 against Martin's 6.20/scsi-queue tree. No
review discussion found on this specific patch (no `Reviewed-by`, no
NAKs, no stable suggestions).

**Step 4.2: Check Who Reviewed the Patch**
- Record: Series is a typical Broadcom/lpfc driver update going through
SCSI maintainer Martin K. Petersen. No explicit per-patch review
comments retrieved.

**Step 4.3: Search for the Bug Report**
- Record: No `Reported-by:` tag. The bug was author-discovered via code
analysis, not a user report.

**Step 4.4: Check for Related Patches and Series**
- Record: This is patch 6 of a 13-patch series. Other patches include
logging improvements, typecast changes, cleanup of `lpfc_fdmi_cmd`
error paths, `txcmplq_cnt` fixes, NVMe abort cleanup on PCI reset, and
version bump. This specific patch is self-contained.

**Step 4.5: Check Stable Mailing List History**
- Record: No prior discussion found.

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1: Identify Key Functions in the Diff**
- Record: `lpfc_check_nlp_post_devloss()` only.

**Step 5.2: Trace Callers**
- Record: Callers verified via grep - 5 total call sites:
`lpfc_mbx_cmpl_fc_reg_login`, `lpfc_nlp_reg_node` (in
`lpfc_hbadisc.c`), and 3 sites in `lpfc_els.c` around FLOGI handling.
These are mainline FC discovery/login completion paths - called
routinely during normal operation, particularly after link events.

**Step 5.3: Trace Callees**
- Record: The function calls `test_and_clear_bit`, `clear_bit`,
`lpfc_nlp_get` (being removed), and `lpfc_printf_vlog` (logging). No
I/O, no complex state changes.

**Step 5.4: Follow the Call Chain**
- Record: Reachable from any FC link-bounce/devloss path. Real-world
triggered every time a fabric ndlp (Fabric_DID, FDMI_DID,
NameServer_DID, Fabric_Cntl_DID) dev_loss_tmo-fires while recovery is
still in progress - a common event on FC fabrics.

**Step 5.5: Search for Similar Patterns**
- Record: N/A - this is a specific function.

## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS

**Step 6.1: Does the Buggy Code Exist in Stable Trees?**
- Record: Verified via `git show
<branch>:drivers/scsi/lpfc/lpfc_hbadisc.c`:
- stable/linux-6.12.y: HAS BUG - `recovering` path has `return
fcf_inuse;` (no put); `lpfc_check_nlp_post_devloss` has
`lpfc_nlp_get(ndlp)` (extra ref).
- stable/linux-6.17.y, 6.18.y, 6.19.y: HAS BUG - same pattern.
- stable/linux-6.6.y: NO BUG - the `recovering` path in 6.6.y does NOT
have the early return; the `lpfc_nlp_put(ndlp)` still executes at
the end of fabric-node handling, so the `lpfc_nlp_get` correctly
balances it. Fix must NOT be backported to 6.6.y.

**Step 6.2: Check for Backport Complications**
- Record: For 6.18.y and 6.19.y: patch applies cleanly (same surrounding
context with `clear_bit(NLP_DROPPED, ...)`). For 6.17.y: needs minor
context adjustment (no `clear_bit(NLP_DROPPED, ...)` line above). For
6.12.y: function still uses the older `spin_lock_irqsave`/`save_flags
&=` form; a manual adjustment (simply removing the
`lpfc_nlp_get(ndlp);` line amidst different surrounding context) is
needed but trivial.

**Step 6.3: Check If Related Fixes Are Already in Stable**
- Record: None. Buggy commit `d1a2ef63fc8b3` went into v6.12 directly
(not backported). No alternate fix present.

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1: Identify the Subsystem and Its Criticality**
- Record: `drivers/scsi/lpfc/` - Broadcom/Emulex Fibre Channel HBA
driver. Criticality: IMPORTANT - used in enterprise storage/SAN
setups. Reference leaks in a driver of this size matter to enterprise
users running long-lived systems.

**Step 7.2: Assess Subsystem Activity**
- Record: Actively maintained (regular version bumps, multiple patches
per quarter). Justin Tee / Broadcom are responsive maintainers.

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: Determine Who Is Affected**
- Record: Affected population - users of lpfc-driven FC HBAs running
v6.12+ stable kernels who experience fabric link bounces,
zoning/fabric reconfiguration, or transient device-loss events on
Fabric_DID/FDMI/NameServer/Fabric_Cntl ndlps.

**Step 8.2: Determine the Trigger Conditions**
- Record: Triggered every time a fabric ndlp enters dev_loss_tmo while
still in a recovering discovery state. On busy fabrics with many
vports, this can happen repeatedly; each occurrence leaks one ndlp
reference.

**Step 8.3: Determine the Failure Mode Severity**
- Record: Failure mode - ndlp kref leak; ndlp objects cannot be freed
during `lpfc_vport` cleanup; potential memory accumulation over time;
can cause WARN messages or stuck/hang behavior on vport
teardown/module unload. Severity: MEDIUM-HIGH - not an immediate crash
or security issue but a persistent resource leak in a commonly-
triggered code path in enterprise storage workloads.

**Step 8.4: Calculate Risk-Benefit Ratio**
- Record: Benefit - fixes a real ref leak introduced by a previous "fix"
commit, benefit is MEDIUM-HIGH for lpfc users. Risk - 1-line removal,
obvious correctness, no possibility of new UAF since we're only
reducing (not adding) puts; risk VERY LOW. Ratio: clearly favorable
for backport.

## PHASE 9: FINAL SYNTHESIS

**Step 9.1: Compile Evidence**
- FOR: Real kref leak bug in common code path; trivially small/obvious
fix; author is subsystem maintainer who introduced the original bug;
bug confirmed to exist in 6.12.y, 6.17.y, 6.18.y, 6.19.y via direct
inspection; code analysis is unambiguous.
- AGAINST: No `Fixes:`, `Reported-by:`, `Cc: stable`; no user report of
symptoms; a similar comment language ("put the initial ref count
back") now stale but not updated.
- UNRESOLVED: Whether this is already in linux-next / scsi-queue for
v6.21 (not verified directly, but the cover letter says it targets
6.20/scsi-queue so likely on its way to mainline imminently).

**Step 9.2: Apply Stable Rules Checklist**
1. Obviously correct and tested? YES - trivial to verify by reading the
two functions together; author has build/test history.
2. Fixes real bug affecting users? YES - refcount leak on every fabric-
ndlp recovery event.
3. Important issue? MEDIUM-HIGH (resource leak in enterprise storage
driver).
4. Small and contained? YES - one line removal.
5. No new features or APIs? YES - pure bug fix.
6. Applies to stable? YES for 6.12.y (with trivial manual adjustment)
and 6.17.y+; must NOT go to 6.6.y and earlier.

**Step 9.3: Check for Exception Categories**
- N/A - normal bug fix, not a quirk/device ID/DT update/build fix.

**Step 9.4: Decision**
- This is a genuine reference-counting bug fix for a regression
introduced by `d1a2ef63fc8b3` in v6.12. The fix is minimal, obviously
correct, and affects a common code path in the lpfc FC driver used by
enterprise storage. Risk is essentially zero; benefit prevents a real
ref leak. Should be backported to 6.12+ stable trees (NOT to 6.6.y and
earlier where the bug does not exist).

## Verification

- [Phase 1] Parsed tags on the supplied commit message: Signed-off-by
Justin Tee / Martin K. Petersen, Link to patch.msgid.link. No
Fixes/Reported-by/Cc-stable/Reviewed-by.
- [Phase 2] Read the diff: single 1-line removal of
`lpfc_nlp_get(ndlp);` in `lpfc_check_nlp_post_devloss` within
`drivers/scsi/lpfc/lpfc_hbadisc.c`.
- [Phase 3] `git log -L:lpfc_check_nlp_post_devloss` confirmed the get
was introduced in `af984c87293b1` (v5.17) where it was correct (paired
with a put).
- [Phase 3] `git show d1a2ef63fc8b3` - confirmed this commit added the
early `return fcf_inuse;` in the recovering branch, breaking the
get/put pairing.
- [Phase 3] `git log --author="Justin Tee"` - confirmed author is active
lpfc maintainer.
- [Phase 3] `git tag --contains d1a2ef63fc8b3` - buggy commit is in
v6.12 and beyond.
- [Phase 4] `b4 am 20260212213008.149873-7-justintee8345@gmail.com` -
downloaded the series, confirmed it is patch 6/13 in "Update lpfc to
revision 14.4.0.14" against 6.20/scsi-queue. No per-patch review
discussion.
- [Phase 5] `grep lpfc_check_nlp_post_devloss` - 5 callers:
`lpfc_mbx_cmpl_fc_reg_login`, `lpfc_nlp_reg_node`, 3 in `lpfc_els.c`
FLOGI paths - all routine discovery paths.
- [Phase 6] `git show stable/linux-6.12.y:...lpfc_hbadisc.c` - confirmed
bug exists (function has lpfc_nlp_get, recovering path has `return
fcf_inuse;`).
- [Phase 6] `git show
stable/linux-6.17.y/6.18.y/6.19.y:...lpfc_hbadisc.c` - same bug
pattern confirmed in each.
- [Phase 6] `git show stable/linux-6.6.y:...lpfc_hbadisc.c` - confirmed
bug does NOT exist (recovering path has no early return; put still
fires).
- [Phase 8] Failure mode: ndlp kref leak on every fabric-ndlp dev_loss
recovery event, MEDIUM-HIGH severity.
- UNVERIFIED: Whether any user has reported hangs/OOM from this specific
leak - no such report exists, but the mechanism is clear from code
analysis.
- UNVERIFIED: Exact mainline SHA of the commit (not yet in my local
`master`), but the patch content matches what is on lore/in the
14.4.0.14 series.

The fix is small, surgical, and addresses a real regression that exists
in multiple active stable trees (6.12+). Meets all stable kernel rules.
The adaptation for 6.12.y requires a trivial manual edit (different
surrounding context) but the change itself is a single-line removal.

**YES**

drivers/scsi/lpfc/lpfc_hbadisc.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 8aaf05d7bb0af..d42b911a0aee1 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -425,7 +425,6 @@ lpfc_check_nlp_post_devloss(struct lpfc_vport *vport,
{
if (test_and_clear_bit(NLP_IN_RECOV_POST_DEV_LOSS, &ndlp->save_flags)) {
clear_bit(NLP_DROPPED, &ndlp->nlp_flag);
- lpfc_nlp_get(ndlp);
lpfc_printf_vlog(vport, KERN_INFO, LOG_DISCOVERY | LOG_NODE,
"8438 Devloss timeout reversed on DID x%x "
"refcnt %d ndlp %p flag x%lx "
--
2.53.0