[PATCH v2] scsi: qedi: fix OOB read in login/text response completion

[PATCH v2] scsi: qedi: fix OOB read in login/text response completion

[Henry Martin]

qedi_process_text_resp() and qedi_process_login_resp() take the payload
length straight from the CQE field (hdr_second_dword &
ISCSI_{LOGIN,TEXT}_RESPONSE_HDR_DATA_SEG_LEN_MASK, 24 bits) written
back by the adapter.  The same raw value feeds the PDU header's
dlength field and the amount of payload buffered, and neither is
bounded:

    hton24(resp_hdr_ptr->dlength, <raw 24-bit value>);
    ...
    pld_len = <raw 24-bit value>;
    qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;

resp_buf is an 8192-byte buffer (qedi_iscsi.c).  A CQE whose data
segment length exceeds the buffer makes resp_wr_ptr point past its end:
the data segment is delivered to __iscsi_complete_pdu() with a
fabricated length, an out-of-bounds read of the buffer delivered to the
iscsi userspace daemon over the iscsi_recv_pdu netlink unicast.

Clamp the payload length to the response buffer size once, before the
header is populated, and use the clamped value for both dlength and the
buffer pointer, so the header always matches the bytes actually
delivered.

This issue was discovered by Tencent CodeBuddy Security.

Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver framework.")
Cc: stable@vger.kernel.org
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v2: Clamp before populating resp_hdr_ptr->dlength and feed the clamped
    value to hton24(), so the embedded header does not advertise more
    payload than iscsi_recv_pdu() copies after the buffer bound is
    enforced (sashiko review).

---
 drivers/scsi/qedi/qedi_fw.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
index 854efa4f61d87..ddab627826180 100644
--- a/drivers/scsi/qedi/qedi_fw.c
+++ b/drivers/scsi/qedi/qedi_fw.c
@@ -102,9 +102,11 @@ static void qedi_process_text_resp(struct qedi_ctx *qedi,
 	resp_hdr_ptr->flags = cqe_text_response->flags;
 	resp_hdr_ptr->hlength = 0;
 
-	hton24(resp_hdr_ptr->dlength,
-	       (cqe_text_response->hdr_second_dword &
-		ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK));
+	pld_len = cqe_text_response->hdr_second_dword &
+		  ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
+	if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
+		pld_len = qedi_conn->gen_pdu.resp_buf_size;
+	hton24(resp_hdr_ptr->dlength, pld_len);
 
 	resp_hdr_ptr->itt = build_itt(cqe->cqe_solicited.itid,
 				      conn->session->age);
@@ -113,8 +115,6 @@ static void qedi_process_text_resp(struct qedi_ctx *qedi,
 	resp_hdr_ptr->exp_cmdsn = cpu_to_be32(cqe_text_response->exp_cmd_sn);
 	resp_hdr_ptr->max_cmdsn = cpu_to_be32(cqe_text_response->max_cmd_sn);
 
-	pld_len = cqe_text_response->hdr_second_dword &
-		  ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
 	qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
 
 	memset(task_ctx, '\0', sizeof(*task_ctx));
@@ -279,9 +279,11 @@ static void qedi_process_login_resp(struct qedi_ctx *qedi,
 	resp_hdr_ptr->flags = cqe_login_response->flags_attr;
 	resp_hdr_ptr->hlength = 0;
 
-	hton24(resp_hdr_ptr->dlength,
-	       (cqe_login_response->hdr_second_dword &
-		ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK));
+	pld_len = cqe_login_response->hdr_second_dword &
+		  ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK;
+	if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
+		pld_len = qedi_conn->gen_pdu.resp_buf_size;
+	hton24(resp_hdr_ptr->dlength, pld_len);
 	resp_hdr_ptr->itt = build_itt(cqe->cqe_solicited.itid,
 				      conn->session->age);
 	resp_hdr_ptr->tsih = cqe_login_response->tsih;
@@ -290,8 +292,6 @@ static void qedi_process_login_resp(struct qedi_ctx *qedi,
 	resp_hdr_ptr->max_cmdsn = cpu_to_be32(cqe_login_response->max_cmd_sn);
 	resp_hdr_ptr->status_class = cqe_login_response->status_class;
 	resp_hdr_ptr->status_detail = cqe_login_response->status_detail;
-	pld_len = cqe_login_response->hdr_second_dword &
-		  ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK;
 	qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
 
 	spin_lock(&qedi_conn->list_lock);
-- 
2.43.7