[PATCH v5 2/3] scsi: bfa: drop unsolicited frames shorter than the headers they must hold

[Yehyeong Lee]

bfa_fcs_uf_recv() keeps the received frame length in a u16 and hands it,
with a pointer to the frame, to every unsolicited frame consumer. Two
lengths are never checked.

The frame is not checked against the FC header that every consumer reads.
bfa_fcs_fabric_uf_recv() takes fchs->d_id and els_cmd->els_code out of it
before dispatching, and the handlers below take the length as a size. The
worst of those is bfa_fcs_lport_echo():

	pyld_len = rx_len - sizeof(struct fchs_s);

pyld_len is an int, so a frame shorter than the FC header makes it
negative. The memcpy below is skipped because the comparison it guards is
signed, but the value is then passed to bfa_fcxp_send(), whose reqlen
parameter is a u32:

	reqi->req_tot_len = reqlen;
	...
	send_req->req_len = cpu_to_be32(reqi->req_tot_len);
	bfa_alen_set(&send_req->req_alen, reqi->req_tot_len,
		     BFA_FCXP_REQ_PLD_PA(fcxp));

so a six-byte frame asks the firmware to send roughly four gigabytes
starting at the fcxp request payload address. The response side of the
same function sanity checks its length against BFA_FCXP_MAX_LBUF_SZ; the
request side does not.

The frame is also not checked against the virtual fabric tagging header
it may advertise. For a tagged frame the function steps over that header:

	fchs = (struct fchs_s *) (vft + 1);
	len -= sizeof(struct fc_vft_s);

and a frame too short to hold the header makes that subtraction wrap, so
the length comes out near 65530 instead of going negative.

Drop a frame that cannot hold the headers it has to hold: the FC header
before anything reads it, and, for a tagged frame, the VFT header plus
the FC header that has to follow it. Both drops use the idiom already in
this function for a frame whose virtual fabric id is unknown. A normal
frame is not affected; the FC header is always 24 bytes and a tagged one
carries it after the 8-byte VFT header.

Fixes: 7725ccfda597 ("[SCSI] bfa: Brocade BFA FC SCSI driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
 drivers/scsi/bfa/bfa_fcs.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c
index 9b57312f43f50..8bb92f935a40f 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -1527,6 +1527,14 @@ bfa_fcs_uf_recv(void *cbarg, struct bfa_uf_s *uf)
 	struct fc_vft_s *vft;
 	struct bfa_fcs_fabric_s *fabric;
 
+	/*
+	 * every consumer reads the FC header, so the frame has to hold one
+	 */
+	if (len < sizeof(struct fchs_s)) {
+		bfa_uf_free(uf);
+		return;
+	}
+
 	/*
 	 * check for VFT header
 	 */
@@ -1550,8 +1558,13 @@ bfa_fcs_uf_recv(void *cbarg, struct bfa_uf_s *uf)
 		}
 
 		/*
-		 * skip vft header
+		 * skip vft header, which the inner FC header has to follow
 		 */
+		if (len < sizeof(struct fc_vft_s) + sizeof(struct fchs_s)) {
+			bfa_uf_free(uf);
+			return;
+		}
+
 		fchs = (struct fchs_s *) (vft + 1);
 		len -= sizeof(struct fc_vft_s);
 
-- 
2.43.0