[ 087/175] target: Fix up handling of short INQUIRY buffers

From: Greg KH
Date: Fri Mar 30 2012 - 17:30:56 EST


3.3-stable review patch. If anyone has any objections, please let me know.

------------------

From: Roland Dreier <roland@xxxxxxxxxxxxxxx>

commit d95b82461c56a6ff8ff248b101049a69ebb20278 upstream.

If the initiator sends us an INQUIRY command with an allocation length
that's shorter than what we want to return, we're simply supposed to
truncate our response and return what the initiator gave us space for,
without signaling any error. Current target code has various tests that
don't fill out the full response if the buffer is too short and
sometimes return errors incorrectly.

Fix this up by allocating a bounce buffer for INQUIRY responses if we
need to, ie if we have cmd->data_length too small as well as
SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC set in cmd->se_cmd_flags -- for most
fabrics, we always allocate at least a full page, but for tcm_loop we
may have a small buffer coming directly from the SCSI stack.

This lets us delete a lot of cmd->data_length checking, and also makes
our INQUIRY handling correct per SPC in a lot more cases.

Signed-off-by: Roland Dreier <roland@xxxxxxxxxxxxxxx>
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
drivers/target/target_core_cdb.c | 150 +++++++++-----------------------------
include/target/target_core_base.h | 2
2 files changed, 37 insertions(+), 115 deletions(-)

--- a/drivers/target/target_core_cdb.c
+++ b/drivers/target/target_core_cdb.c
@@ -66,24 +66,11 @@ target_fill_alua_data(struct se_port *po
}

