[PATCH 0/2] scsi: lpfc: bound EDC descriptor TLV walk

[PATCH 0/2] scsi: lpfc: bound EDC descriptor TLV walk

[Michael Bommarito]

An adjacent Fibre Channel fabric peer or device can crash an LPFC host
with a malformed EDC ELS frame. lpfc_els_rcv_edc() trusts the EDC
descriptor-list length from the received frame without checking that it
fits in the actual ELS payload, so a short frame with an oversized
descriptor-list length walks the TLV list past the receive buffer and
trips a KASAN slab-out-of-bounds read in the ELS receive path.

Patch 1 passes the received payload length into lpfc_els_rcv_edc(),
rejects truncated EDC headers and descriptor lists larger than the
payload, and avoids logging a third payload word unless it is present.
Patch 2 adds same-translation-unit KUnit/KASAN coverage: a benign EDC
frame that must still parse and the malformed frame that must now be
rejected.

Reproduced with the KUnit/KASAN test on f5459048c38a: stock trips
BUG: KASAN: slab-out-of-bounds in lpfc_els_rcv_edc after the benign
control passes; patched rejects the frame and both cases pass.

Cc: stable@vger.kernel.org

Michael Bommarito (2):
  scsi: lpfc: bound EDC descriptor list by payload length
  scsi: lpfc: add KUnit coverage for EDC descriptor bounds

 drivers/scsi/Kconfig         |   7 ++
 drivers/scsi/lpfc/lpfc_els.c | 195 ++++++++++++++++++++++++++++++++---
 2 files changed, 189 insertions(+), 13 deletions(-)

--
2.53.0

[PATCH 1/2] scsi: lpfc: bound EDC descriptor list by payload length

[Michael Bommarito]

drivers/scsi/lpfc/lpfc_els.c:lpfc_els_rcv_edc() trusts the EDC
descriptor-list length from the received frame without checking that it
fits in the actual ELS payload. An adjacent Fibre Channel fabric peer or
device can send an unsolicited EDC frame with a short payload and an
oversized descriptor-list length. The TLV walk can then read past the
receive buffer and trip a KASAN slab-out-of-bounds read in the LPFC ELS
receive path.

Impact: An adjacent Fibre Channel fabric peer or device can crash an
LPFC host via a malformed EDC ELS frame.

Pass the received payload length into lpfc_els_rcv_edc(), reject
truncated EDC headers and descriptor lists larger than the received
payload, and avoid logging a third payload word unless it is present.

Fixes: 9064aeb2df8e ("scsi: lpfc: Add EDC ELS support")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---

