TEST: Add a test that is cmdsn is too low the target just ignores the pdu

Makefile.am

@@ -191,6 +191,7 @@ bin_iscsi_test_cu_SOURCES = test-tool/iscsi-test-cu.c \
 	test-tool/test_inquiry_supported_vpd.c \
 	test-tool/test_inquiry_mandatory_vpd_sbc.c \
 	test-tool/test_iscsi_cmdsn_toohigh.c \
+	test-tool/test_iscsi_cmdsn_toolow.c \
 	test-tool/test_mandatory_sbc.c \
 	test-tool/test_nomedia_sbc.c \
 	test-tool/test_orwrite_simple.c \

include/iscsi-private.h

@@ -71,6 +71,7 @@ struct iscsi_context {
 	unsigned char isid[6];
 	uint32_t itt;
 	uint32_t cmdsn;
+	uint32_t expcmdsn;
 	uint32_t maxcmdsn;
 	uint32_t statsn;
 	enum iscsi_header_digest want_header_digest;

lib/iscsi-command.c

@@ -343,7 +343,7 @@ int
 iscsi_process_scsi_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 			 struct iscsi_in_pdu *in)
 {
-	uint32_t statsn, maxcmdsn, flags, status;
+	uint32_t statsn, maxcmdsn, expcmdsn, flags, status;
 	struct iscsi_scsi_cbdata *scsi_cbdata = &pdu->scsi_cbdata;
 	struct scsi_task *task = scsi_cbdata->task;
 
@@ -353,9 +353,13 @@ iscsi_process_scsi_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 	}
 
 	maxcmdsn = scsi_get_uint32(&in->hdr[32]);
