diff --git a/lib/scsi-lowlevel.c b/lib/scsi-lowlevel.c index d68d9909..5ddd709c 100644 --- a/lib/scsi-lowlevel.c +++ b/lib/scsi-lowlevel.c @@ -962,7 +962,9 @@ scsi_receivecopyresults_datain_unmarshall(struct scsi_task *task) } } - +#ifndef MIN /* instead of including all of iscsi-private.h */ +#define MIN(a, b) (((a) < (b)) ? (a) : (b)) +#endif static void * scsi_persistentreservein_datain_unmarshall(struct scsi_task *task) { @@ -972,21 +974,44 @@ scsi_persistentreservein_datain_unmarshall(struct scsi_task *task) int i; switch (scsi_persistentreservein_sa(task)) { - case SCSI_PERSISTENT_RESERVE_READ_KEYS: - i = task_get_uint32(task, 4); + case SCSI_PERSISTENT_RESERVE_READ_KEYS: { + uint32_t cdb_keys_len; + uint32_t data_keys_len; + uint32_t keys_len; - rk = scsi_malloc(task, offsetof(struct scsi_persistent_reserve_in_read_keys, keys) + i); + if (task->datain.size < 8) { + return NULL; + } + + /* + * SPC5r17: 6.16.2 READ KEYS service action + * The ADDITIONAL LENGTH field indicates the number of bytes in + * the Reservation key list. The contents of the ADDITIONAL + * LENGTH field are not altered based on the allocation length. + */ + cdb_keys_len = task_get_uint32(task, 4); + data_keys_len = task->datain.size - 8; + /* + * Only process as many keys as permitted by the given + * ADDITIONAL LENGTH and data-in size limits. + */ + keys_len = MIN(cdb_keys_len, data_keys_len); + + rk = scsi_malloc(task, + offsetof(struct scsi_persistent_reserve_in_read_keys, + keys) + keys_len); if (rk == NULL) { return NULL; } rk->prgeneration = task_get_uint32(task, 0); - rk->additional_length = task_get_uint32(task, 4); + rk->additional_length = cdb_keys_len; - rk->num_keys = rk->additional_length / 8; + rk->num_keys = keys_len / 8; for (i = 0; i < (int)rk->num_keys; i++) { rk->keys[i] = task_get_uint64(task, 8 + i * 8); } return rk; + } case SCSI_PERSISTENT_RESERVE_READ_RESERVATION: { size_t alloc_sz; diff --git a/test-tool/Makefile.am b/test-tool/Makefile.am index 13d8663e..c658d0dd 100644 --- a/test-tool/Makefile.am +++ b/test-tool/Makefile.am @@ -70,6 +70,7 @@ iscsi_test_cu_SOURCES = iscsi-test-cu.c \ test_preventallow_lun_reset.c \ test_preventallow_2_itnexuses.c \ test_prin_read_keys_simple.c \ + test_prin_read_keys_truncate.c \ test_prin_serviceaction_range.c \ test_prin_report_caps.c \ test_prout_register_simple.c \ diff --git a/test-tool/iscsi-support.c b/test-tool/iscsi-support.c index 958dce96..f0446a61 100644 --- a/test-tool/iscsi-support.c +++ b/test-tool/iscsi-support.c @@ -667,17 +667,18 @@ prin_task(struct scsi_device *sdev, int service_action, } int -prin_read_keys(struct scsi_device *sdev, struct scsi_task **tp, - struct scsi_persistent_reserve_in_read_keys **rkp) +prin_read_keys(struct scsi_device *sdev, + struct scsi_task **tp, + struct scsi_persistent_reserve_in_read_keys **rkp, + uint16_t allocation_len) { - const int buf_sz = 16384; struct scsi_persistent_reserve_in_read_keys *rk = NULL; logging(LOG_VERBOSE, "Send PRIN/READ_KEYS"); *tp = scsi_cdb_persistent_reserve_in(SCSI_PERSISTENT_RESERVE_READ_KEYS, - buf_sz); + allocation_len); assert(*tp != NULL); *tp = send_scsi_command(sdev, *tp, NULL); diff --git a/test-tool/iscsi-support.h b/test-tool/iscsi-support.h index 283d5971..5f79d8bd 100644 --- a/test-tool/iscsi-support.h +++ b/test-tool/iscsi-support.h @@ -822,7 +822,8 @@ int all_zero(const unsigned char *buf, unsigned size); int prin_task(struct scsi_device *sdev, int service_action, int success_expected); int prin_read_keys(struct scsi_device *sdev, struct scsi_task **tp, - struct scsi_persistent_reserve_in_read_keys **rkp); + struct scsi_persistent_reserve_in_read_keys **rkp, + uint16_t allocation_len); int prout_register_and_ignore(struct scsi_device *sdev, unsigned long long key); int prout_register_key(struct scsi_device *sdev, diff --git a/test-tool/iscsi-test-cu.c b/test-tool/iscsi-test-cu.c index 3ec3b091..76a16e08 100644 --- a/test-tool/iscsi-test-cu.c +++ b/test-tool/iscsi-test-cu.c @@ -158,6 +158,7 @@ static CU_TestInfo tests_preventallow[] = { static CU_TestInfo tests_prin_read_keys[] = { { (char *)"Simple", test_prin_read_keys_simple }, + { (char *)"Truncate", test_prin_read_keys_truncate }, CU_TEST_INFO_NULL }; @@ -1069,7 +1070,7 @@ static int clear_pr(struct scsi_device *sdev) struct scsi_persistent_reserve_in_read_keys *rk; res = 0; - if (prin_read_keys(sdev, &pr_task, &rk) != 0) + if (prin_read_keys(sdev, &pr_task, &rk, 16384) != 0) goto out; res = -1; diff --git a/test-tool/iscsi-test-cu.h b/test-tool/iscsi-test-cu.h index adfc776d..d88c6a7c 100644 --- a/test-tool/iscsi-test-cu.h +++ b/test-tool/iscsi-test-cu.h @@ -115,6 +115,7 @@ void test_preventallow_lun_reset(void); void test_preventallow_2_itnexuses(void); void test_prin_read_keys_simple(void); +void test_prin_read_keys_truncate(void); void test_prin_serviceaction_range(void); void test_prin_report_caps_simple(void); diff --git a/test-tool/test_prin_read_keys_simple.c b/test-tool/test_prin_read_keys_simple.c index 127925c8..57970918 100644 --- a/test-tool/test_prin_read_keys_simple.c +++ b/test-tool/test_prin_read_keys_simple.c @@ -37,9 +37,8 @@ test_prin_read_keys_simple(void) logging(LOG_VERBOSE, LOG_BLANK_LINE); logging(LOG_VERBOSE, "Test Persistent Reserve IN READ_KEYS works."); - ret = prin_read_keys(sd, &task, NULL); + ret = prin_read_keys(sd, &task, NULL, 16384); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTEN RESERVE IN is not implemented."); CU_PASS("PERSISTENT RESERVE IN is not implemented."); return; } diff --git a/test-tool/test_prin_read_keys_truncate.c b/test-tool/test_prin_read_keys_truncate.c new file mode 100644 index 00000000..1590b33e --- /dev/null +++ b/test-tool/test_prin_read_keys_truncate.c @@ -0,0 +1,74 @@ +/* -*- mode:c; tab-width:8; c-basic-offset:8; indent-tabs-mode:nil; -*- */ +/* + Copyright (C) 2013 by Lee Duncan + Copyright (C) 2018 David Disseldorp + + 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 "scsi-lowlevel.h" +#include "iscsi-support.h" +#include "iscsi-test-cu.h" + + +void +test_prin_read_keys_truncate(void) +{ + const unsigned long long key = rand_key(); + struct scsi_persistent_reserve_in_read_keys *rk = NULL; + int ret; + + logging(LOG_VERBOSE, LOG_BLANK_LINE); + logging(LOG_VERBOSE, "Test Persistent Reserve IN READ_KEYS works when " + "truncated."); + + ret = prout_register_and_ignore(sd, key); + if (ret == -2) { + CU_PASS("PERSISTENT RESERVE OUT is not implemented."); + return; + } + CU_ASSERT_EQUAL(ret, 0); + + /* + * alloc_len=8 restricts the response buffer to only accepting the + * PRGENERATION and ADDITIONAL LENGTH fields. + */ + ret = prin_read_keys(sd, &task, &rk, 8); + if (ret == -2) { + CU_PASS("PERSISTENT RESERVE IN is not implemented."); + prout_register_key(sd, 0, key); + return; + } + CU_ASSERT_EQUAL(ret, 0); + + /* + * SPC5r17: 6.16.2 READ KEYS service action + * The ADDITIONAL LENGTH field indicates the number of bytes in + * the Reservation key list. The contents of the ADDITIONAL + * LENGTH field are not altered based on the allocation length. + */ + CU_ASSERT_NOT_EQUAL(rk->additional_length, 0); + /* key array should have been truncated in the response */ + CU_ASSERT_EQUAL(rk->num_keys, 0); + + /* remove our key from the target */ + ret = prout_register_key(sd, 0, key); + CU_ASSERT_EQUAL(0, ret); +} diff --git a/test-tool/test_prin_report_caps.c b/test-tool/test_prin_report_caps.c index 705cfb5d..8e6c302a 100644 --- a/test-tool/test_prin_report_caps.c +++ b/test-tool/test_prin_report_caps.c @@ -63,7 +63,6 @@ test_prin_report_caps_simple(void) /* register our reservation key with the target */ ret = prout_register_and_ignore(sd, key); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTENT RESERVE OUT is not implemented."); CU_PASS("PERSISTENT RESERVE OUT is not implemented."); return; } diff --git a/test-tool/test_prin_serviceaction_range.c b/test-tool/test_prin_serviceaction_range.c index 4551c0aa..4bc1094f 100644 --- a/test-tool/test_prin_serviceaction_range.c +++ b/test-tool/test_prin_serviceaction_range.c @@ -38,12 +38,11 @@ test_prin_serviceaction_range(void) logging(LOG_VERBOSE, "Test Persistent Reserve IN Serviceaction range."); /* verify PRIN/READ_KEYS works -- XXX redundant -- remove this? */ - ret = prin_read_keys(sd, &task, NULL); + ret = prin_read_keys(sd, &task, NULL, 16384); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTEN RESERVE IN is not implemented."); CU_PASS("PERSISTENT RESERVE IN is not implemented."); return; - } + } CU_ASSERT_EQUAL(ret, 0); /* verify that PRIN/SA={0,1,2,3} works ... */ diff --git a/test-tool/test_prout_clear_simple.c b/test-tool/test_prout_clear_simple.c index 8bcfe47c..c5709457 100644 --- a/test-tool/test_prout_clear_simple.c +++ b/test-tool/test_prout_clear_simple.c @@ -43,13 +43,12 @@ test_prout_clear_simple(void) /* register our reservation key with the target */ ret = prout_register_and_ignore(sd, key); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTENT RESERVE OUT is not implemented."); CU_PASS("PERSISTENT RESERVE OUT is not implemented."); return; - } + } CU_ASSERT_EQUAL(ret, 0); - ret = prin_read_keys(sd, &tsk, &rk); + ret = prin_read_keys(sd, &tsk, &rk, 16384); CU_ASSERT_EQUAL(ret, 0); CU_ASSERT_NOT_EQUAL(rk, NULL); if (!rk) @@ -79,7 +78,7 @@ test_prout_clear_simple(void) ret = prin_verify_not_reserved(sd); CU_ASSERT_EQUAL(ret, 0); - ret = prin_read_keys(sd, &tsk, &rk); + ret = prin_read_keys(sd, &tsk, &rk, 16384); CU_ASSERT_EQUAL(ret, 0); CU_ASSERT_NOT_EQUAL(rk, NULL); if (!rk) diff --git a/test-tool/test_prout_preempt.c b/test-tool/test_prout_preempt.c index 201accac..0c11c335 100644 --- a/test-tool/test_prout_preempt.c +++ b/test-tool/test_prout_preempt.c @@ -54,7 +54,6 @@ test_prout_preempt_rm_reg(void) ret = prout_register_and_ignore(sd, k1); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTEN RESERVE OUT is not implemented."); CU_PASS("PERSISTENT RESERVE OUT is not implemented."); return; } @@ -79,7 +78,7 @@ test_prout_preempt_rm_reg(void) CU_ASSERT_EQUAL(ret, 0); /* confirm that k1 and k2 are registered */ - ret = prin_read_keys(sd, &tsk, &rk); + ret = prin_read_keys(sd, &tsk, &rk, 16384); CU_ASSERT_EQUAL_FATAL(ret, 0); CU_ASSERT_EQUAL(rk->num_keys, 2); @@ -100,7 +99,7 @@ test_prout_preempt_rm_reg(void) ret = test_iscsi_tur_until_good(sd2, &num_uas); CU_ASSERT_EQUAL(ret, 0); - ret = prin_read_keys(sd, &tsk, &rk); + ret = prin_read_keys(sd, &tsk, &rk, 16384); CU_ASSERT_EQUAL_FATAL(ret, 0); CU_ASSERT_EQUAL(rk->num_keys, 1); diff --git a/test-tool/test_prout_register_simple.c b/test-tool/test_prout_register_simple.c index c7058753..275f6925 100644 --- a/test-tool/test_prout_register_simple.c +++ b/test-tool/test_prout_register_simple.c @@ -40,7 +40,6 @@ test_prout_register_simple(void) /* register our reservation key with the target */ ret = prout_register_and_ignore(sd, key); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTEN RESERVE OUT is not implemented."); CU_PASS("PERSISTENT RESERVE OUT is not implemented."); return; } diff --git a/test-tool/test_prout_reserve_access.c b/test-tool/test_prout_reserve_access.c index 4f178480..2da88e06 100644 --- a/test-tool/test_prout_reserve_access.c +++ b/test-tool/test_prout_reserve_access.c @@ -50,7 +50,6 @@ verify_persistent_reserve_access(struct scsi_device *sd1, struct scsi_device *sd /* register our reservation key with the target */ ret = prout_register_and_ignore(sd1, key); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTEN RESERVE OUT is not implemented."); CU_PASS("PERSISTENT RESERVE OUT is not implemented."); return; } diff --git a/test-tool/test_prout_reserve_ownership.c b/test-tool/test_prout_reserve_ownership.c index b14478cc..1b47758a 100644 --- a/test-tool/test_prout_reserve_ownership.c +++ b/test-tool/test_prout_reserve_ownership.c @@ -47,7 +47,6 @@ verify_persistent_reserve_ownership(struct scsi_device *sd1, struct scsi_device /* register our reservation key with the target */ ret = prout_register_and_ignore(sd1, key1); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTEN RESERVE OUT is not implemented."); CU_PASS("PERSISTENT RESERVE OUT is not implemented."); return; } diff --git a/test-tool/test_prout_reserve_simple.c b/test-tool/test_prout_reserve_simple.c index 14d47932..4f9ebbda 100644 --- a/test-tool/test_prout_reserve_simple.c +++ b/test-tool/test_prout_reserve_simple.c @@ -55,7 +55,6 @@ test_prout_reserve_simple(void) /* register our reservation key with the target */ ret = prout_register_and_ignore(sd, key); if (ret == -2) { - logging(LOG_NORMAL, "[SKIPPED] PERSISTEN RESERVE OUT is not implemented."); CU_PASS("PERSISTENT RESERVE OUT is not implemented."); return; }