From 4427cac695604fe1392cf4f10f2f0eece27c3355 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 8 Oct 2019 16:43:29 +0900 Subject: [PATCH] lib/iscsi: Check data segment length separately for immediate data We can move data segment length check from iscsi_check_data_segment_length() to iscsi_op_text() and iscsi_op_scsi(). Task Management Function request, SNACK request, and Logout request don't have data segment, and so any related check is not added. Of course we can add check if data segment length is zero though. This patch also changes the return type of spdk_get_max_immediate_data_size() and a related variable spdk_iscsi_pdu::data_segment_len to uint32_t to remove unnecessary casts. They are little to stand as an independent patch. The next patch will remove iscsi_check_data_segment_length(). Signed-off-by: Shuhei Matsumoto Change-Id: I736ec234d2726de0c70bbae7e748a5b1b5134a32 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470725 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu --- lib/iscsi/iscsi.c | 48 +++++++++++++++++------------------------------ lib/iscsi/iscsi.h | 4 ++-- 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index f3a6bd18f..fc95d308e 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -376,34 +376,7 @@ static bool iscsi_check_data_segment_length(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu, int data_len) { - int max_segment_len = 0; - - /* - * Determine the maximum segment length expected for this PDU. - * This will be used to make sure the initiator did not send - * us too much immediate data. - * - * This value is specified separately by the initiator and target, - * and not negotiated. So we can use the #define safely here, - * since the value is not dependent on the initiator's maximum - * segment lengths (FirstBurstLength/MaxRecvDataSegmentLength), - * and SPDK currently does not allow configuration of these values - * at runtime. - */ - if (conn->sess == NULL) { - return true; - } else if (pdu->bhs.opcode == ISCSI_OP_SCSI_DATAOUT || - pdu->bhs.opcode == ISCSI_OP_NOPOUT) { - return true; - } else { - max_segment_len = spdk_get_max_immediate_data_size(); - } - if (data_len <= max_segment_len) { - return true; - } else { - SPDK_ERRLOG("Data(%d) > MaxSegment(%d)\n", data_len, max_segment_len); - return false; - } + return true; } static int @@ -2262,6 +2235,12 @@ iscsi_op_text(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) struct iscsi_bhs_text_req *reqh; struct iscsi_bhs_text_resp *rsph; + if (pdu->data_segment_len > spdk_get_max_immediate_data_size()) { + SPDK_ERRLOG("data segment len(=%zu) > immediate data len(=%"PRIu32")\n", + pdu->data_segment_len, spdk_get_max_immediate_data_size()); + return iscsi_reject(conn, pdu, ISCSI_REASON_PROTOCOL_ERROR); + } + data_len = 0; alloc_len = conn->MaxRecvDataSegmentLength; @@ -3382,9 +3361,16 @@ iscsi_op_scsi(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) return 0; } + if (pdu->data_segment_len > spdk_get_max_immediate_data_size()) { + SPDK_ERRLOG("data segment len(=%zu) > immediate data len(=%"PRIu32")\n", + pdu->data_segment_len, spdk_get_max_immediate_data_size()); + spdk_iscsi_task_put(task); + return iscsi_reject(conn, pdu, ISCSI_REASON_PROTOCOL_ERROR); + } + if (pdu->data_segment_len > transfer_len) { - SPDK_ERRLOG("data segment len(=%d) > task transfer len(=%d)\n", - (int)pdu->data_segment_len, transfer_len); + SPDK_ERRLOG("data segment len(=%zu) > task transfer len(=%d)\n", + pdu->data_segment_len, transfer_len); spdk_iscsi_task_put(task); return iscsi_reject(conn, pdu, ISCSI_REASON_PROTOCOL_ERROR); } @@ -4634,7 +4620,7 @@ spdk_iscsi_read_pdu(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu **_pdu) struct spdk_mempool *pool; uint32_t crc32c; int ahs_len; - int data_len; + uint32_t data_len; int rc; if (conn->pdu_in_progress == NULL) { diff --git a/lib/iscsi/iscsi.h b/lib/iscsi/iscsi.h index b48601061..3d26be9fb 100644 --- a/lib/iscsi/iscsi.h +++ b/lib/iscsi/iscsi.h @@ -171,7 +171,7 @@ struct spdk_iscsi_pdu { size_t data_segment_len; int bhs_valid_bytes; int ahs_valid_bytes; - int data_valid_bytes; + uint32_t data_valid_bytes; int hdigest_valid_bytes; int ddigest_valid_bytes; int ref; @@ -446,7 +446,7 @@ int spdk_iscsi_conn_handle_queued_datain_tasks(struct spdk_iscsi_conn *conn); void spdk_iscsi_op_abort_task_set(struct spdk_iscsi_task *task, uint8_t function); -static inline int +static inline uint32_t spdk_get_max_immediate_data_size(void) { /*