On 20/05/2026 14:30, Michael Bommarito wrote:
An adjacent Fibre Channel fabric actor that can deliver an FPIN ELS
frame to an lpfc or qla2xxx Linux initiator can trigger a non-return
in the generic FC transport. This is not a local userspace or IP
network path; the attacker must be able to inject fabric traffic, for
example as a compromised switch or fabric controller, or as a same-zone
N_Port on a fabric that permits source spoofing.
The Link-Integrity and Peer-Congestion FPIN walkers used a u8 loop
counter against the 32-bit on-wire pname_count field, and did not bound
pname_count by the descriptor body already validated by the TLV walker.
A pname_count of 256 therefore wraps the counter and keeps the loop
condition true indefinitely.
Factor the shared pname_list[] walk into one helper, widen the counter
to u32, and clamp pname_count against the entries that fit in the
descriptor body before iterating.
Fixes: 3dcfe0de5a97 ("scsi: fc: Parse FPIN packets and update statistics")
Cc:[email protected]
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito<[email protected]>
Regardless of nitpick below:
Reviewed-by: John Garry <[email protected]>
---
Changes in v4:
- Use min() rather than min_t(u32, ...) for the pname_count clamp and
fold away the temporary max_count variable, as David Laight suggested.
Changes in v3:
- State the fabric-adjacent threat model explicitly in the commit
message and clarify that this is not local userspace or IP-network
reachable.
- Use min_t(u32, ...) for the pname_count clamp, as Christoph suggested.
- Use FC_TLV_DESC_LENGTH_FROM_SZ() instead of open-coding the descriptor
body length calculation.
- Factor the duplicate LI and peer-congestion pname walker into a common
helper while preserving the LI-only host-stat update.
Changes in v2:
- Drop the redundant cover letter shipped with v1. A single-patch send
does not need one, and the v1 cover carried stale draft markers.
drivers/scsi/scsi_transport_fc.c | 77 +++++++++++++++++---------------
1 file changed, 41 insertions(+), 36 deletions(-)
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index dce95e361daf0..173ed6373f04b 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -737,6 +737,37 @@ fc_cn_stats_update(u16 event_type, struct fc_fpin_stats
*stats)
}
}
+static void
+fc_fpin_pname_stats_update(struct Scsi_Host *shost,
+ struct fc_rport *attach_rport, u16 event_type,
+ u32 desc_len, u32 fixed_len, u32 pname_count,
+ __be64 *pname_list,
+ void (*stats_update)(u16 event_type,
+ struct fc_fpin_stats *stats))
+{
+ u32 i;
+ struct fc_rport *rport;
+ u64 wwpn;
+
+ if (desc_len < fixed_len)
+ pname_count = 0;
you could return directly here to avoid extra indentation in else leg
+ else
+ pname_count = min(pname_count, (desc_len - fixed_len) /
+ sizeof(pname_list[0]));
+
+ for (i = 0; i < pname_count; i++) {
+ wwpn = be64_to_cpu(pname_list[i]);
+ rport = fc_find_rport_by_wwpn(shost, wwpn);
+ if (rport &&
+ (rport->roles & FC_PORT_ROLE_FCP_TARGET ||
+ rport->roles & FC_PORT_ROLE_NVME_TARGET)) {
+ if (rport == attach_rport)
+ continue;
+ stats_update(event_type, &rport->fpin_stats);
+ }
+ }
+}