I reproduced this with a same-translation-unit KUnit/KASAN test on
f5459048c38a. Without the patch, the malformed EDC frame triggers
BUG: KASAN: slab-out-of-bounds in lpfc_els_rcv_edc after the benign EDC
control passes. With the patch, the benign and malformed KUnit cases
both pass.
 drivers/scsi/lpfc/lpfc_els.c | 41 ++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index cee709617a313..5408b116f2d5a 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -9409,13 +9409,14 @@ lpfc_els_rcv_fan(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
  * @vport: pointer to a host virtual N_Port data structure.
  * @cmdiocb: pointer to lpfc command iocb data structure.
  * @ndlp: pointer to a node-list data structure.
+ * @payload_len: received EDC payload length in bytes.
  *
  * Return code
  *   0 - Successfully processed echo iocb (currently always return 0)
  **/
 static int
 lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
-		 struct lpfc_nodelist *ndlp)
+		 struct lpfc_nodelist *ndlp, u32 payload_len)
 {
 	struct lpfc_hba  *phba = vport->phba;
 	struct fc_els_edc *edc_req;
@@ -9423,25 +9424,39 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 	uint8_t *payload;
 	uint32_t *ptr, dtag;
 	const char *dtag_nm;
-	int desc_cnt = 0, bytes_remain;
+	int desc_cnt = 0;
+	u32 bytes_remain, desc_len, word2 = 0;
 	struct fc_diag_lnkflt_desc *plnkflt;
 
 	payload = cmdiocb->cmd_dmabuf->virt;
 
+	/* No signal support unless there is a congestion descriptor */
+	phba->cgn_reg_signal = EDC_CG_SIG_NOTSUPPORTED;
+	phba->cgn_sig_freq = 0;
+	phba->cgn_reg_fpin = LPFC_CGN_FPIN_ALARM | LPFC_CGN_FPIN_WARN;
+
+	if (payload_len < sizeof(*edc_req))
+		goto out;
+
 	edc_req = (struct fc_els_edc *)payload;
-	bytes_remain = be32_to_cpu(edc_req->desc_len);
+	desc_len = be32_to_cpu(edc_req->desc_len);
+	if (desc_len > payload_len - sizeof(*edc_req)) {
+		lpfc_printf_log(phba, KERN_WARNING,
+				LOG_ELS | LOG_CGN_MGMT | LOG_LDS_EVENT,
+				"6468 Bad EDC descriptor list length %u: %u\n",
+				desc_len, payload_len);
+		goto out;
+	}
+	bytes_remain = desc_len;
 
 	ptr = (uint32_t *)payload;
+	if (payload_len >= 3 * sizeof(*ptr))
+		word2 = be32_to_cpu(*(ptr + 2));
 	lpfc_printf_vlog(vport, KERN_INFO,
 			 LOG_ELS | LOG_CGN_MGMT | LOG_LDS_EVENT,
-			 "3319 Rcv EDC payload len %d: x%x x%x x%x\n",
+			 "3319 Rcv EDC payload len %u: x%x x%x x%x\n",
 			 bytes_remain, be32_to_cpu(*ptr),
-			 be32_to_cpu(*(ptr + 1)), be32_to_cpu(*(ptr + 2)));
-
-	/* No signal support unless there is a congestion descriptor */
-	phba->cgn_reg_signal = EDC_CG_SIG_NOTSUPPORTED;
-	phba->cgn_sig_freq = 0;
-	phba->cgn_reg_fpin = LPFC_CGN_FPIN_ALARM | LPFC_CGN_FPIN_WARN;
+			 be32_to_cpu(*(ptr + 1)), word2);
 
 	if (bytes_remain <= 0)
 		goto out;
@@ -9471,7 +9486,7 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 				lpfc_printf_log(phba, KERN_WARNING,
 					LOG_ELS | LOG_CGN_MGMT | LOG_LDS_EVENT,
 					"6465 Truncated Link Fault Diagnostic "
-					"descriptor[%d]: %d vs 0x%zx 0x%zx\n",
+					"descriptor[%d]: %u vs 0x%zx 0x%zx\n",
 					desc_cnt, bytes_remain,
 					FC_TLV_DESC_SZ_FROM_LENGTH(tlv),
 					sizeof(struct fc_diag_lnkflt_desc));
@@ -9497,7 +9512,7 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 				lpfc_printf_log(
 					phba, KERN_WARNING, LOG_CGN_MGMT,
 					"6466 Truncated cgn signal Diagnostic "
-					"descriptor[%d]: %d vs 0x%zx 0x%zx\n",
+					"descriptor[%d]: %u vs 0x%zx 0x%zx\n",
 					desc_cnt, bytes_remain,
 					FC_TLV_DESC_SZ_FROM_LENGTH(tlv),
 					sizeof(struct fc_diag_cg_sig_desc));
@@ -10815,7 +10830,7 @@ lpfc_els_unsol_buffer(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 		/* There are no replies, so no rjt codes */
 		break;
 	case ELS_CMD_EDC:
-		lpfc_els_rcv_edc(vport, elsiocb, ndlp);
+		lpfc_els_rcv_edc(vport, elsiocb, ndlp, payload_len);
 		break;
 	case ELS_CMD_RDF:
 		phba->fc_stat.elsRcvRDF++;

Re: [PATCH 1/2] scsi: lpfc: bound EDC descriptor list by payload length

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] An integer overflow in the FC_TLV_DESC_SZ_FROM_LENGTH macro allows an attacker to bypass bounds checks, resulting in kernel panics or infinite loops.
- [High] Missing bounds check on unknown TLV descriptors in lpfc_els_rcv_edc() and lpfc_cmpl_els_edc() allows KASAN out-of-bounds read.
--

