[DRAFT][PATCH] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker

[DRAFT][PATCH] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker

[Michael Bommarito]

drivers/scsi/fcoe/fcoe_ctlr.c::fcoe_ctlr_recv_clr_vlink() advances
the descriptor cursor by an attacker-supplied fip_dlen without
ever requiring dlen >= sizeof(struct fip_desc) in the default
branch.  The named descriptor cases (FIP_DT_MAC, FIP_DT_NAME,
FIP_DT_VN_ID) check their per-type minimum lengths, but a
FIP_DT_NON_CRITICAL descriptor (fip_dtype >= 128, which the
standard requires receivers to silently ignore) skips that check
entirely.

The function is reached on every host that has selected an FCoE
Forwarder and logged into the fabric: any L2 peer on the FCoE
control VLAN that spoofs the elected FCF source MAC, or wins
FIP election, can deliver a CVL frame; FIP frames are not
cryptographically authenticated.

A FIP CVL frame with one FIP_DT_NON_CRITICAL descriptor whose
fip_dlen == 0 leaves desc and rlen unchanged after one loop
iteration, so the loop condition rlen >= sizeof(*desc) stays
true forever and fcoe_ctlr_recv_work never returns.

Impact: an unauthenticated L2 peer on the FCoE control VLAN can
hang fcoe_ctlr_recv_work on an fcoe, qedf, or bnx2fc initiator
indefinitely by emitting one FIP CVL frame whose single
descriptor has fip_dtype == FIP_DT_NON_CRITICAL and
fip_dlen == 0, blocking every subsequent FIP frame (FCF
keepalives, FLOGI, FDISC, real CVLs) on that controller and,
once the fabric ages out the session, leaving FCoE storage on
the affected initiator unavailable until reboot.

Reject the descriptor in the default branch when fip_dlen *
FIP_BPW is less than sizeof(struct fip_desc), i.e. when the
attacker-supplied length cannot even cover the descriptor
header.  This is the same lower-bound that the named cases
already apply and is the minimum scope that closes the loop.

I reproduced this on a KASAN-enabled x86_64 mainline kernel at
f0db6484b6ea via an out-of-tree module that initialises a real
struct fcoe_ctlr through fcoe_ctlr_init(FIP_MODE_FABRIC),
installs a fcoe_fcf with fcf_mac and switch_name, sets
ctlr->state = FIP_ST_ENABLED and lp->port_id, then queues a
crafted FIP CVL skb whose single descriptor has fip_dtype ==
FIP_DT_NON_CRITICAL and fip_dlen == 0, and calls the exported
fcoe_ctlr_recv() from the init thread.  Without the patch, a
bounded watchdog timer fires three seconds into the call with
the workqueue still inside fcoe_ctlr_recv_work (RIP
fcoe_ctlr_recv_work+0x1161/0x34d0 in [libfcoe]).  The
patched-kernel A/B run, the legitimate non-critical-descriptor
regression (fip_dlen == 1) run, and the checkpatch and
get_maintainer outputs are pending the final patch draft and
will be captured before send.  A reproducer is available
off-list on request.

Three driver-private walkers in qedf and fnic share the same
algorithmic invariant (qedf_fcoe_process_vlan_resp at
drivers/scsi/qedf/qedf_fip.c:90, qedf CVL walker at
qedf_fip.c:233, fnic_fcoe_process_vlan_resp at
drivers/scsi/fnic/fip.c:117).  Those are companion fixes for a
separate posting; they need the same dlen lower bound in their
own walker bodies and live-evidence on qedf or fnic hardware.

Fixes: 97c8389d54b9 ("[SCSI] fcoe, libfcoe: Add support for FIP. FCoE discovery and keep-alive.")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>

[PATCH] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker

[Michael Bommarito]

drivers/scsi/fcoe/fcoe_ctlr.c::fcoe_ctlr_recv_clr_vlink() advanced
the descriptor cursor by an attacker-supplied fip_dlen without
ever requiring dlen >= sizeof(struct fip_desc) in the default
branch.  The named descriptor cases (FIP_DT_MAC, FIP_DT_NAME,
FIP_DT_VN_ID) checked their per-type minimum lengths, but a
FIP_DT_NON_CRITICAL descriptor (fip_dtype >= 128, which the
standard requires receivers to silently ignore) skipped that
check entirely.

