From c013db36fc5bd75d1929951e4c579dead1368f0d Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 4 Jan 2018 11:58:32 +0900 Subject: [PATCH] scsi: SCSI string name format it appears that scsi name string designator format in SPDK is not correct. The name strings are not null terminated (which causees garbage to appear in scsi_inq -p 0x83). Further, they are not padded correctly. SCSI port name and device name strings must be null terminated. Further, the length must be a multiple of 4 bytes, and must be padded with 0s. See SPC-5 Section 7.7.6.11. Change-Id: Id7c4ad27e5c3a17ad68e5e466142801c0d03b1f2 Signed-off-by: Karandeep Chahal Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/393027 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris --- lib/scsi/scsi_bdev.c | 18 ++++++- lib/scsi/scsi_internal.h | 2 +- test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c | 47 +++++++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/lib/scsi/scsi_bdev.c b/lib/scsi/scsi_bdev.c index fe66708b9..a33cbc497 100644 --- a/lib/scsi/scsi_bdev.c +++ b/lib/scsi/scsi_bdev.c @@ -168,6 +168,20 @@ spdk_bdev_scsi_report_luns(struct spdk_scsi_lun *lun, return hlen + len; } +static int +spdk_bdev_scsi_pad_scsi_name(char *dst, const char *name) +{ + size_t len; + + len = strnlen(name, SPDK_SCSI_DEV_MAX_NAME); + memcpy(dst, name, len); + do { + dst[len++] = '\0'; + } while (len & 3); + + return len; +} + static int spdk_bdev_scsi_inquiry(struct spdk_bdev *bdev, struct spdk_scsi_task *task, uint8_t *cdb, uint8_t *data, uint16_t alloc_len) @@ -269,7 +283,7 @@ spdk_bdev_scsi_inquiry(struct spdk_bdev *bdev, struct spdk_scsi_task *task, /* Check total length by calculated how much space all entries take */ len = sizeof(struct spdk_scsi_desig_desc) + 8; len += sizeof(struct spdk_scsi_desig_desc) + 8 + 16 + MAX_SERIAL_STRING; - len += sizeof(struct spdk_scsi_desig_desc) + SPDK_SCSI_DEV_MAX_NAME; + len += sizeof(struct spdk_scsi_desig_desc) + SPDK_SCSI_DEV_MAX_NAME + 1; len += sizeof(struct spdk_scsi_desig_desc) + SPDK_SCSI_PORT_MAX_NAME_LENGTH; len += sizeof(struct spdk_scsi_desig_desc) + 4; len += sizeof(struct spdk_scsi_desig_desc) + 4; @@ -325,7 +339,7 @@ spdk_bdev_scsi_inquiry(struct spdk_bdev *bdev, struct spdk_scsi_task *task, desig->reserved0 = 0; desig->piv = 1; desig->reserved1 = 0; - desig->len = snprintf(desig->desig, SPDK_SCSI_DEV_MAX_NAME, "%s", dev->name); + desig->len = spdk_bdev_scsi_pad_scsi_name(desig->desig, dev->name); len += sizeof(struct spdk_scsi_desig_desc) + desig->len; buf += sizeof(struct spdk_scsi_desig_desc) + desig->len; diff --git a/lib/scsi/scsi_internal.h b/lib/scsi/scsi_internal.h index 3624ddd9e..aca7332bd 100644 --- a/lib/scsi/scsi_internal.h +++ b/lib/scsi/scsi_internal.h @@ -61,7 +61,7 @@ struct spdk_scsi_dev { int id; int is_allocated; - char name[SPDK_SCSI_DEV_MAX_NAME]; + char name[SPDK_SCSI_DEV_MAX_NAME + 1]; struct spdk_scsi_lun *lun[SPDK_SCSI_DEV_MAX_LUN]; 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 638187db4..c9c3d6ca2 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 @@ -532,6 +532,52 @@ inquiry_overflow_test(void) } } +static void +scsi_name_padding_test(void) +{ + char name[SPDK_SCSI_DEV_MAX_NAME + 10]; + char buf[SPDK_SCSI_DEV_MAX_NAME + 1]; + int written, i; + + /* case 1 */ + memset(name, '\0', sizeof(name)); + memset(name, 'x', 251); + written = spdk_bdev_scsi_pad_scsi_name(buf, name); + + CU_ASSERT(written == 252); + CU_ASSERT(buf[250] == 'x'); + CU_ASSERT(buf[251] == '\0'); + + /* case 2: */ + memset(name, '\0', sizeof(name)); + memset(name, 'x', 252); + written = spdk_bdev_scsi_pad_scsi_name(buf, name); + + CU_ASSERT(written == 256); + CU_ASSERT(buf[251] == 'x'); + for (i = 252; i < 256; i++) { + CU_ASSERT(buf[i] == '\0'); + } + + /* case 3 */ + memset(name, '\0', sizeof(name)); + memset(name, 'x', 255); + written = spdk_bdev_scsi_pad_scsi_name(buf, name); + + CU_ASSERT(written == 256); + CU_ASSERT(buf[254] == 'x'); + CU_ASSERT(buf[255] == '\0'); + + /* case 4 */ + memset(name, '\0', sizeof(name)); + memset(name, 'x', sizeof(name)); + written = spdk_bdev_scsi_pad_scsi_name(buf, name); + + CU_ASSERT(written == 256); + CU_ASSERT(buf[254] == 'x'); + CU_ASSERT(buf[255] == '\0'); +} + /* * This test is to verify specific error translation from bdev to scsi. */ @@ -718,6 +764,7 @@ main(int argc, char **argv) || 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_add_test(suite, "scsi name padding test", scsi_name_padding_test) == NULL ) { CU_cleanup_registry(); return CU_get_error();