--- Patch [1]: [PATCH 1/2] scsi: lpfc: bound EDC descriptor list by payload length ---
commit 12e45848685e8826e33fd1952148968b98cdc126
Author: Michael Bommarito <michael.bommarito@gmail.com>

    scsi: lpfc: bound EDC descriptor list by payload length
    
    drivers/scsi/lpfc/lpfc_els.c:lpfc_els_rcv_edc() trusts the EDC
    descriptor-list length from the received frame without checking that it
    fits in the actual ELS payload. An adjacent Fibre Channel fabric peer or
    device can send an unsolicited EDC frame with a short payload and an
    oversized descriptor-list length. The TLV walk can then read past the
    receive buffer and trip a KASAN slab-out-of-bounds read in the LPFC ELS
    receive path.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 52fc5058976d9..0023524528d93 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
[ ... ]
> @@ -9443,25 +9444,39 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
>  	uint8_t *payload;
>  	uint32_t *ptr, dtag;
>  	const char *dtag_nm;
> -	int desc_cnt = 0, bytes_remain;
> +	int desc_cnt = 0;
> +	u32 bytes_remain, desc_len, word2 = 0;

[Severity: High]
This is a pre-existing issue, but does changing bytes_remain to u32 bypass
the while loop condition if we encounter an unknown TLV descriptor?

If an adjacent peer sends an EDC frame with an unrecognized descriptor tag,
we hit the default case in lpfc_els_rcv_edc() (and lpfc_cmpl_els_edc()),
where we subtract the unvalidated parsed TLV length from bytes_remain:

drivers/scsi/lpfc/lpfc_els.c:lpfc_els_rcv_edc() {
    ...
		default:
			dtag_nm = lpfc_get_tlv_dtag_nm(dtag);
			...
		}
		bytes_remain -= FC_TLV_DESC_SZ_FROM_LENGTH(tlv);
		tlv = fc_tlv_next_desc(tlv);
    ...
}

Since bytes_remain is now u32, subtracting an unvalidated length could
underflow to a large positive value. This would bypass the 
bytes_remain < FC_TLV_DESC_HDR_SZ check on the next iteration and lead to
an out-of-bounds memory read.

