Commit c013db36 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Jim Harris
Browse files

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: default avatarKarandeep Chahal <devilsgrotto@gmail.com>
Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/393027


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 8bb603da

lib/scsi/scsi_bdev.c

+16 −2
Original line number Diff line number Diff line
@@ -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;

lib/scsi/scsi_internal.h

+1 −1
Original line number Diff line number Diff line
@@ -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];

test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c

+47 −0
Original line number Diff line number Diff line
@@ -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();