From 1971294017da789e32522099e6c618cca40375fc Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Wed, 17 May 2017 17:09:23 -0700 Subject: [PATCH 1/5] test_report_supported_opcodes_rctd: Fix a NULL pointer dereference Avoid that the test tool crashes due to a NULL pointer dereference if unmarshalling the Data-In buffer fails. Signed-off-by: Bart Van Assche --- test-tool/test_report_supported_opcodes_rctd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test-tool/test_report_supported_opcodes_rctd.c b/test-tool/test_report_supported_opcodes_rctd.c index 4129f2d..81ad5bd 100644 --- a/test-tool/test_report_supported_opcodes_rctd.c +++ b/test-tool/test_report_supported_opcodes_rctd.c @@ -70,6 +70,8 @@ test_report_supported_opcodes_rctd(void) logging(LOG_VERBOSE, "Unmarshall the DATA-IN buffer"); rsoc = scsi_datain_unmarshall(rso_task); CU_ASSERT_NOT_EQUAL(rsoc, NULL); + if (!rsoc) + goto free_task; logging(LOG_VERBOSE, "Verify that all returned command descriptors " "have a timeout description"); @@ -94,5 +96,6 @@ test_report_supported_opcodes_rctd(void) } } +free_task: scsi_free_scsi_task(rso_task); } From cba570408e61db26ac86a522d00ba68d04ae921b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 4 May 2017 15:08:32 -0700 Subject: [PATCH 2/5] test_async_*_simple: Fix a CUnit assertion failure Using any of the CU_ASSERT macros after a test has finished causes the test tool to abort. This patch adds an iSCSI logout to two tests to avoid that the test tool aborts e.g. as follows: ==11578== Process terminating with default action of signal 6 (SIGABRT) ==11578== at 0x54BB77F: raise (raise.c:58) ==11578== by 0x54BD379: abort (abort.c:89) ==11578== by 0x54B3B46: __assert_fail_base (assert.c:92) ==11578== by 0x54B3BF1: __assert_fail (assert.c:101) ==11578== by 0x504213E: CU_assertImplementation (in /usr/lib/x86_64-linux-gnu/libcunit.so.1.0.1) ==11578== by 0x16FA36: test_async_abort_cb (test_async_abort_simple.c:67) ==11578== by 0x5274B82: iscsi_process_task_mgmt_reply (task_mgmt.c:100) ==11578== by 0x525E226: iscsi_process_pdu (pdu.c:598) ==11578== by 0x526F2AA: iscsi_read_from_socket (socket.c:677) ==11578== by 0x5270015: iscsi_tcp_service (socket.c:963) ==11578== by 0x52700A5: iscsi_service (socket.c:980) ==11578== by 0x52707E1: event_loop (sync.c:69) Signed-off-by: Bart Van Assche --- test-tool/test_async_abort_simple.c | 5 +++++ test-tool/test_async_lu_reset_simple.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/test-tool/test_async_abort_simple.c b/test-tool/test_async_abort_simple.c index b7ed825..3f3ff58 100644 --- a/test-tool/test_async_abort_simple.c +++ b/test-tool/test_async_abort_simple.c @@ -203,4 +203,9 @@ test_async_abort_simple(void) } scsi_free_scsi_task(state.wtask); + + /* Avoid that callbacks get invoked after this test finished */ + iscsi_logout_sync(sd->iscsi_ctx); + iscsi_destroy_context(sd->iscsi_ctx); + sd->iscsi_ctx = NULL; } diff --git a/test-tool/test_async_lu_reset_simple.c b/test-tool/test_async_lu_reset_simple.c index 64e2559..e957a5c 100644 --- a/test-tool/test_async_lu_reset_simple.c +++ b/test-tool/test_async_lu_reset_simple.c @@ -192,4 +192,9 @@ test_async_lu_reset_simple(void) } scsi_free_scsi_task(state.wtask); + + /* Avoid that callbacks get invoked after this test finished */ + iscsi_logout_sync(sd->iscsi_ctx); + iscsi_destroy_context(sd->iscsi_ctx); + sd->iscsi_ctx = NULL; } From 7658e5f0a5f35160bcc994780351e1849abbc03c Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Wed, 10 May 2017 10:13:07 -0700 Subject: [PATCH 3/5] test_extended_copy_simple: Fix buffer comparison code Compare the buffers in their entirety instead of only a prefix of the buffers. Signed-off-by: Bart Van Assche --- test-tool/test_extendedcopy_simple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-tool/test_extendedcopy_simple.c b/test-tool/test_extendedcopy_simple.c index 79c4862..872b017 100644 --- a/test-tool/test_extendedcopy_simple.c +++ b/test-tool/test_extendedcopy_simple.c @@ -75,7 +75,7 @@ test_extendedcopy_simple(void) block_size, 0, 0, 0, 0, 0, buf2, EXPECT_STATUS_GOOD); - if (memcmp(buf1, buf2, 2048)) { + if (memcmp(buf1, buf2, 2048*block_size)) { CU_FAIL("Blocks were not copied correctly"); } From 6332c5e3d1a50a14ea7c390427af1b2ac0a5aee3 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Wed, 10 May 2017 10:13:07 -0700 Subject: [PATCH 4/5] test_extended_copy_simple: Limit transfer size to half of the capacity This avoids that the test fails for LUNs with less than 2048 blocks. Signed-off-by: Bart Van Assche --- test-tool/test_extendedcopy_simple.c | 30 ++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/test-tool/test_extendedcopy_simple.c b/test-tool/test_extendedcopy_simple.c index 872b017..ce3a7a7 100644 --- a/test-tool/test_extendedcopy_simple.c +++ b/test-tool/test_extendedcopy_simple.c @@ -33,18 +33,27 @@ test_extendedcopy_simple(void) int tgt_desc_len = 0, seg_desc_len = 0, offset = XCOPY_DESC_OFFSET; struct iscsi_data data; unsigned char *xcopybuf; - unsigned char *buf1 = malloc(2048*block_size); - unsigned char *buf2 = malloc(2048*block_size); + unsigned int copied_blocks; + unsigned char *buf1; + unsigned char *buf2; + + + copied_blocks = num_blocks / 2; + if (copied_blocks > 2048) + copied_blocks = 2048; + buf1 = malloc(copied_blocks * block_size); + buf2 = malloc(copied_blocks * block_size); logging(LOG_VERBOSE, LOG_BLANK_LINE); logging(LOG_VERBOSE, - "Test EXTENDED COPY of 2048 blocks from start of LUN to end of LUN"); + "Test EXTENDED COPY of %u blocks from start of LUN to end of LUN", + copied_blocks); CHECK_FOR_DATALOSS; - logging(LOG_VERBOSE, "Write 2048 blocks of 'A' at LBA:0"); - memset(buf1, 'A', 2048*block_size); - WRITE16(sd, 0, 2048*block_size, block_size, 0, 0, 0, 0, 0, + logging(LOG_VERBOSE, "Write %u blocks of 'A' at LBA:0", copied_blocks); + memset(buf1, 'A', copied_blocks * block_size); + WRITE16(sd, 0, copied_blocks * block_size, block_size, 0, 0, 0, 0, 0, buf1, EXPECT_STATUS_GOOD); data.size = XCOPY_DESC_OFFSET + @@ -61,7 +70,7 @@ test_extendedcopy_simple(void) /* Iniitialize segment descriptor list with one segment descriptor */ offset += populate_seg_desc_b2b(xcopybuf+offset, 0, 0, 0, 0, - 2048, 0, num_blocks - 2048); + copied_blocks, 0, num_blocks - copied_blocks); seg_desc_len = offset - XCOPY_DESC_OFFSET - tgt_desc_len; /* Initialize the parameter list header */ @@ -70,12 +79,13 @@ test_extendedcopy_simple(void) EXTENDEDCOPY(sd, &data, EXPECT_STATUS_GOOD); - logging(LOG_VERBOSE, "Read 2048 blocks from end of the LUN"); - READ16(sd, NULL, num_blocks - 2048, 2048*block_size, + logging(LOG_VERBOSE, "Read %u blocks from end of the LUN", + copied_blocks); + READ16(sd, NULL, num_blocks - copied_blocks, copied_blocks * block_size, block_size, 0, 0, 0, 0, 0, buf2, EXPECT_STATUS_GOOD); - if (memcmp(buf1, buf2, 2048*block_size)) { + if (memcmp(buf1, buf2, copied_blocks * block_size)) { CU_FAIL("Blocks were not copied correctly"); } From 494bddc6607b189044aee96ab4d2cc749436e47e Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 21 Feb 2017 17:10:18 -0800 Subject: [PATCH 5/5] test_multipathio_simple: Only compare if READ10 succeeded Since the data in the read buffer is not valid if READ10() failed, only compare the data in the read buffer if READ10() succeeded. Signed-off-by: Bart Van Assche --- test-tool/iscsi-support.h | 10 ++++++---- test-tool/test_multipathio_simple.c | 18 +++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/test-tool/iscsi-support.h b/test-tool/iscsi-support.h index b301b6f..283d597 100644 --- a/test-tool/iscsi-support.h +++ b/test-tool/iscsi-support.h @@ -320,7 +320,7 @@ do { \ } while (0); #define READ10(...) \ - do { \ + ({ \ int _r; \ _r = read10(__VA_ARGS__); \ if (_r == -2) { \ @@ -331,7 +331,8 @@ do { \ return; \ } \ CU_ASSERT_EQUAL(_r, 0); \ - } while (0); + _r; \ + }) #define READ12(...) \ do { \ @@ -602,7 +603,7 @@ do { \ } while (0); #define WRITE10(...) \ - do { \ + ({ \ int _r; \ _r = write10(__VA_ARGS__); \ if (_r == -2) { \ @@ -613,7 +614,8 @@ do { \ return; \ } \ CU_ASSERT_EQUAL(_r, 0); \ - } while (0); + _r; \ + }) #define WRITE12(...) \ do { \ diff --git a/test-tool/test_multipathio_simple.c b/test-tool/test_multipathio_simple.c index ff1dcab..2a9f38e 100644 --- a/test-tool/test_multipathio_simple.c +++ b/test-tool/test_multipathio_simple.c @@ -34,6 +34,7 @@ test_multipathio_simple(void) int write_path; unsigned char *write_buf = alloca(256 * block_size); unsigned char *read_buf = alloca(256 * block_size); + int ret; CHECK_FOR_DATALOSS; CHECK_FOR_SBC; @@ -58,16 +59,19 @@ test_multipathio_simple(void) && maximum_transfer_length < i) { break; } - WRITE10(mp_sds[write_path], 0, i * block_size, + ret = WRITE10(mp_sds[write_path], 0, i * block_size, block_size, 0, 0, 0, 0, 0, write_buf, EXPECT_STATUS_GOOD); - READ10(mp_sds[read_path], NULL, 0, i * block_size, - block_size, 0, 0, 0, 0, 0, read_buf, - EXPECT_STATUS_GOOD); - + if (ret < 0) + continue; + ret = READ10(mp_sds[read_path], NULL, 0, i * block_size, + block_size, 0, 0, 0, 0, 0, read_buf, + EXPECT_STATUS_GOOD); + if (ret < 0) + continue; /* compare written and read data */ - CU_ASSERT_EQUAL(0, - memcmp(write_buf, read_buf, i * block_size)); + ret = memcmp(write_buf, read_buf, i * block_size); + CU_ASSERT_EQUAL(0, ret); } }