[ ... ]
> @@ -9491,7 +9506,7 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
>  				lpfc_printf_log(phba, KERN_WARNING,
>  					LOG_ELS | LOG_CGN_MGMT | LOG_LDS_EVENT,
>  					"6465 Truncated Link Fault Diagnostic "
> -					"descriptor[%d]: %d vs 0x%zx 0x%zx\n",
> +					"descriptor[%d]: %u vs 0x%zx 0x%zx\n",
>  					desc_cnt, bytes_remain,
>  					FC_TLV_DESC_SZ_FROM_LENGTH(tlv),

[Severity: Critical]
This isn't a bug introduced by this patch, but can FC_TLV_DESC_SZ_FROM_LENGTH
overflow and bypass bounds checks entirely?

Looking at the macro definition:

include/uapi/scsi/fc/fc_els.h:
#define FC_TLV_DESC_SZ_FROM_LENGTH(tlv)		\
		(__be32_to_cpu((tlv)->desc_len) + FC_TLV_DESC_HDR_SZ)

It performs an unchecked addition of 8 to an attacker-controlled 32-bit
integer. If an attacker crafts a desc_len near the 32-bit maximum limit
(e.g., 0xFFFFFFF8), the macro could overflow to exactly 0 on 32-bit systems,
or 4GB on 64-bit systems.

In lpfc_els_rcv_fpin(), an overflow to 0 allows this bounds check to pass
incorrectly:

drivers/scsi/lpfc/lpfc_els.c:lpfc_els_rcv_fpin() {
    ...
	while (bytes_remain >= FC_TLV_DESC_HDR_SZ &&
	       bytes_remain >= FC_TLV_DESC_SZ_FROM_LENGTH(tlv)) {
    ...
}

Since the evaluated length is 0, neither bytes_remain nor the tlv pointer
would advance, causing an infinite loop. On 64-bit systems, this could lead
to a panic when the large offset is applied to the pointer.

>  					sizeof(struct fc_diag_lnkflt_desc));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710022932.3741311-1-michael.bommarito@gmail.com?part=1

[PATCH 2/2] scsi: lpfc: add KUnit coverage for EDC descriptor bounds

[Michael Bommarito]

Add KUnit coverage for lpfc_els_rcv_edc() descriptor-list bounds. The
tests live in lpfc_els.c so they can drive the real static EDC parser
without exporting test-only symbols.

The suite covers a valid congestion-signaling EDC descriptor and a
malformed EDC payload whose top-level descriptor-list length exceeds the
received payload. That malformed case documents the frame rejected by
the previous patch.

Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/scsi/Kconfig         |   7 ++
 drivers/scsi/lpfc/lpfc_els.c | 154 +++++++++++++++++++++++++++++++++++
 2 files changed, 161 insertions(+)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 19d0884479a24..e0f01ddd22707 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1163,6 +1163,13 @@ config SCSI_LPFC_DEBUG_FS
 	  This makes debugging information from the lpfc driver
 	  available via the debugfs filesystem.
 
+config LPFC_EDC_KUNIT_TEST
+	bool "KUnit tests for lpfc EDC descriptor bounds" if !KUNIT_ALL_TESTS
+	depends on KUNIT && SCSI_LPFC
+	default KUNIT_ALL_TESTS
+	help
+	  Internal validation coverage for lpfc EDC descriptor list bounds.
+
 source "drivers/scsi/elx/Kconfig"
 
 config SCSI_SIM710
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 5408b116f2d5a..0711f88da3f1a 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -12574,3 +12574,157 @@ lpfc_cmpl_els_uvem(struct lpfc_hba *phba, struct lpfc_iocbq *icmdiocb,
 	lpfc_els_free_iocb(phba, icmdiocb);
 	lpfc_nlp_put(ndlp);
 }
+
+#if IS_ENABLED(CONFIG_LPFC_EDC_KUNIT_TEST)
+#include <kunit/device.h>
+#include <kunit/test.h>
+
+static void lpfc_edc_kunit_prep_stub(struct lpfc_iocbq *cmdiocbq,
+				     struct lpfc_vport *vport,
+				     struct lpfc_dmabuf *bmp, u16 cmd_size,
+				     u32 did, u32 elscmd, u8 tmo,
+				     u8 expect_rsp)
+{
+}
+
+static void lpfc_edc_kunit_release_stub(struct lpfc_hba *phba,
+					struct lpfc_iocbq *iocbq)
+{
+}
+
+static int lpfc_edc_kunit_issue_mbox_stub(struct lpfc_hba *phba,
+					  LPFC_MBOXQ_t *mboxq,
+					  uint32_t flag)
+{
+	mempool_free(mboxq, phba->mbox_mem_pool);
+	return 0;
+}
+
+static void lpfc_edc_kunit_setup_hba(struct kunit *test,
+				     struct lpfc_hba *phba,
+				     struct lpfc_vport *vport)
+{
+	struct lpfc_iocbq *free_iocbs;
+	struct device *kdev;
+	int i;
+
+	spin_lock_init(&phba->hbalock);
+	INIT_LIST_HEAD(&phba->lpfc_iocb_list);
+	INIT_LIST_HEAD(&phba->elsbuf);
+	phba->link_state = LPFC_LINK_DOWN;
+	phba->sli_rev = LPFC_SLI_REV3;
+	phba->__lpfc_sli_prep_els_req_rsp = lpfc_edc_kunit_prep_stub;
+	phba->__lpfc_sli_release_iocbq = lpfc_edc_kunit_release_stub;
+	phba->lpfc_sli_issue_mbox = lpfc_edc_kunit_issue_mbox_stub;
+	phba->pport = vport;
+
+	free_iocbs = kunit_kzalloc(test, 4 * sizeof(*free_iocbs), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, free_iocbs);
+	for (i = 0; i < 4; i++)
+		list_add_tail(&free_iocbs[i].list, &phba->lpfc_iocb_list);
+
+	kdev = kunit_device_register(test, "lpfc_edc_test");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, kdev);
+	phba->lpfc_mbuf_pool = dma_pool_create("lpfc_edc_mbuf", kdev,
+					       LPFC_BPL_SIZE, 8, 0);
+	KUNIT_ASSERT_NOT_NULL(test, phba->lpfc_mbuf_pool);
+	phba->mbox_mem_pool = mempool_create_kmalloc_pool(1,
+							  sizeof(LPFC_MBOXQ_t));
+	KUNIT_ASSERT_NOT_NULL(test, phba->mbox_mem_pool);
+}
+
+static void lpfc_edc_kunit_teardown_hba(struct lpfc_hba *phba)
+{
+	if (phba->mbox_mem_pool)
+		mempool_destroy(phba->mbox_mem_pool);
+	if (phba->lpfc_mbuf_pool)
+		dma_pool_destroy(phba->lpfc_mbuf_pool);
+}
+
+static void lpfc_edc_kunit_run(struct kunit *test, bool malformed)
+{
+	struct lpfc_hba *phba;
+	struct lpfc_vport *vport;
+	struct lpfc_nodelist *ndlp;
+	struct lpfc_iocbq *cmdiocb;
+	struct lpfc_dmabuf *cmd_dmabuf;
+	struct fc_els_edc *edc;
+	struct fc_tlv_desc *tlv;
+	size_t payload_len;
+
+	phba = kunit_kzalloc(test, sizeof(*phba), GFP_KERNEL);
+	vport = kunit_kzalloc(test, sizeof(*vport), GFP_KERNEL);
+	ndlp = kunit_kzalloc(test, sizeof(*ndlp), GFP_KERNEL);
+	cmdiocb = kunit_kzalloc(test, sizeof(*cmdiocb), GFP_KERNEL);
+	cmd_dmabuf = kunit_kzalloc(test, sizeof(*cmd_dmabuf), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, phba);
+	KUNIT_ASSERT_NOT_NULL(test, vport);
+	KUNIT_ASSERT_NOT_NULL(test, ndlp);
+	KUNIT_ASSERT_NOT_NULL(test, cmdiocb);
+	KUNIT_ASSERT_NOT_NULL(test, cmd_dmabuf);
+
+	vport->phba = phba;
+	ndlp->nlp_DID = Fabric_Cntl_DID;
+	lpfc_edc_kunit_setup_hba(test, phba, vport);
+
+	if (malformed)
+		payload_len = sizeof(*edc) + FC_TLV_DESC_HDR_SZ;
+	else
+		payload_len = sizeof(*edc) +
+			      sizeof(struct fc_diag_cg_sig_desc);
+
+	cmd_dmabuf->virt = kunit_kzalloc(test, payload_len, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, cmd_dmabuf->virt);
+	INIT_LIST_HEAD(&cmd_dmabuf->list);
+
+	edc = cmd_dmabuf->virt;
+	edc->edc_cmd = ELS_EDC;
+	tlv = edc->desc;
+	if (malformed) {
+		edc->desc_len = cpu_to_be32(0x100);
+		tlv->desc_tag = cpu_to_be32(0xdeadbeef);
+		tlv->desc_len = cpu_to_be32(0);
+	} else {
+		struct fc_diag_cg_sig_desc *cgn = (void *)tlv;
+
+		edc->desc_len = cpu_to_be32(sizeof(*cgn));
+		cgn->desc_tag = cpu_to_be32(ELS_DTAG_CG_SIGNAL_CAP);
+		cgn->desc_len =
+			cpu_to_be32(FC_TLV_DESC_LENGTH_FROM_SZ(*cgn));
+		cgn->xmt_signal_capability =
+			cpu_to_be32(EDC_CG_SIG_NOTSUPPORTED);
+		cgn->xmt_signal_frequency.count =
+			cpu_to_be16(EDC_CG_SIGFREQ_CNT_MIN);
+		cgn->xmt_signal_frequency.units =
+			cpu_to_be16(EDC_CG_SIGFREQ_MSEC);
+	}
+
+	cmdiocb->cmd_dmabuf = cmd_dmabuf;
+	lpfc_els_rcv_edc(vport, cmdiocb, ndlp, payload_len);
+
+	KUNIT_EXPECT_TRUE(test, true);
+	lpfc_edc_kunit_teardown_hba(phba);
+}
+
+static void lpfc_edc_control_test(struct kunit *test)
+{
+	lpfc_edc_kunit_run(test, false);
+}
+
+static void lpfc_edc_oob_test(struct kunit *test)
+{
+	lpfc_edc_kunit_run(test, true);
+}
+
+static struct kunit_case lpfc_edc_test_cases[] = {
+	KUNIT_CASE(lpfc_edc_control_test),
+	KUNIT_CASE(lpfc_edc_oob_test),
+	{}
+};
+
+static struct kunit_suite lpfc_edc_test_suite = {
+	.name = "lpfc_edc",
+	.test_cases = lpfc_edc_test_cases,
+};
+kunit_test_suite(lpfc_edc_test_suite);
+#endif /* CONFIG_LPFC_EDC_KUNIT_TEST */

