d/numam-spdk
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

scsi: do not fill out more than allocation length for standard inquiry

Browse Source 
The translation code currently cheats a bit - it allocates a full 4KB
buffer for any DATA_IN command that is not a READ, and then the
different SCSI commands that fall into this category (INQUIRY,
READ_CAPACITY, MODE_SENSE, etc.) can write as much data as they
want without having to worry about a buffer overrun.  Code higher
up the stack makes sure we only send the correct amount of data back
to the iSCSI initiator.

This patch fixes this behavior for standard INQUIRY (EVPD = 0).
Future patches will fix the behavior for other non-READ DATA_IN
commands, at which point we can remove the 4KB allocation and
only allocate the amount of data specified in the CDB.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: If5e4a10eeba9851e2d91cab71228d2fc2d5baad0
This commit is contained in:
Jim Harris 2016-09-06 12:36:29 -07:00 committed by Ben Walker
parent 9e501ce2fe
commit d3b58c006f
2 changed files with 91 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
lib/scsi/scsi_bdev.c
View File

@ -47,6 +47,9 @@
#define DEFAULT_DISK_ROTATION_RATE 7200 /* 7200 rpm */
#define DEFAULT_DISK_FORM_FACTOR 0x02 /* 3.5 inch */
#define INQUIRY_OFFSET(field) offsetof(struct spdk_scsi_cdb_inquiry_data, field) + \
sizeof(((struct spdk_scsi_cdb_inquiry_data *)0x0)->field)
static int
spdk_hex2bin(char ch)
{
@ -697,25 +700,50 @@ spdk_bdev_scsi_inquiry(struct spdk_bdev *bdev, struct spdk_scsi_task *task,
/* PRODUCT REVISION LEVEL */
spdk_strcpy_pad(inqdata->product_rev, DEFAULT_DISK_REVISION, 4, ' ');
/* Vendor specific */
memset(inqdata->vendor, 0x20, 20);
/*
* Standard inquiry data ends here. Only populate remaining fields if alloc_len
* indicates enough space to hold it.
*/
len = INQUIRY_OFFSET(product_rev) - 5;
/* CLOCKING(3-2) QAS(1) IUS(0) */
inqdata->ius = 0;
if (alloc_len >= INQUIRY_OFFSET(vendor)) {
/* Vendor specific */
memset(inqdata->vendor, 0x20, 20);
len += sizeof(inqdata->vendor);
}
/* Reserved */
inqdata->reserved = 0;
if (alloc_len >= INQUIRY_OFFSET(ius)) {
/* CLOCKING(3-2) QAS(1) IUS(0) */
inqdata->ius = 0;
len += sizeof(inqdata->ius);
}
if (alloc_len >= INQUIRY_OFFSET(reserved)) {
/* Reserved */
inqdata->reserved = 0;
len += sizeof(inqdata->reserved);
}
/* VERSION DESCRIPTOR 1-8 */
to_be16(inqdata->desc, 0x0960);
to_be16(&inqdata->desc[2], 0x0300); /* SPC-3 (no version claimed) */
to_be16(&inqdata->desc[4], 0x320); /* SBC-2 (no version claimed) */
to_be16(&inqdata->desc[6], 0x0040); /* SAM-2 (no version claimed) */
/* 96 - 74 + 8 */
/* Reserved[74-95] */
memset(&inqdata->desc[8], 0, 30);
if (alloc_len >= INQUIRY_OFFSET(reserved) + 2) {
to_be16(&inqdata->desc[0], 0x0960);
len += 2;
}
len = alloc_len - hlen;
if (alloc_len >= INQUIRY_OFFSET(reserved) + 4) {
to_be16(&inqdata->desc[2], 0x0300); /* SPC-3 (no version claimed) */
len += 2;
}
if (alloc_len >= INQUIRY_OFFSET(reserved) + 6) {
to_be16(&inqdata->desc[4], 0x320); /* SBC-2 (no version claimed) */
len += 2;
}
if (alloc_len >= INQUIRY_OFFSET(reserved) + 8) {
to_be16(&inqdata->desc[6], 0x0040); /* SAM-2 (no version claimed) */
len += 2;
}
/* ADDITIONAL LENGTH */
inqdata->add_len = len;
@ -724,6 +752,7 @@ spdk_bdev_scsi_inquiry(struct spdk_bdev *bdev, struct spdk_scsi_task *task,
return hlen + len;
inq_error:
task->data_transferred = 0;
spdk_scsi_task_set_check_condition(task,
SPDK_SCSI_SENSE_NO_SENSE,
0x0, 0x0);

48
test/lib/scsi/scsi_bdev/scsi_bdev_ut.c
View File

@ -416,6 +416,53 @@ inquiry_standard_test(void)
CU_ASSERT_EQUAL(rc, 0);
}
static void
_inquiry_overflow_test(uint8_t alloc_len)
{
struct spdk_bdev bdev;
struct spdk_scsi_task task;
struct spdk_scsi_lun lun;
struct spdk_scsi_dev dev;
struct spdk_bdev_fn_table fn_table;
uint8_t cdb[6];
/* expects a 4K internal data buffer */
char data[256], data_compare[256];
int rc;
bdev.fn_table = &fn_table;
cdb[0] = 0x12;
cdb[1] = 0x00; // EVPD = 0
cdb[2] = 0x00; // PageCode zero - requesting standard inquiry
cdb[3] = 0x00;
cdb[4] = alloc_len; // Indicate data size used by conformance test
cdb[5] = 0x00;
task.cdb = cdb;
snprintf(&dev.name[0], sizeof(dev.name), "spdk_iscsi_translation_test");
lun.dev = &dev;
task.lun = &lun;
memset(data, 0, sizeof(data));
memset(data_compare, 0, sizeof(data_compare));
task.rbuf = data;
rc = spdk_bdev_scsi_execute(&bdev, &task);
CU_ASSERT_EQUAL(rc, 0);
CU_ASSERT_EQUAL(memcmp(data + alloc_len, data_compare + alloc_len, sizeof(data) - alloc_len), 0);
CU_ASSERT(task.data_transferred <= alloc_len);
}
static void
inquiry_overflow_test(void)
{
int i;
for (i = 0; i < 256; i++) {
_inquiry_overflow_test(i);
}
}
int
main(int argc, char **argv)
{
@ -439,6 +486,7 @@ main(int argc, char **argv)
|| CU_add_test(suite, "mode sense 10 test", mode_sense_10_test) == NULL
|| CU_add_test(suite, "inquiry evpd test", inquiry_evpd_test) == NULL
|| CU_add_test(suite, "inquiry standard test", inquiry_standard_test) == NULL
|| CU_add_test(suite, "inquiry overflow test", inquiry_overflow_test) == NULL
) {
CU_cleanup_registry();
return CU_get_error();