From 4df179bfd44dac82f6523ec8b474d8702d639bd6 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Sun, 19 May 2013 08:18:09 -0700 Subject: [PATCH] TESTS: Add checks for CDB sanity to REPORT SUPPORTED OPCODES Read all individual opcodes and check that CDB length > 0 and that CDB[0] Usage Data is 0xFF --- include/scsi-lowlevel.h | 10 ++ lib/scsi-lowlevel.c | 156 ++++++++++++------ ...est_report_supported_opcodes_one_command.c | 40 ++++- 3 files changed, 154 insertions(+), 52 deletions(-) diff --git a/include/scsi-lowlevel.h b/include/scsi-lowlevel.h index acdc936..51c7cfe 100644 --- a/include/scsi-lowlevel.h +++ b/include/scsi-lowlevel.h @@ -760,6 +760,16 @@ struct scsi_report_supported_op_codes { struct scsi_command_descriptor descriptors[0]; }; +struct scsi_report_supported_op_codes_one_command { + uint8_t ctdp; + uint8_t support; + uint8_t cdb_length; + uint8_t cdb_usage_data[16]; + + /* only present if CTDP==1 */ + struct scsi_op_timeout_descriptor to; +}; + struct scsi_persistent_reserve_in_read_keys { uint32_t prgeneration; uint32_t additional_length; diff --git a/lib/scsi-lowlevel.c b/lib/scsi-lowlevel.c index c091539..c86968b 100644 --- a/lib/scsi-lowlevel.c +++ b/lib/scsi-lowlevel.c @@ -812,6 +812,12 @@ scsi_maintenancein_sa(const struct scsi_task *task) return task->cdb[1]; } +static inline uint8_t +scsi_report_supported_opcodes_options(const struct scsi_task *task) +{ + return task->cdb[2] & 0x07; +} + /* * parse the data in blob and calculate the size of a full maintenancein * datain structure @@ -822,7 +828,15 @@ scsi_maintenancein_datain_getfullsize(struct scsi_task *task) switch (scsi_maintenancein_sa(task)) { case SCSI_REPORT_SUPPORTED_OP_CODES: - return task_get_uint32(task, 0) + 4; + switch (scsi_report_supported_opcodes_options(task)) { + case SCSI_REPORT_SUPPORTING_OPS_ALL: + return task_get_uint32(task, 0) + 4; + case SCSI_REPORT_SUPPORTING_OPCODE: + case SCSI_REPORT_SUPPORTING_SERVICEACTION: + return 4 + + (task_get_uint8(task, 1) & 0x80) ? 12 : 0 + + task_get_uint16(task, 2); + } default: return -1; } @@ -835,64 +849,104 @@ static void * scsi_maintenancein_datain_unmarshall(struct scsi_task *task) { struct scsi_report_supported_op_codes *rsoc; + struct scsi_report_supported_op_codes_one_command *rsoc_one; int len, i; switch (scsi_maintenancein_sa(task)) { case SCSI_REPORT_SUPPORTED_OP_CODES: - if (task->datain.size < 4) { - return NULL; - } - - len = task_get_uint32(task, 0); - /* len / 8 is not always correct since if CTDP==1 then the - * descriptor is 20 bytes in size intead of 8. - * It doesnt matter here though since it just means we would - * allocate more descriptors at the end of the structure than - * we strictly need. This avoids having to traverse the - * datain buffer twice. - */ - rsoc = scsi_malloc(task, - offsetof(struct scsi_report_supported_op_codes, - descriptors) + - len / 8 * sizeof(struct scsi_command_descriptor)); - if (rsoc == NULL) { - return NULL; - } - - rsoc->num_descriptors = 0; - i = 4; - while (len >= 8) { - struct scsi_command_descriptor *desc; - - desc = &rsoc->descriptors[rsoc->num_descriptors++]; - desc->opcode = task_get_uint8(task, i); - desc->sa = task_get_uint16(task, i + 2); - desc->ctdp = !!(task_get_uint8(task, i + 5) & 0x02); - desc->servactv = !!(task_get_uint8(task, i + 5) & 0x01); - desc->cdb_len = task_get_uint16(task, i + 6); - - len -= 8; - i += 8; - - /* No tiemout description */ - if (!desc->ctdp) { - continue; + switch (scsi_report_supported_opcodes_options(task)) { + case SCSI_REPORT_SUPPORTING_OPS_ALL: + if (task->datain.size < 4) { + return NULL; } - desc->to.descriptor_length = - task_get_uint16(task, i); - desc->to.command_specific = - task_get_uint8(task, i + 3); - desc->to.nominal_processing_timeout = - task_get_uint32(task, i + 4); - desc->to.recommended_timeout = - task_get_uint32(task, i + 8); + len = task_get_uint32(task, 0); + /* len / 8 is not always correct since if CTDP==1 then + * the descriptor is 20 bytes in size intead of 8. + * It doesnt matter here though since it just means + * we would allocate more descriptors at the end of + * the structure than we strictly need. This avoids + * having to traverse the datain buffer twice. + */ + rsoc = scsi_malloc(task, + offsetof(struct scsi_report_supported_op_codes, + descriptors) + + len / 8 * sizeof(struct scsi_command_descriptor)); + if (rsoc == NULL) { + return NULL; + } - len -= desc->to.descriptor_length + 2; - i += desc->to.descriptor_length + 2; + rsoc->num_descriptors = 0; + i = 4; + while (len >= 8) { + struct scsi_command_descriptor *desc; + + desc = &rsoc->descriptors[rsoc->num_descriptors++]; + desc->opcode = + task_get_uint8(task, i); + desc->sa = + task_get_uint16(task, i + 2); + desc->ctdp = + !!(task_get_uint8(task, i + 5) & 0x02); + desc->servactv = + !!(task_get_uint8(task, i + 5) & 0x01); + desc->cdb_len = + task_get_uint16(task, i + 6); + + len -= 8; + i += 8; + + /* No tiemout description */ + if (!desc->ctdp) { + continue; + } + + desc->to.descriptor_length = + task_get_uint16(task, i); + desc->to.command_specific = + task_get_uint8(task, i + 3); + desc->to.nominal_processing_timeout = + task_get_uint32(task, i + 4); + desc->to.recommended_timeout = + task_get_uint32(task, i + 8); + + len -= desc->to.descriptor_length + 2; + i += desc->to.descriptor_length + 2; + } + return rsoc; + case SCSI_REPORT_SUPPORTING_OPCODE: + case SCSI_REPORT_SUPPORTING_SERVICEACTION: + rsoc_one = scsi_malloc(task, sizeof(struct scsi_report_supported_op_codes_one_command)); + if (rsoc_one == NULL) { + return NULL; + } + + rsoc_one->ctdp = + !!(task_get_uint8(task, 1) & 0x80); + rsoc_one->support = + task_get_uint8(task, 1) & 0x07; + rsoc_one->cdb_length = + task_get_uint16(task, 2); + if(rsoc_one->cdb_length <= 16) { + memcpy(rsoc_one->cdb_usage_data, + &task->datain.data[4], + rsoc_one->cdb_length); + } + + if (rsoc_one->ctdp) { + i = 4 + rsoc_one->cdb_length; + + rsoc_one->to.descriptor_length = + task_get_uint16(task, i); + rsoc_one->to.command_specific = + task_get_uint8(task, i + 3); + rsoc_one->to.nominal_processing_timeout = + task_get_uint32(task, i + 4); + rsoc_one->to.recommended_timeout = + task_get_uint32(task, i + 8); + } + return rsoc_one; } - - return rsoc; }; return NULL; diff --git a/test-tool/test_report_supported_opcodes_one_command.c b/test-tool/test_report_supported_opcodes_one_command.c index 8ada542..be4b40b 100644 --- a/test-tool/test_report_supported_opcodes_one_command.c +++ b/test-tool/test_report_supported_opcodes_one_command.c @@ -30,7 +30,9 @@ test_report_supported_opcodes_one_command(void) { int i, ret; struct scsi_task *rso_task; + struct scsi_task *one_task; struct scsi_report_supported_op_codes *rsoc; + struct scsi_report_supported_op_codes_one_command *rsoc_one; logging(LOG_VERBOSE, LOG_BLANK_LINE); logging(LOG_VERBOSE, "Test READ_SUPPORTED_OPCODES reading one-command"); @@ -56,7 +58,7 @@ test_report_supported_opcodes_one_command(void) CU_ASSERT_NOT_EQUAL(rsoc, NULL); - logging(LOG_VERBOSE, "Verify read one-command for all supported " + logging(LOG_VERBOSE, "Verify read one-command works for all supported " "opcodes"); for (i = 0; i < rsoc->num_descriptors; i++) { logging(LOG_VERBOSE, "Check opcode:0x%02x ServiceAction:0x%02x", @@ -107,5 +109,41 @@ test_report_supported_opcodes_one_command(void) CU_ASSERT_EQUAL(ret, 0); } + + logging(LOG_VERBOSE, "Verify read one-command CDB looks sane"); + for (i = 0; i < rsoc->num_descriptors; i++) { + logging(LOG_VERBOSE, "Check CDB for opcode:0x%02x " + "ServiceAction:0x%02x", + rsoc->descriptors[i].opcode, + rsoc->descriptors[i].sa); + ret = report_supported_opcodes( + iscsic, tgt_lun, + 0, + rsoc->descriptors[i].servactv ? + SCSI_REPORT_SUPPORTING_SERVICEACTION : + SCSI_REPORT_SUPPORTING_OPCODE, + rsoc->descriptors[i].opcode, + rsoc->descriptors[i].sa, + 65535, &one_task); + + logging(LOG_VERBOSE, "Unmarshall the DATA-IN buffer"); + rsoc_one = scsi_datain_unmarshall(one_task); + CU_ASSERT_NOT_EQUAL(rsoc_one, NULL); + + logging(LOG_VERBOSE, "Verify CDB length is not 0"); + CU_ASSERT_NOT_EQUAL(rsoc_one->cdb_length, 0); + if (rsoc_one->cdb_length != 0) { + logging(LOG_NORMAL, "[FAILED] CDB length is 0"); + } + + logging(LOG_VERBOSE, "Verify CDB[0] Usage Data is 0xFF"); + CU_ASSERT_EQUAL(rsoc_one->cdb_usage_data[0], 0xff); + if (rsoc_one->cdb_usage_data[0] != 0xff) { + logging(LOG_NORMAL, "[FAILED] CDB[0] Usage Data is " + "not 0xFF"); + } + } + + scsi_free_scsi_task(rso_task); }