Re: [PATCH 2/2] scsi: lpfc: add KUnit coverage for EDC descriptor bounds

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The KUnit test leaks memory pools if an assertion fails because cleanup is deferred instead of registered with KUnit.
- [Medium] The KUnit test uses an uninitialized `ndlp` node, causing the test to silently abort early and mask a missing mock function crash.

Pre-existing issues:
- [High] Missing bounds check for unknown EDC diagnostic descriptors allows an underflow of `bytes_remain`, leading to an out-of-bounds memory read or infinite loop.
--

--- Patch [2]: [PATCH 2/2] scsi: lpfc: add KUnit coverage for EDC descriptor bounds ---
commit dd6d0b3cfc06c72b1e47a15c2af79c7992a6552f
Author: Michael Bommarito <michael.bommarito@gmail.com>

scsi: lpfc: add KUnit coverage for EDC descriptor bounds

This patch adds KUnit coverage for lpfc_els_rcv_edc() descriptor-list bounds.
The tests live in lpfc_els.c so they can drive the real static EDC parser
without exporting test-only symbols.

> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 0023524528d93..3193a2aaac2f1 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -12592,3 +12592,157 @@ lpfc_cmpl_els_uvem(struct lpfc_hba *phba, struct lpfc_iocbq *icmdiocb,

[ ... ]

> +static void lpfc_edc_kunit_setup_hba(struct kunit *test,
> +				     struct lpfc_hba *phba,
> +				     struct lpfc_vport *vport)
> +{

[ ... ]

> +	phba->lpfc_mbuf_pool = dma_pool_create("lpfc_edc_mbuf", kdev,
> +					       LPFC_BPL_SIZE, 8, 0);
> +	KUNIT_ASSERT_NOT_NULL(test, phba->lpfc_mbuf_pool);
> +	phba->mbox_mem_pool = mempool_create_kmalloc_pool(1,
> +							  sizeof(LPFC_MBOXQ_t));
> +	KUNIT_ASSERT_NOT_NULL(test, phba->mbox_mem_pool);
> +}

[Severity: Medium]
If any KUnit assertion fails later in lpfc_edc_kunit_run() (such as the
allocation check for cmd_dmabuf->virt), the test will abort immediately 
without executing lpfc_edc_kunit_teardown_hba(). Will this leak the dma
and memory pools created here? Should these be registered for automatic
cleanup with KUnit instead?

[ ... ]

> +static void lpfc_edc_kunit_run(struct kunit *test, bool malformed)
> +{
> +	struct lpfc_hba *phba;
> +	struct lpfc_vport *vport;
> +	struct lpfc_nodelist *ndlp;
> +	struct lpfc_iocbq *cmdiocb;
> +	struct lpfc_dmabuf *cmd_dmabuf;
> +	struct fc_els_edc *edc;
> +	struct fc_tlv_desc *tlv;
> +	size_t payload_len;
> +
> +	phba = kunit_kzalloc(test, sizeof(*phba), GFP_KERNEL);
> +	vport = kunit_kzalloc(test, sizeof(*vport), GFP_KERNEL);
> +	ndlp = kunit_kzalloc(test, sizeof(*ndlp), GFP_KERNEL);

[Severity: Medium]
Is the kref for this node list left as zero by the zero-allocation?

When lpfc_els_rcv_edc() calls lpfc_issue_els_edc_rsp() later in this path,
it attempts to take a reference via lpfc_nlp_get(ndlp). Because kref is
zero, won't this fail and return NULL, causing the function to silently abort
and skip sending the response? 

This early abort would mask a potential crash, as phba->__lpfc_sli_issue_iocb
does not appear to be stubbed in the lpfc_edc_kunit_setup_hba() mock setup.

> +	cmdiocb = kunit_kzalloc(test, sizeof(*cmdiocb), GFP_KERNEL);
> +	cmd_dmabuf = kunit_kzalloc(test, sizeof(*cmd_dmabuf), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, phba);
> +	KUNIT_ASSERT_NOT_NULL(test, vport);
> +	KUNIT_ASSERT_NOT_NULL(test, ndlp);
> +	KUNIT_ASSERT_NOT_NULL(test, cmdiocb);
> +	KUNIT_ASSERT_NOT_NULL(test, cmd_dmabuf);

[ ... ]

> +	cmdiocb->cmd_dmabuf = cmd_dmabuf;
> +	lpfc_els_rcv_edc(vport, cmdiocb, ndlp, payload_len);

[Severity: High]
This is a pre-existing issue, but missing bounds checks for unknown EDC
diagnostic descriptors in lpfc_els_rcv_edc() could allow an underflow of
bytes_remain.

In the target function:

lpfc_els.c:lpfc_els_rcv_edc() {
    ...
    while (bytes_remain) {
        ...
        switch (dtag) {
        ...
        default:
            dtag_nm = lpfc_get_tlv_dtag_nm(dtag);
            lpfc_printf_log(phba, KERN_WARNING, ...);
        }
        bytes_remain -= FC_TLV_DESC_SZ_FROM_LENGTH(tlv);
        tlv = fc_tlv_next_desc(tlv);
    ...
}

If a remote FC node sends an unknown descriptor tag whose length is maliciously
crafted to be larger than the remaining payload, won't bytes_remain underflow
to a huge value since there is no bounds check in the default switch case?
This could bypass the loop termination and allow an out-of-bounds read on the
next iteration.

> +
> +	KUNIT_EXPECT_TRUE(test, true);
> +	lpfc_edc_kunit_teardown_hba(phba);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710022932.3741311-1-michael.bommarito@gmail.com?part=2

Re: [PATCH 0/2] scsi: lpfc: bound EDC descriptor TLV walk

[Justin Tee]

Hi Michael,

There are already current plans to address this concern in an upcoming
lpfc version update.  Please stay tuned when we post the version
update.

Regards,
Justin

On Thu, Jul 9, 2026 at 7:30 PM Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> An adjacent Fibre Channel fabric peer or device can crash an LPFC host
> with a malformed EDC ELS frame. lpfc_els_rcv_edc() trusts the EDC
> descriptor-list length from the received frame without checking that it
> fits in the actual ELS payload, so a short frame with an oversized
> descriptor-list length walks the TLV list past the receive buffer and
> trips a KASAN slab-out-of-bounds read in the ELS receive path.
>
> Patch 1 passes the received payload length into lpfc_els_rcv_edc(),
> rejects truncated EDC headers and descriptor lists larger than the
> payload, and avoids logging a third payload word unless it is present.
> Patch 2 adds same-translation-unit KUnit/KASAN coverage: a benign EDC
> frame that must still parse and the malformed frame that must now be
> rejected.
>
> Reproduced with the KUnit/KASAN test on f5459048c38a: stock trips
> BUG: KASAN: slab-out-of-bounds in lpfc_els_rcv_edc after the benign
> control passes; patched rejects the frame and both cases pass.
>
> Cc: stable@vger.kernel.org
>
> Michael Bommarito (2):
>   scsi: lpfc: bound EDC descriptor list by payload length
>   scsi: lpfc: add KUnit coverage for EDC descriptor bounds
>
>  drivers/scsi/Kconfig         |   7 ++
>  drivers/scsi/lpfc/lpfc_els.c | 195 ++++++++++++++++++++++++++++++++---
>  2 files changed, 189 insertions(+), 13 deletions(-)
>
> --
> 2.53.0
>