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

[Hannes Reinecke <hare@suse.de>]

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