-	if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) {
+	if (iscsi_serial32_compare(maxcmdsn, iscsi->maxcmdsn) > 0) {
 		iscsi->maxcmdsn = maxcmdsn;
 	}
+	expcmdsn = scsi_get_uint32(&in->hdr[28]);
+	if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) {
+		iscsi->expcmdsn = expcmdsn;
+	}
 
 	flags = in->hdr[1];
 	if ((flags&ISCSI_PDU_DATA_FINAL) == 0) {
@@ -464,7 +468,7 @@ int
 iscsi_process_scsi_data_in(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 			   struct iscsi_in_pdu *in, int *is_finished)
 {
-	uint32_t statsn, maxcmdsn, flags, status;
+	uint32_t statsn, maxcmdsn, expcmdsn, flags, status;
 	struct iscsi_scsi_cbdata *scsi_cbdata = &pdu->scsi_cbdata;
 	struct scsi_task *task = scsi_cbdata->task;
 	int dsl;
@@ -475,9 +479,13 @@ iscsi_process_scsi_data_in(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 	}
 
 	maxcmdsn = scsi_get_uint32(&in->hdr[32]);
-	if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) {
+	if (iscsi_serial32_compare(maxcmdsn, iscsi->maxcmdsn) > 0) {
 		iscsi->maxcmdsn = maxcmdsn;
 	}
+	expcmdsn = scsi_get_uint32(&in->hdr[28]);
+	if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) {
+		iscsi->expcmdsn = expcmdsn;
+	}
 
 	flags = in->hdr[1];
 	if ((flags&ISCSI_PDU_DATA_ACK_REQUESTED) != 0) {
@@ -550,16 +558,20 @@ int
 iscsi_process_r2t(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 			 struct iscsi_in_pdu *in)
 {
-	uint32_t ttt, offset, len, maxcmdsn;
+	uint32_t ttt, offset, len, maxcmdsn, expcmdsn;
 
 	ttt    = scsi_get_uint32(&in->hdr[20]);
 	offset = scsi_get_uint32(&in->hdr[40]);
 	len    = scsi_get_uint32(&in->hdr[44]);
 
 	maxcmdsn = scsi_get_uint32(&in->hdr[32]);
-	if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) {
+	if (iscsi_serial32_compare(maxcmdsn, iscsi->maxcmdsn) > 0) {
 		iscsi->maxcmdsn = maxcmdsn;
 	}
+	expcmdsn = scsi_get_uint32(&in->hdr[28]);
+	if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) {
+		iscsi->expcmdsn = expcmdsn;
+	}
 
 	pdu->datasn = 0;
 	iscsi_send_data_out(iscsi, pdu, ttt, offset, len);

lib/login.c

@@ -951,7 +951,7 @@ int
 iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 			  struct iscsi_in_pdu *in)
 {
-	uint32_t status, maxcmdsn;
+  uint32_t status, maxcmdsn, expcmdsn;
 	char *ptr = (char *)in->data;
 	int size = in->data_pos;
 
@@ -960,9 +960,13 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 	iscsi->statsn = scsi_get_uint16(&in->hdr[24]);
 
 	maxcmdsn = scsi_get_uint32(&in->hdr[32]);
-	if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) {
+	if (iscsi_serial32_compare(maxcmdsn, iscsi->maxcmdsn) > 0) {
 		iscsi->maxcmdsn = maxcmdsn;
 	}
+	expcmdsn = scsi_get_uint32(&in->hdr[28]);
+	if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) {
+		iscsi->expcmdsn = expcmdsn;
+	}
 	
 	/* XXX here we should parse the data returned in case the target
 	 * renegotiated some some parameters.
@@ -1149,12 +1153,16 @@ int
 iscsi_process_logout_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 struct iscsi_in_pdu *in)
 {
-	uint32_t maxcmdsn;
+  uint32_t maxcmdsn, expcmdsn;
 
 	maxcmdsn = scsi_get_uint32(&in->hdr[32]);
-	if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) {
+	if (iscsi_serial32_compare(maxcmdsn, iscsi->maxcmdsn) > 0) {
 		iscsi->maxcmdsn = maxcmdsn;
 	}
+	expcmdsn = scsi_get_uint32(&in->hdr[28]);
+	if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) {
+		iscsi->expcmdsn = expcmdsn;
+	}
 
 	iscsi->is_loggedin = 0;
 	pdu->callback(iscsi, SCSI_STATUS_GOOD, NULL, pdu->private_data);

lib/task_mgmt.c

@@ -92,14 +92,18 @@ int
 iscsi_process_task_mgmt_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
 			    struct iscsi_in_pdu *in)
 {
-	uint32_t response, maxcmdsn;
+	uint32_t response, maxcmdsn, expcmdsn;
 
 	response = in->hdr[2];
 
 	maxcmdsn = scsi_get_uint32(&in->hdr[32]);
-	if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) {
+	if (iscsi_serial32_compare(maxcmdsn, iscsi->maxcmdsn) > 0) {
 		iscsi->maxcmdsn = maxcmdsn;
 	}
+	expcmdsn = scsi_get_uint32(&in->hdr[28]);
+	if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) {
+		iscsi->expcmdsn = expcmdsn;
+	}
 
 	pdu->callback(iscsi, SCSI_STATUS_GOOD, &response, pdu->private_data);
 

test-tool/iscsi-test-cu.c

@@ -432,6 +432,7 @@ static CU_SuiteInfo scsi_suites[] = {
 
 static CU_TestInfo tests_iscsi_cmdsn[] = {
 	{ (char *)"iSCSICmdSnTooHigh", test_iscsi_cmdsn_toohigh },
+	{ (char *)"iSCSICmdSnTooLow", test_iscsi_cmdsn_toolow },
 	CU_TEST_INFO_NULL
 };
 

test-tool/iscsi-test-cu.h

@@ -50,6 +50,7 @@ void test_inquiry_supported_vpd(void);
 void test_inquiry_mandatory_vpd_sbc(void);
 
 void test_iscsi_cmdsn_toohigh(void);
+void test_iscsi_cmdsn_toolow(void);
 
 void test_mandatory_sbc(void);
 

test-tool/test_iscsi_cmdsn_toohigh.c

@@ -35,6 +35,10 @@ static int my_iscsi_queue_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu
 		old_cmdsn = *(uint32_t *)&pdu->outdata.data[24];
 		/* change the cmdsn so it becomes too big */
 		*(uint32_t *)&pdu->outdata.data[24] = htonl(iscsi->maxcmdsn + 1);
+		/* fudge the cmdsn value back to where it should be if this
+		 * pdu is ignored.
+		 */
+		iscsi->cmdsn--;
 		break;
 	case 2:
 		*(uint32_t *)&pdu->outdata.data[24] = old_cmdsn;
@@ -60,8 +64,8 @@ void test_iscsi_cmdsn_toohigh(void)
 	iscsic->target_max_recv_data_segment_length = block_size;
 	local_iscsi_queue_pdu = my_iscsi_queue_pdu;
 	change_cmdsn = 1;
-	/* we dont want autoreconnect since some targets will drop the
-	 * connection on this condition.
+	/* we dont want autoreconnect since some targets will incorrectly
+	 * drop the connection on this condition.
 	 */
 	iscsi_set_noautoreconnect(iscsic, 1);
 	iscsi_set_timeout(iscsic, 3);
@@ -75,7 +79,6 @@ void test_iscsi_cmdsn_toohigh(void)
 	}
 
 	
