drivers/scsi/scsi_transport_fc.c | 81 ++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 30 deletions(-)
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: 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-hardening/20260518140945.2751273-1-michael.bommarito@gmail.com/
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> 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/20260518140945.2751273-1-michael.bommarito@gmail.com/ > The patch hunks below are byte-identical to v1's 0001. Cover letters for single patches are not required, but totally fine if they add value. I don't think you needed one here, but it also wasn't actually harmful.
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: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
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..0684d8c69c3c6 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, max_count;
+ struct fc_rport *rport;
+ u64 wwpn;
+
+ if (desc_len < fixed_len)
+ max_count = 0;
+ else
+ max_count = (desc_len - fixed_len) / sizeof(pname_list[0]);
+ pname_count = min_t(u32, pname_count, max_count);
+
+ 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);
+ }
+ }
+}
+
/*
* fc_fpin_li_stats_update - routine to update Link Integrity
* event statistics.
@@ -747,13 +778,11 @@ 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;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
struct fc_fn_li_desc *li_desc = (struct fc_fn_li_desc *)tlv;
u16 event_type = be16_to_cpu(li_desc->event_type);
- u64 wwpn;
rport = fc_find_rport_by_wwpn(shost,
be64_to_cpu(li_desc->attached_wwpn));
@@ -764,22 +793,11 @@ 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);
- }
- }
- }
+ fc_fpin_pname_stats_update(shost, attach_rport, event_type,
+ be32_to_cpu(li_desc->desc_len),
+ FC_TLV_DESC_LENGTH_FROM_SZ(*li_desc),
+ be32_to_cpu(li_desc->pname_count),
+ li_desc->pname_list, fc_li_stats_update);
if (fc_host->port_name == be64_to_cpu(li_desc->attached_wwpn))
fc_li_stats_update(event_type, &fc_host->fpin_stats);
@@ -827,13 +845,11 @@ static void
fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
struct fc_tlv_desc *tlv)
{
- u8 i;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_fn_peer_congn_desc *pc_desc =
(struct fc_fn_peer_congn_desc *)tlv;
u16 event_type = be16_to_cpu(pc_desc->event_type);
- u64 wwpn;
rport = fc_find_rport_by_wwpn(shost,
be64_to_cpu(pc_desc->attached_wwpn));
@@ -844,22 +860,11 @@ 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);
- }
- }
- }
+ fc_fpin_pname_stats_update(shost, attach_rport, event_type,
+ be32_to_cpu(pc_desc->desc_len),
+ FC_TLV_DESC_LENGTH_FROM_SZ(*pc_desc),
+ be32_to_cpu(pc_desc->pname_count),
+ pc_desc->pname_list, fc_cn_stats_update);
}
/*
--
2.53.0On Tue, 19 May 2026 15:06:15 -0400
Michael Bommarito <michael.bommarito@gmail.com> 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: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> 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..0684d8c69c3c6 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, max_count;
> + struct fc_rport *rport;
> + u64 wwpn;
> +
> + if (desc_len < fixed_len)
> + max_count = 0;
> + else
> + max_count = (desc_len - fixed_len) / sizeof(pname_list[0]);
> + pname_count = min_t(u32, pname_count, max_count);
No min_t() please, everything is unsigned so min() in fine.
The above might even more readable without the extra variable:
if (desc_len < fixed_len)
pname_count = min(pname_count, (desc_len - fixed_len) / sizeof(pname_list[0]));
If you think the line is too long s/pname_count/count/g
-- David
> +
> + 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);
> + }
> + }
> +}
> +
> /*
> * fc_fpin_li_stats_update - routine to update Link Integrity
> * event statistics.
> @@ -747,13 +778,11 @@ 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;
> struct fc_rport *rport = NULL;
> struct fc_rport *attach_rport = NULL;
> struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
> struct fc_fn_li_desc *li_desc = (struct fc_fn_li_desc *)tlv;
> u16 event_type = be16_to_cpu(li_desc->event_type);
> - u64 wwpn;
>
> rport = fc_find_rport_by_wwpn(shost,
> be64_to_cpu(li_desc->attached_wwpn));
> @@ -764,22 +793,11 @@ 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);
> - }
> - }
> - }
> + fc_fpin_pname_stats_update(shost, attach_rport, event_type,
> + be32_to_cpu(li_desc->desc_len),
> + FC_TLV_DESC_LENGTH_FROM_SZ(*li_desc),
> + be32_to_cpu(li_desc->pname_count),
> + li_desc->pname_list, fc_li_stats_update);
>
> if (fc_host->port_name == be64_to_cpu(li_desc->attached_wwpn))
> fc_li_stats_update(event_type, &fc_host->fpin_stats);
> @@ -827,13 +845,11 @@ static void
> fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
> struct fc_tlv_desc *tlv)
> {
> - u8 i;
> struct fc_rport *rport = NULL;
> struct fc_rport *attach_rport = NULL;
> struct fc_fn_peer_congn_desc *pc_desc =
> (struct fc_fn_peer_congn_desc *)tlv;
> u16 event_type = be16_to_cpu(pc_desc->event_type);
> - u64 wwpn;
>
> rport = fc_find_rport_by_wwpn(shost,
> be64_to_cpu(pc_desc->attached_wwpn));
> @@ -844,22 +860,11 @@ 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);
> - }
> - }
> - }
> + fc_fpin_pname_stats_update(shost, attach_rport, event_type,
> + be32_to_cpu(pc_desc->desc_len),
> + FC_TLV_DESC_LENGTH_FROM_SZ(*pc_desc),
> + be32_to_cpu(pc_desc->pname_count),
> + pc_desc->pname_list, fc_cn_stats_update);
> }
>
> /*
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: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
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;
+ 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);
+ }
+ }
+}
+
/*
* fc_fpin_li_stats_update - routine to update Link Integrity
* event statistics.
@@ -747,13 +778,11 @@ 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;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
struct fc_fn_li_desc *li_desc = (struct fc_fn_li_desc *)tlv;
u16 event_type = be16_to_cpu(li_desc->event_type);
- u64 wwpn;
rport = fc_find_rport_by_wwpn(shost,
be64_to_cpu(li_desc->attached_wwpn));
@@ -764,22 +793,11 @@ 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);
- }
- }
- }
+ fc_fpin_pname_stats_update(shost, attach_rport, event_type,
+ be32_to_cpu(li_desc->desc_len),
+ FC_TLV_DESC_LENGTH_FROM_SZ(*li_desc),
+ be32_to_cpu(li_desc->pname_count),
+ li_desc->pname_list, fc_li_stats_update);
if (fc_host->port_name == be64_to_cpu(li_desc->attached_wwpn))
fc_li_stats_update(event_type, &fc_host->fpin_stats);
@@ -827,13 +845,11 @@ static void
fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
struct fc_tlv_desc *tlv)
{
- u8 i;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_fn_peer_congn_desc *pc_desc =
(struct fc_fn_peer_congn_desc *)tlv;
u16 event_type = be16_to_cpu(pc_desc->event_type);
- u64 wwpn;
rport = fc_find_rport_by_wwpn(shost,
be64_to_cpu(pc_desc->attached_wwpn));
@@ -844,22 +860,11 @@ 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);
- }
- }
- }
+ fc_fpin_pname_stats_update(shost, attach_rport, event_type,
+ be32_to_cpu(pc_desc->desc_len),
+ FC_TLV_DESC_LENGTH_FROM_SZ(*pc_desc),
+ be32_to_cpu(pc_desc->pname_count),
+ pc_desc->pname_list, fc_cn_stats_update);
}
/*
--
2.53.0On 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:stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito<michael.bommarito@gmail.com>
Regardless of nitpick below:
Reviewed-by: John Garry <john.g.garry@oracle.com>
> ---
> 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);
> + }
> + }
> +}
© 2016 - 2026 Red Hat, Inc.