static int
-target_emulate_inquiry_std(struct se_cmd *cmd)
+target_emulate_inquiry_std(struct se_cmd *cmd, char *buf)
{
struct se_lun *lun = cmd->se_lun;
struct se_device *dev = cmd->se_dev;
struct se_portal_group *tpg = lun->lun_sep->sep_tpg;
- unsigned char *buf;
-
- /*
- * Make sure we at least have 6 bytes of INQUIRY response
- * payload going back for EVPD=0
- */
- if (cmd->data_length < 6) {
- pr_err("SCSI Inquiry payload length: %u"
- " too small for EVPD=0\n", cmd->data_length);
- return -EINVAL;
- }
-
- buf = transport_kmap_data_sg(cmd);

if (dev == tpg->tpg_virt_lun0.lun_se_dev) {
buf[0] = 0x3f; /* Not connected */
@@ -112,29 +99,13 @@ target_emulate_inquiry_std(struct se_cmd
if (dev->se_sub_dev->t10_alua.alua_type == SPC3_ALUA_EMULATED)
target_fill_alua_data(lun->lun_sep, buf);

- if (cmd->data_length < 8) {
- buf[4] = 1; /* Set additional length to 1 */
- goto out;
- }
-
buf[7] = 0x32; /* Sync=1 and CmdQue=1 */

- /*
- * Do not include vendor, product, reversion info in INQUIRY
- * response payload for cdbs with a small allocation length.
- */
- if (cmd->data_length < 36) {
- buf[4] = 3; /* Set additional length to 3 */
- goto out;
- }
-
snprintf(&buf[8], 8, "LIO-ORG");
snprintf(&buf[16], 16, "%s", dev->se_sub_dev->t10_wwn.model);
snprintf(&buf[32], 4, "%s", dev->se_sub_dev->t10_wwn.revision);
buf[4] = 31; /* Set additional length to 31 */

-out:
- transport_kunmap_data_sg(cmd);
return 0;
}

@@ -152,12 +123,6 @@ target_emulate_evpd_80(struct se_cmd *cm
unit_serial_len = strlen(dev->se_sub_dev->t10_wwn.unit_serial);
unit_serial_len++; /* For NULL Terminator */

- if (((len + 4) + unit_serial_len) > cmd->data_length) {
- len += unit_serial_len;
- buf[2] = ((len >> 8) & 0xff);
- buf[3] = (len & 0xff);
- return 0;
- }
len += sprintf(&buf[4], "%s",
dev->se_sub_dev->t10_wwn.unit_serial);
len++; /* Extra Byte for NULL Terminator */
@@ -229,9 +194,6 @@ target_emulate_evpd_83(struct se_cmd *cm
if (!(dev->se_sub_dev->su_dev_flags & SDF_EMULATED_VPD_UNIT_SERIAL))
goto check_t10_vend_desc;

- if (off + 20 > cmd->data_length)
- goto check_t10_vend_desc;
-
/* CODE SET == Binary */
buf[off++] = 0x1;

@@ -283,12 +245,6 @@ check_t10_vend_desc:
strlen(&dev->se_sub_dev->t10_wwn.unit_serial[0]);
unit_serial_len++; /* For NULL Terminator */

- if ((len + (id_len + 4) +
- (prod_len + unit_serial_len)) >
- cmd->data_length) {
- len += (prod_len + unit_serial_len);
- goto check_port;
- }
id_len += sprintf(&buf[off+12], "%s:%s", prod,
&dev->se_sub_dev->t10_wwn.unit_serial[0]);
}
@@ -306,7 +262,6 @@ check_t10_vend_desc:
/*
* struct se_port is only set for INQUIRY VPD=1 through $FABRIC_MOD
*/
-check_port:
port = lun->lun_sep;
if (port) {
struct t10_alua_lu_gp *lu_gp;
@@ -323,10 +278,6 @@ check_port:
* Get the PROTOCOL IDENTIFIER as defined by spc4r17
* section 7.5.1 Table 362
*/
- if (((len + 4) + 8) > cmd->data_length) {
- len += 8;
- goto check_tpgi;
- }
buf[off] =
(tpg->se_tpg_tfo->get_fabric_proto_ident(tpg) << 4);
buf[off++] |= 0x1; /* CODE SET == Binary */
@@ -350,15 +301,10 @@ check_port:
* Get the PROTOCOL IDENTIFIER as defined by spc4r17
* section 7.5.1 Table 362
*/
-check_tpgi:
if (dev->se_sub_dev->t10_alua.alua_type !=
SPC3_ALUA_EMULATED)
goto check_scsi_name;

- if (((len + 4) + 8) > cmd->data_length) {
- len += 8;
- goto check_lu_gp;
- }
tg_pt_gp_mem = port->sep_alua_tg_pt_gp_mem;
if (!tg_pt_gp_mem)
goto check_lu_gp;
@@ -391,10 +337,6 @@ check_tpgi:
* section 7.7.3.8
*/
check_lu_gp:
- if (((len + 4) + 8) > cmd->data_length) {
- len += 8;
- goto check_scsi_name;
- }
lu_gp_mem = dev->dev_alua_lu_gp_mem;
if (!lu_gp_mem)
goto check_scsi_name;
@@ -435,10 +377,6 @@ check_scsi_name:
/* Header size + Designation descriptor */
scsi_name_len += 4;

- if (((len + 4) + scsi_name_len) > cmd->data_length) {
- len += scsi_name_len;
- goto set_len;
- }
buf[off] =
(tpg->se_tpg_tfo->get_fabric_proto_ident(tpg) << 4);
buf[off++] |= 0x3; /* CODE SET == UTF-8 */
@@ -474,7 +412,6 @@ check_scsi_name:
/* Header size + Designation descriptor */
len += (scsi_name_len + 4);
}
-set_len:
buf[2] = ((len >> 8) & 0xff);
buf[3] = (len & 0xff); /* Page Length for VPD 0x83 */
return 0;
@@ -484,9 +421,6 @@ set_len:
static int
target_emulate_evpd_86(struct se_cmd *cmd, unsigned char *buf)
{
- if (cmd->data_length < 60)
- return 0;
-
buf[3] = 0x3c;
/* Set HEADSUP, ORDSUP, SIMPSUP */
buf[5] = 0x07;
@@ -512,20 +446,6 @@ target_emulate_evpd_b0(struct se_cmd *cm
if (dev->se_sub_dev->se_dev_attrib.emulate_tpu || dev->se_sub_dev->se_dev_attrib.emulate_tpws)
have_tp = 1;

- if (cmd->data_length < (0x10 + 4)) {
- pr_debug("Received data_length: %u"
- " too small for EVPD 0xb0\n",
- cmd->data_length);
- return -EINVAL;
- }
-
- if (have_tp && cmd->data_length < (0x3c + 4)) {
- pr_debug("Received data_length: %u"
- " too small for TPE=1 EVPD 0xb0\n",
- cmd->data_length);
- have_tp = 0;
- }
-
buf[0] = dev->transport->get_device_type(dev);
buf[3] = have_tp ? 0x3c : 0x10;

@@ -548,10 +468,9 @@ target_emulate_evpd_b0(struct se_cmd *cm
put_unaligned_be32(dev->se_sub_dev->se_dev_attrib.optimal_sectors, &buf[12]);

/*
- * Exit now if we don't support TP or the initiator sent a too
- * short buffer.
+ * Exit now if we don't support TP.
*/
- if (!have_tp || cmd->data_length < (0x3c + 4))
+ if (!have_tp)
return 0;

/*
@@ -589,10 +508,7 @@ target_emulate_evpd_b1(struct se_cmd *cm

buf[0] = dev->transport->get_device_type(dev);
buf[3] = 0x3c;
-
- if (cmd->data_length >= 5 &&
- dev->se_sub_dev->se_dev_attrib.is_nonrot)
- buf[5] = 1;
+ buf[5] = dev->se_sub_dev->se_dev_attrib.is_nonrot ? 1 : 0;

return 0;
}
@@ -671,8 +587,6 @@ target_emulate_evpd_00(struct se_cmd *cm
{
int p;

- if (cmd->data_length < 8)
- return 0;
/*
* Only report the INQUIRY EVPD=1 pages after a valid NAA
* Registered Extended LUN WWN has been set via ConfigFS
@@ -681,8 +595,7 @@ target_emulate_evpd_00(struct se_cmd *cm
if (cmd->se_dev->se_sub_dev->su_dev_flags &
SDF_EMULATED_VPD_UNIT_SERIAL) {
buf[3] = ARRAY_SIZE(evpd_handlers);
- for (p = 0; p < min_t(int, ARRAY_SIZE(evpd_handlers),
- cmd->data_length - 4); ++p)
+ for (p = 0; p < ARRAY_SIZE(evpd_handlers); ++p)
buf[p + 4] = evpd_handlers[p].page;
}

@@ -693,45 +606,50 @@ int target_emulate_inquiry(struct se_tas
{
struct se_cmd *cmd = task->task_se_cmd;
struct se_device *dev = cmd->se_dev;
- unsigned char *buf;
+ unsigned char *buf, *map_buf;
unsigned char *cdb = cmd->t_task_cdb;
int p, ret;

+ map_buf = transport_kmap_data_sg(cmd);
+ /*
+ * If SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC is not set, then we
+ * know we actually allocated a full page. Otherwise, if the
+ * data buffer is too small, allocate a temporary buffer so we
+ * don't have to worry about overruns in all our INQUIRY
+ * emulation handling.
+ */
+ if (cmd->data_length < SE_INQUIRY_BUF &&
+ (cmd->se_cmd_flags & SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC)) {
+ buf = kzalloc(SE_INQUIRY_BUF, GFP_KERNEL);
+ if (!buf) {
+ transport_kunmap_data_sg(cmd);
+ cmd->scsi_sense_reason = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+ return -ENOMEM;
+ }
+ } else {
+ buf = map_buf;
+ }
+
if (!(cdb[1] & 0x1)) {
if (cdb[2]) {
pr_err("INQUIRY with EVPD==0 but PAGE CODE=%02x\n",
cdb[2]);
cmd->scsi_sense_reason = TCM_INVALID_CDB_FIELD;
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}

- ret = target_emulate_inquiry_std(cmd);
+ ret = target_emulate_inquiry_std(cmd, buf);
goto out;
}

- /*
- * Make sure we at least have 4 bytes of INQUIRY response
- * payload for 0x00 going back for EVPD=1. Note that 0x80
- * and 0x83 will check for enough payload data length and
- * jump to set_len: label when there is not enough inquiry EVPD
- * payload length left for the next outgoing EVPD metadata
- */
- if (cmd->data_length < 4) {
- pr_err("SCSI Inquiry payload length: %u"
- " too small for EVPD=1\n", cmd->data_length);
- cmd->scsi_sense_reason = TCM_INVALID_CDB_FIELD;
- return -EINVAL;
- }
-
- buf = transport_kmap_data_sg(cmd);
-
buf[0] = dev->transport->get_device_type(dev);

for (p = 0; p < ARRAY_SIZE(evpd_handlers); ++p) {
if (cdb[2] == evpd_handlers[p].page) {
buf[1] = cdb[2];
ret = evpd_handlers[p].emulate(cmd, buf);
- goto out_unmap;
+ goto out;
}
}

@@ -739,9 +657,13 @@ int target_emulate_inquiry(struct se_tas
cmd->scsi_sense_reason = TCM_INVALID_CDB_FIELD;
ret = -EINVAL;

-out_unmap:
- transport_kunmap_data_sg(cmd);
out:
+ if (buf != map_buf) {
+ memcpy(map_buf, buf, cmd->data_length);
+ kfree(buf);
+ }
+ transport_kunmap_data_sg(cmd);
+
if (!ret) {
task->task_scsi_status = GOOD;
transport_complete_task(task, 1);
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -118,9 +118,9 @@
/* Queue Algorithm Modifier default for restricted reordering in control mode page */
#define DA_EMULATE_REST_REORD 0

+#define SE_INQUIRY_BUF 512
#define SE_MODE_PAGE_BUF 512

-
/* struct se_hba->hba_flags */
enum hba_flags_table {
HBA_FLAGS_INTERNAL_USE = 0x01,


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/