drivers/scsi/scsi_transport_fc.c::fc_fpin_li_stats_update() and
fc_fpin_peer_congn_stats_update() walked the on-wire
pname_list[] with a u8 loop counter against the 32-bit __be32
pname_count field, and never bounded pname_count by the
descriptor body the TLV walker already validated.
A fabric-side FPIN sender (the elected fabric controller, a
co-tenant N_Port that spoofs S_ID 0xFFFFFD after FLOGI, or a
compromised switch supervisor) could hang the FC ELS receive
thread of an lpfc or qla2xxx initiator indefinitely by
emitting one FPIN ELS frame whose Link-Integrity or
Peer-Congestion descriptor sets pname_count to 256, because
the u8 counter rolls over and the loop condition i <
pname_count stays true for every value i can take.
Widen the loop counter to u32 in both walkers and clamp
pname_count against the per-descriptor available bytes
(desc_len minus the fixed body that precedes pname_list[])
before iterating. A malformed descriptor that claims more
entries than its TLV body can hold is rejected by the clamp.
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]>
---
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-hardening/[email protected]/
The patch hunks below are byte-identical to v1's 0001.
drivers/scsi/scsi_transport_fc.c | 81 ++++++++++++++++++++------------
1 file changed, 51 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index dce95e361daf0..cef0c85693fd4 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -747,7 +747,7 @@ fc_cn_stats_update(u16 event_type, struct fc_fpin_stats
*stats)
static void
fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
{
- u8 i;
+ u32 i, pname_count, max_count, desc_len;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
@@ -764,20 +764,34 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct
fc_tlv_desc *tlv)
fc_li_stats_update(event_type, &attach_rport->fpin_stats);
}
- if (be32_to_cpu(li_desc->pname_count) > 0) {
- for (i = 0;
- i < be32_to_cpu(li_desc->pname_count);
- i++) {
- wwpn = be64_to_cpu(li_desc->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;
- fc_li_stats_update(event_type,
- &rport->fpin_stats);
- }
+ /*
+ * Clamp pname_count to the number of pname_list entries
+ * the descriptor body can actually hold. desc_len
+ * excludes the desc_tag/desc_len header (FC_TLV_DESC_HDR_SZ),
+ * so the bytes available for pname_list[] are
+ * desc_len - sizeof(fixed fields before pname_list[]).
+ */
+ pname_count = be32_to_cpu(li_desc->pname_count);
+ desc_len = be32_to_cpu(li_desc->desc_len);
+ if (desc_len < sizeof(*li_desc) - FC_TLV_DESC_HDR_SZ)
+ max_count = 0;
+ else
+ max_count = (desc_len -
+ (sizeof(*li_desc) - FC_TLV_DESC_HDR_SZ)) /
+ sizeof(li_desc->pname_list[0]);
+ if (pname_count > max_count)
+ pname_count = max_count;
+
+ for (i = 0; i < pname_count; i++) {
+ wwpn = be64_to_cpu(li_desc->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;
+ fc_li_stats_update(event_type,
+ &rport->fpin_stats);
}
}
@@ -827,7 +841,7 @@ static void
fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
struct fc_tlv_desc *tlv)
{
- u8 i;
+ u32 i, pname_count, max_count, desc_len;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_fn_peer_congn_desc *pc_desc =
@@ -844,20 +858,27 @@ fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
fc_cn_stats_update(event_type, &attach_rport->fpin_stats);
}
- if (be32_to_cpu(pc_desc->pname_count) > 0) {
- for (i = 0;
- i < be32_to_cpu(pc_desc->pname_count);
- i++) {
- wwpn = be64_to_cpu(pc_desc->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;
- fc_cn_stats_update(event_type,
- &rport->fpin_stats);
- }
+ pname_count = be32_to_cpu(pc_desc->pname_count);
+ desc_len = be32_to_cpu(pc_desc->desc_len);
+ if (desc_len < sizeof(*pc_desc) - FC_TLV_DESC_HDR_SZ)
+ max_count = 0;
+ else
+ max_count = (desc_len -
+ (sizeof(*pc_desc) - FC_TLV_DESC_HDR_SZ)) /
+ sizeof(pc_desc->pname_list[0]);
+ if (pname_count > max_count)
+ pname_count = max_count;
+
+ for (i = 0; i < pname_count; i++) {
+ wwpn = be64_to_cpu(pc_desc->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;
+ fc_cn_stats_update(event_type,
+ &rport->fpin_stats);
}
}
}
--
2.53.0