An unauthenticated L2 peer on the FCoE control VLAN could hang
fcoe_ctlr_recv_work on an fcoe, qedf, or bnx2fc initiator
indefinitely by emitting one FIP CVL frame whose single
descriptor had fip_dtype == FIP_DT_NON_CRITICAL and fip_dlen
== 0: the cursor advanced zero bytes per iteration and the
loop condition rlen >= sizeof(*desc) stayed true forever,
blocking every subsequent FIP frame on that controller.

Tighten the outer dlen guard to also reject dlen <
sizeof(struct fip_desc), so a malformed descriptor whose
length cannot even cover the descriptor header is rejected
before the switch.  This is the same lower-bound the named
cases already apply and is the minimum scope that closes the
loop.

Fixes: 97c8389d54b9 ("[SCSI] fcoe, libfcoe: Add support for FIP. FCoE discovery and keep-alive.")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/scsi/fcoe/fcoe_ctlr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 02cd4410efca7..496ddd45f74da 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -1385,7 +1385,7 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 
 	while (rlen >= sizeof(*desc)) {
 		dlen = desc->fip_dlen * FIP_BPW;
-		if (dlen > rlen)
+		if (dlen < sizeof(*desc) || dlen > rlen)
 			goto err;
 		/* Drop CVL if there are duplicate critical descriptors */
 		if ((desc->fip_dtype < 32) &&
-- 
2.53.0

[PATCH v2] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker

[Michael Bommarito]

drivers/scsi/fcoe/fcoe_ctlr.c::fcoe_ctlr_recv_clr_vlink() advanced
the descriptor cursor by an attacker-supplied fip_dlen without
ever requiring dlen >= sizeof(struct fip_desc) in the default
branch.  The named descriptor cases (FIP_DT_MAC, FIP_DT_NAME,
FIP_DT_VN_ID) checked their per-type minimum lengths, but a
FIP_DT_NON_CRITICAL descriptor (fip_dtype >= 128, which the
standard requires receivers to silently ignore) skipped that
check entirely.

An unauthenticated L2 peer on the FCoE control VLAN could hang
fcoe_ctlr_recv_work on an fcoe, qedf, or bnx2fc initiator
indefinitely by emitting one FIP CVL frame whose single
descriptor had fip_dtype == FIP_DT_NON_CRITICAL and fip_dlen
== 0: the cursor advanced zero bytes per iteration and the
loop condition rlen >= sizeof(*desc) stayed true forever,
blocking every subsequent FIP frame on that controller.

Tighten the outer dlen guard to also reject dlen <
sizeof(struct fip_desc), so a malformed descriptor whose
length cannot even cover the descriptor header is rejected
before the switch.  This is the same lower-bound the named
cases already apply and is the minimum scope that closes the
loop.

Fixes: 97c8389d54b9 ("[SCSI] fcoe, libfcoe: Add support for FIP. FCoE discovery and keep-alive.")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2: drop the redundant cover letter shipped with v1.  A
    single-patch send should not carry a cover; the lead
    belongs in the commit message, which the patch below
    already has.  The v1 cover also carried stale drafting-
    time envelope markers that should have been stripped
    before send.  Apologies for the noise; please ignore the
    v1 cover at
    https://lore.kernel.org/linux-scsi/20260518141150.2755252-1-michael.bommarito@gmail.com/
    The patch hunk below is byte-identical to v1's 0001.

 drivers/scsi/fcoe/fcoe_ctlr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 02cd4410efca7..496ddd45f74da 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -1385,7 +1385,7 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 
 	while (rlen >= sizeof(*desc)) {
 		dlen = desc->fip_dlen * FIP_BPW;
-		if (dlen > rlen)
+		if (dlen < sizeof(*desc) || dlen > rlen)
 			goto err;
 		/* Drop CVL if there are duplicate critical descriptors */
 		if ((desc->fip_dtype < 32) &&
-- 
2.53.0

Re: [PATCH v2] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker

[Hannes Reinecke]

On 5/18/26 16:43, Michael Bommarito wrote:
> drivers/scsi/fcoe/fcoe_ctlr.c::fcoe_ctlr_recv_clr_vlink() advanced
> the descriptor cursor by an attacker-supplied fip_dlen without
> ever requiring dlen >= sizeof(struct fip_desc) in the default
> branch.  The named descriptor cases (FIP_DT_MAC, FIP_DT_NAME,
> FIP_DT_VN_ID) checked their per-type minimum lengths, but a
> FIP_DT_NON_CRITICAL descriptor (fip_dtype >= 128, which the
> standard requires receivers to silently ignore) skipped that
> check entirely.
> 
> An unauthenticated L2 peer on the FCoE control VLAN could hang
> fcoe_ctlr_recv_work on an fcoe, qedf, or bnx2fc initiator
> indefinitely by emitting one FIP CVL frame whose single
> descriptor had fip_dtype == FIP_DT_NON_CRITICAL and fip_dlen
> == 0: the cursor advanced zero bytes per iteration and the
> loop condition rlen >= sizeof(*desc) stayed true forever,
> blocking every subsequent FIP frame on that controller.
> 
> Tighten the outer dlen guard to also reject dlen <
> sizeof(struct fip_desc), so a malformed descriptor whose
> length cannot even cover the descriptor header is rejected
> before the switch.  This is the same lower-bound the named
> cases already apply and is the minimum scope that closes the
> loop.
> 
> Fixes: 97c8389d54b9 ("[SCSI] fcoe, libfcoe: Add support for FIP. FCoE discovery and keep-alive.")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> v2: drop the redundant cover letter shipped with v1.  A
>      single-patch send should not carry a cover; the lead
>      belongs in the commit message, which the patch below
>      already has.  The v1 cover also carried stale drafting-
>      time envelope markers that should have been stripped
>      before send.  Apologies for the noise; please ignore the
>      v1 cover at
>      https://lore.kernel.org/linux-scsi/20260518141150.2755252-1-michael.bommarito@gmail.com/
>      The patch hunk below is byte-identical to v1's 0001.
> 
>   drivers/scsi/fcoe/fcoe_ctlr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
> index 02cd4410efca7..496ddd45f74da 100644
> --- a/drivers/scsi/fcoe/fcoe_ctlr.c
> +++ b/drivers/scsi/fcoe/fcoe_ctlr.c
> @@ -1385,7 +1385,7 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
>   
>   	while (rlen >= sizeof(*desc)) {
>   		dlen = desc->fip_dlen * FIP_BPW;
> -		if (dlen > rlen)
> +		if (dlen < sizeof(*desc) || dlen > rlen)
>   			goto err;
>   		/* Drop CVL if there are duplicate critical descriptors */
>   		if ((desc->fip_dtype < 32) &&

You could just have sent the patch; no need to have such an elaborate 
description for a simple buffer underflow...

But anyway.

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v2] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker

[Martin K. Petersen]

On Mon, 18 May 2026 10:43:07 -0400, Michael Bommarito wrote:

> drivers/scsi/fcoe/fcoe_ctlr.c::fcoe_ctlr_recv_clr_vlink() advanced
> the descriptor cursor by an attacker-supplied fip_dlen without
> ever requiring dlen >= sizeof(struct fip_desc) in the default
> branch.  The named descriptor cases (FIP_DT_MAC, FIP_DT_NAME,
> FIP_DT_VN_ID) checked their per-type minimum lengths, but a
> FIP_DT_NON_CRITICAL descriptor (fip_dtype >= 128, which the
> standard requires receivers to silently ignore) skipped that
> check entirely.
> 
> [...]

Applied to 7.1/scsi-fixes, thanks!

[1/1] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker
      https://git.kernel.org/mkp/scsi/c/9eed1bd59937

-- 
Martin K. Petersen