-
 	iscsi_set_noautoreconnect(iscsic, 0);
 	logging(LOG_VERBOSE, "Send a TESTUNITREADY with CMDSN == EXPCMDSN. should work again");
 	change_cmdsn = 2;

test-tool/test_iscsi_cmdsn_toolow.c

@@ -0,0 +1,89 @@
+/* 
+   Copyright (C) 2013 by Ronnie Sahlberg <ronniesahlberg@gmail.com>
+   
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+   
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+   
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <stdio.h>
+#include <arpa/inet.h>
+#include <CUnit/CUnit.h>
+
+#include "iscsi.h"
+#include "iscsi-private.h"
+#include "scsi-lowlevel.h"
+#include "iscsi-test-cu.h"
+
+static int change_cmdsn;
+
+static int my_iscsi_queue_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu)
+{
+	static uint32_t old_cmdsn;
+
+	switch (change_cmdsn) {
+	case 1:
+		old_cmdsn = *(uint32_t *)&pdu->outdata.data[24];
+		/* change the cmdsn so it becomes too big */
+		*(uint32_t *)&pdu->outdata.data[24] = htonl(iscsi->expcmdsn + 1);
+		/* fudge the cmdsn value back to where it should be if this
+		 * pdu is ignored.
+		 */
+		iscsi->cmdsn--;
+		break;
+	case 2:
+		*(uint32_t *)&pdu->outdata.data[24] = old_cmdsn;
+		break;
+	}
+
+	change_cmdsn = 0;	
+	return 0;
+}
+
+void test_iscsi_cmdsn_toolow(void)
+{ 
+	int ret;
+
+	logging(LOG_VERBOSE, LOG_BLANK_LINE);
+	logging(LOG_VERBOSE, "Test sending invalid iSCSI CMDSN");
+	logging(LOG_VERBOSE, "CMDSN MUST be in the range EXPCMDSN and MAXCMDSN");
+
+	logging(LOG_VERBOSE, "RFC3720:3.2.2.1 CMDSN < EXPCMDSN must be silently ignored by the target");
+	logging(LOG_VERBOSE, "Send a TESTUNITREADY with CMDSN == EXPCMDSN-1. Should be ignored by the target.");
+
+	iscsic->use_immediate_data = ISCSI_IMMEDIATE_DATA_NO;
+	iscsic->target_max_recv_data_segment_length = block_size;
+	local_iscsi_queue_pdu = my_iscsi_queue_pdu;
+	change_cmdsn = 1;
+	/* we dont want autoreconnect since some targets will incorrectly
+	 * drop the connection on this condition.
+	 */
+	iscsi_set_noautoreconnect(iscsic, 1);
+	iscsi_set_timeout(iscsic, 3);
+
+	ret = testunitready(iscsic, tgt_lun);
+	CU_ASSERT_EQUAL(ret, -1);
+	if (ret == -1) {
+		logging(LOG_VERBOSE, "[SUCCESS] We did not receive a reply");
+	} else {
+		logging(LOG_VERBOSE, "[FAILURE] We got a response from the target but SMDSN was outside of the window.");
+	}
+
+	
+
+	iscsi_set_noautoreconnect(iscsic, 0);
+	logging(LOG_VERBOSE, "Send a TESTUNITREADY with CMDSN == EXPCMDSN. should work again");
+	change_cmdsn = 2;
+	ret = testunitready(iscsic, tgt_lun);
+	CU_ASSERT_EQUAL(ret, 0);
+
+}