From b7dd6b533b2b17c7a9d8e6dff6a50ce74f5e7166 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Tue, 30 Apr 2013 19:05:23 -0700 Subject: [PATCH] TEST: Add a test that is cmdsn is too low the target just ignores the pdu --- Makefile.am | 1 + include/iscsi-private.h | 1 + lib/iscsi-command.c | 24 ++++++-- lib/login.c | 16 +++-- lib/task_mgmt.c | 8 ++- test-tool/iscsi-test-cu.c | 1 + test-tool/iscsi-test-cu.h | 1 + test-tool/test_iscsi_cmdsn_toohigh.c | 9 ++- test-tool/test_iscsi_cmdsn_toolow.c | 89 ++++++++++++++++++++++++++++ 9 files changed, 135 insertions(+), 15 deletions(-) create mode 100644 test-tool/test_iscsi_cmdsn_toolow.c diff --git a/Makefile.am b/Makefile.am index ee0aedc..5fea7c8 100644 --- a/Makefile.am +++ b/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 \ diff --git a/include/iscsi-private.h b/include/iscsi-private.h index 91e5f38..e5e0366 100644 --- a/include/iscsi-private.h +++ b/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; diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index 94505d7..9aa02d1 100644 --- a/lib/iscsi-command.c +++ b/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); diff --git a/lib/login.c b/lib/login.c index 12eea24..07ca3dd 100644 --- a/lib/login.c +++ b/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); diff --git a/lib/task_mgmt.c b/lib/task_mgmt.c index 007a37c..68889a4 100644 --- a/lib/task_mgmt.c +++ b/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); diff --git a/test-tool/iscsi-test-cu.c b/test-tool/iscsi-test-cu.c index 1461bad..59f3fae 100644 --- a/test-tool/iscsi-test-cu.c +++ b/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 }; diff --git a/test-tool/iscsi-test-cu.h b/test-tool/iscsi-test-cu.h index 3c35bf4..6cbcf56 100644 --- a/test-tool/iscsi-test-cu.h +++ b/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); diff --git a/test-tool/test_iscsi_cmdsn_toohigh.c b/test-tool/test_iscsi_cmdsn_toohigh.c index 70e3ea1..24aebbb 100644 --- a/test-tool/test_iscsi_cmdsn_toohigh.c +++ b/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; diff --git a/test-tool/test_iscsi_cmdsn_toolow.c b/test-tool/test_iscsi_cmdsn_toolow.c new file mode 100644 index 0000000..fdf7713 --- /dev/null +++ b/test-tool/test_iscsi_cmdsn_toolow.c @@ -0,0 +1,89 @@ +/* + Copyright (C) 2013 by Ronnie Sahlberg + + 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 . +*/ + +#include +#include +#include + +#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); + +}