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

Michael Bommarito posted 1 patch 6 days, 13 hours ago
drivers/scsi/fcoe/fcoe_ctlr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker
Posted by Michael Bommarito 6 days, 13 hours ago
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
Posted by Martin K. Petersen 2 days, 1 hour ago
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
Re: [PATCH v2] scsi: fcoe: reject FIP descriptors with zero fip_dlen in CVL walker
Posted by Hannes Reinecke 5 days, 19 hours ago
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