129 lines
4.2 KiB
Diff
129 lines
4.2 KiB
Diff
From c2c8b7d986c86ff2ec031873d3b08e7e5eae22ea Mon Sep 17 00:00:00 2001
|
|
From: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
|
|
Date: Mon, 8 Feb 2021 10:28:44 -0500
|
|
Subject: [PATCH 10/15] lib/iscsi: return immediately from iscsi_parse_params
|
|
if len is 0
|
|
|
|
The spec does not disallow TEXT PDUs with no data. In that
|
|
case, just return immediately from iscsi_parse_params.
|
|
|
|
This avoids a NULL pointer dereference with a TEXT PDU that has
|
|
no data, but CONTINUE flag is set.
|
|
|
|
Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
|
|
Signed-off-by: Jim Harris <james.r.harris@intel.com>
|
|
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6319 (master)
|
|
|
|
(cherry picked from commit f3fd56fc3c73ee7595bbe7c69cf8048fe2577e1a)
|
|
Change-Id: I2605293daf171633a45132d7b5532fdfc9128aff
|
|
Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
|
|
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6601
|
|
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
|
|
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
|
|
Reviewed-by: Jim Harris <james.r.harris@intel.com>
|
|
---
|
|
lib/iscsi/param.c | 10 +++++++++
|
|
test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 38 ++++++++++++++++++++++++++++++++++
|
|
test/unit/lib/iscsi/param.c/param_ut.c | 8 +++++++
|
|
3 files changed, 56 insertions(+)
|
|
|
|
diff --git a/lib/iscsi/param.c b/lib/iscsi/param.c
|
|
index 1a353c4..ab2c9cd 100644
|
|
--- a/lib/iscsi/param.c
|
|
+++ b/lib/iscsi/param.c
|
|
@@ -315,6 +315,16 @@ iscsi_parse_params(struct iscsi_param **params, const uint8_t *data,
|
|
char *p;
|
|
int i;
|
|
|
|
+ /* Spec does not disallow TEXT PDUs with zero length, just return
|
|
+ * immediately in that case, since there is no param data to parse
|
|
+ * and any existing partial parameter would remain as-is.
|
|
+ */
|
|
+ if (len == 0) {
|
|
+ return 0;
|
|
+ }
|
|
+
|
|
+ assert(data != NULL);
|
|
+
|
|
/* strip the partial text parameters if previous PDU have C enabled */
|
|
if (partial_parameter && *partial_parameter) {
|
|
for (i = 0; i < len && data[i] != '\0'; i++) {
|
|
diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c
|
|
index c901e9e..56082a0 100644
|
|
--- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c
|
|
+++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c
|
|
@@ -1993,6 +1993,43 @@ pdu_hdr_op_data_test(void)
|
|
g_task_pool_is_empty = false;
|
|
}
|
|
|
|
+/* Test an ISCSI_OP_TEXT PDU with CONTINUE bit set but
|
|
+ * no data.
|
|
+ */
|
|
+static void
|
|
+empty_text_with_cbit_test(void)
|
|
+{
|
|
+ struct spdk_iscsi_sess sess = {};
|
|
+ struct spdk_iscsi_conn conn = {};
|
|
+ struct spdk_scsi_dev dev = {};
|
|
+ struct spdk_iscsi_pdu *req_pdu;
|
|
+ int rc;
|
|
+
|
|
+ req_pdu = iscsi_get_pdu(&conn);
|
|
+
|
|
+ sess.ExpCmdSN = 0;
|
|
+ sess.MaxCmdSN = 64;
|
|
+ sess.session_type = SESSION_TYPE_NORMAL;
|
|
+ sess.MaxBurstLength = 1024;
|
|
+
|
|
+ conn.full_feature = 1;
|
|
+ conn.sess = &sess;
|
|
+ conn.dev = &dev;
|
|
+ conn.state = ISCSI_CONN_STATE_RUNNING;
|
|
+
|
|
+ memset(&req_pdu->bhs, 0, sizeof(req_pdu->bhs));
|
|
+ req_pdu->bhs.opcode = ISCSI_OP_TEXT;
|
|
+ req_pdu->bhs.flags = ISCSI_TEXT_CONTINUE;
|
|
+
|
|
+ rc = iscsi_pdu_hdr_handle(&conn, req_pdu);
|
|
+ CU_ASSERT(rc == 0);
|
|
+ CU_ASSERT(!req_pdu->is_rejected);
|
|
+ rc = iscsi_pdu_payload_handle(&conn, req_pdu);
|
|
+ CU_ASSERT(rc == 0);
|
|
+
|
|
+ iscsi_put_pdu(req_pdu);
|
|
+}
|
|
+
|
|
int
|
|
main(int argc, char **argv)
|
|
{
|
|
@@ -2024,6 +2061,7 @@ main(int argc, char **argv)
|
|
CU_ADD_TEST(suite, pdu_hdr_op_task_mgmt_test);
|
|
CU_ADD_TEST(suite, pdu_hdr_op_nopout_test);
|
|
CU_ADD_TEST(suite, pdu_hdr_op_data_test);
|
|
+ CU_ADD_TEST(suite, empty_text_with_cbit_test);
|
|
|
|
CU_basic_set_mode(CU_BRM_VERBOSE);
|
|
CU_basic_run_tests();
|
|
diff --git a/test/unit/lib/iscsi/param.c/param_ut.c b/test/unit/lib/iscsi/param.c/param_ut.c
|
|
index ccf6264..9b3e820 100644
|
|
--- a/test/unit/lib/iscsi/param.c/param_ut.c
|
|
+++ b/test/unit/lib/iscsi/param.c/param_ut.c
|
|
@@ -264,6 +264,14 @@ parse_valid_test(void)
|
|
EXPECT_VAL("F", "IIII");
|
|
CU_ASSERT_PTR_NULL(partial_parameter);
|
|
|
|
+ /* partial parameter: NULL data */
|
|
+ /* It is technically allowed to have a TEXT PDU with no data, yet
|
|
+ * CONTINUE bit is enabled - make sure we handle that case correctly.
|
|
+ */
|
|
+ rc = iscsi_parse_params(¶ms, NULL, 0, true, &partial_parameter);
|
|
+ CU_ASSERT(rc == 0);
|
|
+ CU_ASSERT_PTR_NULL(partial_parameter);
|
|
+
|
|
/* Second partial parameter is the only parameter */
|
|
PARSE("OOOO", true, &partial_parameter);
|
|
CU_ASSERT_STRING_EQUAL(partial_parameter, "OOOO");
|
|
--
|
|
1.8.3.1
|
|
|