[PATCH] scsi: scsi_transport_fc: reject truncated FPIN payloads

[PATCH] scsi: scsi_transport_fc: reject truncated FPIN payloads

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

fc_host_fpin_rcv() computes the length of the TLV area as

	bytes_remain = fpin_len - offsetof(struct fc_els_fpin, fpin_desc);

without checking that the payload is at least as large as the fixed
FC-ELS header.  For fpin_len smaller than the header size the subtraction
underflows to a huge u32, and the min_t() on the next line also reads
fpin->desc_len at offset 4, which is outside the payload when fpin_len is
4 or less.  The TLV walker then parses memory that is not part of the FPIN.

Reject payloads that cannot hold the header: there is no descriptor list
to parse and no statistics to update.  This is not only defensive:
qla2xxx forwards the firmware-provided frame size minus the 24-byte PURX
ELS header, so a frame of exactly that size reaches this helper with a
zero length.  fpin_len is supplied by the LLDD and the helper is exported,
so it should not assume a minimum.

Fixes: 3dcfe0de5a97 ("scsi: fc: Parse FPIN packets and update statistics")
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index bec6ca38375b..a935e0c723c2 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -904,8 +904,11 @@ fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf,
 	u32 dtag;
 	enum fc_host_event_code event_code =
 		event_acknowledge ? FCH_EVT_LINK_FPIN_ACK : FCH_EVT_LINK_FPIN;

+	if (fpin_len < offsetof(struct fc_els_fpin, fpin_desc))
+		return;
+
 	/* Update Statistics */
 	tlv = (struct fc_tlv_desc *)&fpin->fpin_desc[0];
 	bytes_remain = fpin_len - offsetof(struct fc_els_fpin, fpin_desc);
 	bytes_remain = min_t(u32, bytes_remain, be32_to_cpu(fpin->desc_len));

Re: [PATCH] scsi: scsi_transport_fc: reject truncated FPIN payloads

[Martin K. Petersen]

> fc_host_fpin_rcv() computes the length of the TLV area as
>
> 	bytes_remain = fpin_len - offsetof(struct fc_els_fpin, fpin_desc);
>
> without checking that the payload is at least as large as the fixed
> FC-ELS header.

Applied to 7.5/scsi-staging, thanks!

-- 
Martin K. Petersen