From 0b1bbb45322ca42d2915e3213875f191ec5ba6a4 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 12 May 2017 13:24:15 -0700 Subject: [PATCH] scsi_bdev: add Maximum Transfer Length check We report SPDK_WORK_BLOCK_SIZE (1 MiB) as the Maximum Transfer Length in the Block Limits VPD. The initiator should not submit a request larger than this; if it does, fail the request. Change-Id: I39575cdca4555ac1f78e055a48569be3f47e4781 Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/373162 Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/scsi/scsi_bdev.c | 14 ++++++ test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c | 47 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/lib/scsi/scsi_bdev.c b/lib/scsi/scsi_bdev.c index 2ae147010..2a3b80618 100644 --- a/lib/scsi/scsi_bdev.c +++ b/lib/scsi/scsi_bdev.c @@ -1443,6 +1443,8 @@ spdk_bdev_scsi_readwrite(struct spdk_bdev *bdev, struct spdk_scsi_task *task, uint64_t lba, uint32_t xfer_len, bool is_read) { + uint32_t max_xfer_len; + if (task->dxfer_dir != SPDK_SCSI_DIR_NONE && task->dxfer_dir != (is_read ? SPDK_SCSI_DIR_FROM_DEV : SPDK_SCSI_DIR_TO_DEV)) { SPDK_ERRLOG("Incorrect data direction\n"); @@ -1459,6 +1461,18 @@ spdk_bdev_scsi_readwrite(struct spdk_bdev *bdev, return SPDK_SCSI_TASK_COMPLETE; } + /* Transfer Length is limited to the Block Limits VPD page Maximum Transfer Length */ + max_xfer_len = SPDK_WORK_BLOCK_SIZE / spdk_bdev_get_block_size(bdev); + if (xfer_len > max_xfer_len) { + SPDK_ERRLOG("xfer_len %" PRIu32 " > maximum transfer length %" PRIu32 "\n", + xfer_len, max_xfer_len); + spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, + SPDK_SCSI_SENSE_ILLEGAL_REQUEST, + SPDK_SCSI_ASC_INVALID_FIELD_IN_CDB, + SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); + return SPDK_SCSI_TASK_COMPLETE; + } + if (is_read) { return spdk_bdev_scsi_read(bdev, task, lba, xfer_len); } else { diff --git a/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c b/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c index d342efb29..08de75af5 100644 --- a/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c +++ b/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c @@ -629,6 +629,52 @@ lba_range_test(void) spdk_put_task(&task); } +static void +xfer_len_test(void) +{ + struct spdk_bdev bdev; + struct spdk_scsi_task task; + uint8_t cdb[16]; + int rc; + + spdk_init_task(&task); + task.cdb = cdb; + + memset(cdb, 0, sizeof(cdb)); + cdb[0] = 0x88; /* READ (16) */ + + /* Test block device size of 512 MiB */ + g_test_bdev_num_blocks = 512 * 1024 * 1024; + + /* 1 block */ + to_be64(&cdb[2], 0); /* LBA */ + to_be32(&cdb[10], 1); /* transfer length */ + task.transfer_len = 1 * 512; + rc = spdk_bdev_scsi_execute(&bdev, &task); + CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); + CU_ASSERT(task.status == SPDK_SCSI_STATUS_GOOD); + + /* max transfer length (as reported in block limits VPD page) */ + to_be64(&cdb[2], 0); /* LBA */ + to_be32(&cdb[10], SPDK_WORK_BLOCK_SIZE / 512); /* transfer length */ + task.transfer_len = SPDK_WORK_BLOCK_SIZE; + rc = spdk_bdev_scsi_execute(&bdev, &task); + CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); + CU_ASSERT(task.status == SPDK_SCSI_STATUS_GOOD); + + /* max transfer length plus one block (invalid) */ + to_be64(&cdb[2], 0); /* LBA */ + to_be32(&cdb[10], SPDK_WORK_BLOCK_SIZE / 512 + 1); /* transfer length */ + task.transfer_len = SPDK_WORK_BLOCK_SIZE + 512; + rc = spdk_bdev_scsi_execute(&bdev, &task); + CU_ASSERT(rc == SPDK_SCSI_TASK_COMPLETE); + CU_ASSERT(task.status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT((task.sense_data[2] & 0xf) == SPDK_SCSI_SENSE_ILLEGAL_REQUEST); + CU_ASSERT(task.sense_data[12] == SPDK_SCSI_ASC_INVALID_FIELD_IN_CDB); + + spdk_put_task(&task); +} + int main(int argc, char **argv) { @@ -655,6 +701,7 @@ main(int argc, char **argv) || CU_add_test(suite, "inquiry overflow test", inquiry_overflow_test) == NULL || CU_add_test(suite, "task complete test", task_complete_test) == NULL || CU_add_test(suite, "LBA range test", lba_range_test) == NULL + || CU_add_test(suite, "transfer length test", xfer_len_test) == NULL ) { CU_cleanup_registry(); return CU_get_error();