From e864212078ded276bdb272b2e0ee6a979357ca8a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 23 Dec 2016 11:37:53 +0100 Subject: [PATCH 01/11] target: add XCOPY target/segment desc sense codes As defined in http://www.t10.org/lists/asc-num.htm. To be used during validation of XCOPY target and segment descriptor lists. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_transport.c | 24 ++++++++++++++++++++++++ include/target/target_core_base.h | 4 ++++ 2 files changed, 28 insertions(+) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 7dfefd66df93..1cadc9eefa21 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1693,6 +1693,10 @@ void transport_generic_request_failure(struct se_cmd *cmd, case TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED: case TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED: case TCM_COPY_TARGET_DEVICE_NOT_REACHABLE: + case TCM_TOO_MANY_TARGET_DESCS: + case TCM_UNSUPPORTED_TARGET_DESC_TYPE_CODE: + case TCM_TOO_MANY_SEGMENT_DESCS: + case TCM_UNSUPPORTED_SEGMENT_DESC_TYPE_CODE: break; case TCM_OUT_OF_RESOURCES: sense_reason = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; @@ -2808,6 +2812,26 @@ static const struct sense_info sense_info_table[] = { .key = ILLEGAL_REQUEST, .asc = 0x26, /* INVALID FIELD IN PARAMETER LIST */ }, + [TCM_TOO_MANY_TARGET_DESCS] = { + .key = ILLEGAL_REQUEST, + .asc = 0x26, + .ascq = 0x06, /* TOO MANY TARGET DESCRIPTORS */ + }, + [TCM_UNSUPPORTED_TARGET_DESC_TYPE_CODE] = { + .key = ILLEGAL_REQUEST, + .asc = 0x26, + .ascq = 0x07, /* UNSUPPORTED TARGET DESCRIPTOR TYPE CODE */ + }, + [TCM_TOO_MANY_SEGMENT_DESCS] = { + .key = ILLEGAL_REQUEST, + .asc = 0x26, + .ascq = 0x08, /* TOO MANY SEGMENT DESCRIPTORS */ + }, + [TCM_UNSUPPORTED_SEGMENT_DESC_TYPE_CODE] = { + .key = ILLEGAL_REQUEST, + .asc = 0x26, + .ascq = 0x09, /* UNSUPPORTED SEGMENT DESCRIPTOR TYPE CODE */ + }, [TCM_PARAMETER_LIST_LENGTH_ERROR] = { .key = ILLEGAL_REQUEST, .asc = 0x1a, /* PARAMETER LIST LENGTH ERROR */ diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 29e6858bb164..43edf82e54ff 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -174,6 +174,10 @@ enum tcm_sense_reason_table { TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED = R(0x16), TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED = R(0x17), TCM_COPY_TARGET_DEVICE_NOT_REACHABLE = R(0x18), + TCM_TOO_MANY_TARGET_DESCS = R(0x19), + TCM_UNSUPPORTED_TARGET_DESC_TYPE_CODE = R(0x1a), + TCM_TOO_MANY_SEGMENT_DESCS = R(0x1b), + TCM_UNSUPPORTED_SEGMENT_DESC_TYPE_CODE = R(0x1c), #undef R }; From 61c359194c46cbffec9a1f2c59c1c4011222ad84 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 23 Dec 2016 11:37:54 +0100 Subject: [PATCH 02/11] target: use XCOPY TOO MANY TARGET DESCRIPTORS sense spc4r37 6.4.3.4 states: If the number of CSCD descriptors exceeds the allowed number, the copy manager shall terminate the command with CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to TOO MANY TARGET DESCRIPTORS. LIO currently responds with INVALID FIELD IN PARAMETER LIST, which sees it fail the libiscsi ExtendedCopy.DescrLimits test. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 37d5caebffa6..db265ad10fa4 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -201,9 +201,11 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, " multiple of %d\n", XCOPY_TARGET_DESC_LEN); return -EINVAL; } - if (tdll > 64) { + if (tdll > RCR_OP_MAX_TARGET_DESC_COUNT * XCOPY_TARGET_DESC_LEN) { pr_err("XCOPY target descriptor supports a maximum" " two src/dest descriptors, tdll: %hu too large..\n", tdll); + /* spc4r37 6.4.3.4 CSCD DESCRIPTOR LIST LENGTH field */ + *sense_ret = TCM_TOO_MANY_TARGET_DESCS; return -EINVAL; } /* From af9f62c1686268c0517b289274d38f3a03bebd2a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 23 Dec 2016 11:37:55 +0100 Subject: [PATCH 03/11] target: bounds check XCOPY segment descriptor list Check the length of the XCOPY request segment descriptor list against the value advertised via the MAXIMUM SEGMENT DESCRIPTOR COUNT field in the RECEIVE COPY OPERATING PARAMETERS response. spc4r37 6.4.3.5 states: If the number of segment descriptors exceeds the allowed number, the copy manager shall terminate the command with CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to TOO MANY SEGMENT DESCRIPTORS. This functionality is testable using the libiscsi ExtendedCopy.DescrLimits test. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index db265ad10fa4..da0f2da732e7 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -308,17 +308,26 @@ static int target_xcopy_parse_segdesc_02(struct se_cmd *se_cmd, struct xcopy_op static int target_xcopy_parse_segment_descriptors(struct se_cmd *se_cmd, struct xcopy_op *xop, unsigned char *p, - unsigned int sdll) + unsigned int sdll, sense_reason_t *sense_ret) { unsigned char *desc = p; unsigned int start = 0; int offset = sdll % XCOPY_SEGMENT_DESC_LEN, rc, ret = 0; + *sense_ret = TCM_INVALID_PARAMETER_LIST; + if (offset != 0) { pr_err("XCOPY segment descriptor list length is not" " multiple of %d\n", XCOPY_SEGMENT_DESC_LEN); return -EINVAL; } + if (sdll > RCR_OP_MAX_SG_DESC_COUNT * XCOPY_SEGMENT_DESC_LEN) { + pr_err("XCOPY supports %u segment descriptor(s), sdll: %u too" + " large..\n", RCR_OP_MAX_SG_DESC_COUNT, sdll); + /* spc4r37 6.4.3.5 SEGMENT DESCRIPTOR LIST LENGTH field */ + *sense_ret = TCM_TOO_MANY_SEGMENT_DESCS; + return -EINVAL; + } while (start < sdll) { /* @@ -916,7 +925,8 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) seg_desc = &p[16]; seg_desc += (rc * XCOPY_TARGET_DESC_LEN); - rc = target_xcopy_parse_segment_descriptors(se_cmd, xop, seg_desc, sdll); + rc = target_xcopy_parse_segment_descriptors(se_cmd, xop, seg_desc, + sdll, &ret); if (rc <= 0) { xcopy_pt_undepend_remotedev(xop); goto out; From 7d38706669ce00603b187f667a4eb67c94eac098 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 23 Dec 2016 11:37:56 +0100 Subject: [PATCH 04/11] target: bounds check XCOPY total descriptor list length spc4r37 6.4.3.5 states: If the combined length of the CSCD descriptors and segment descriptors exceeds the allowed value, then the copy manager shall terminate the command with CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to PARAMETER LIST LENGTH ERROR. This functionality can be tested using the libiscsi ExtendedCopy.DescrLimits test. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index da0f2da732e7..0d10fcf438d1 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -894,6 +894,12 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) */ tdll = get_unaligned_be16(&p[2]); sdll = get_unaligned_be32(&p[8]); + if (tdll + sdll > RCR_OP_MAX_DESC_LIST_LEN) { + pr_err("XCOPY descriptor list length %u exceeds maximum %u\n", + tdll + sdll, RCR_OP_MAX_DESC_LIST_LEN); + ret = TCM_PARAMETER_LIST_LENGTH_ERROR; + goto out; + } inline_dl = get_unaligned_be32(&p[12]); if (inline_dl != 0) { From c243849720ac237e9e7191fe57f619bb3a871d4c Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 2 Jan 2017 18:04:04 +0100 Subject: [PATCH 05/11] target: return UNSUPPORTED TARGET/SEGMENT DESC TYPE CODE sense Use UNSUPPORTED TARGET DESCRIPTOR TYPE CODE and UNSUPPORTED SEGMENT DESCRIPTOR TYPE CODE additional sense codes if a descriptor type in an XCOPY request is not supported, as specified in spc4r37 6.4.5 and 6.4.6. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 0d10fcf438d1..52738a108e5f 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -199,6 +199,7 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, if (offset != 0) { pr_err("XCOPY target descriptor list length is not" " multiple of %d\n", XCOPY_TARGET_DESC_LEN); + *sense_ret = TCM_UNSUPPORTED_TARGET_DESC_TYPE_CODE; return -EINVAL; } if (tdll > RCR_OP_MAX_TARGET_DESC_COUNT * XCOPY_TARGET_DESC_LEN) { @@ -240,6 +241,7 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, default: pr_err("XCOPY unsupported descriptor type code:" " 0x%02x\n", desc[0]); + *sense_ret = TCM_UNSUPPORTED_TARGET_DESC_TYPE_CODE; goto out; } } @@ -319,6 +321,7 @@ static int target_xcopy_parse_segment_descriptors(struct se_cmd *se_cmd, if (offset != 0) { pr_err("XCOPY segment descriptor list length is not" " multiple of %d\n", XCOPY_SEGMENT_DESC_LEN); + *sense_ret = TCM_UNSUPPORTED_SEGMENT_DESC_TYPE_CODE; return -EINVAL; } if (sdll > RCR_OP_MAX_SG_DESC_COUNT * XCOPY_SEGMENT_DESC_LEN) { @@ -346,6 +349,7 @@ static int target_xcopy_parse_segment_descriptors(struct se_cmd *se_cmd, default: pr_err("XCOPY unsupported segment descriptor" "type: 0x%02x\n", desc[0]); + *sense_ret = TCM_UNSUPPORTED_SEGMENT_DESC_TYPE_CODE; goto out; } } From 94aae4caacda89a1bdb7198b260f4ca3595b7ed7 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 2 Jan 2017 18:04:05 +0100 Subject: [PATCH 06/11] target: simplify XCOPY wwn->se_dev lookup helper target_xcopy_locate_se_dev_e4() is used to locate an se_dev, based on the WWN provided with the XCOPY request. Remove a couple of unneeded arguments, and rely on the caller for the src/dst test. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 52738a108e5f..155db18ee4e7 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -53,18 +53,13 @@ static int target_xcopy_gen_naa_ieee(struct se_device *dev, unsigned char *buf) return 0; } -static int target_xcopy_locate_se_dev_e4(struct se_cmd *se_cmd, struct xcopy_op *xop, - bool src) +static int target_xcopy_locate_se_dev_e4(const unsigned char *dev_wwn, + struct se_device **found_dev) { struct se_device *se_dev; - unsigned char tmp_dev_wwn[XCOPY_NAA_IEEE_REGEX_LEN], *dev_wwn; + unsigned char tmp_dev_wwn[XCOPY_NAA_IEEE_REGEX_LEN]; int rc; - if (src) - dev_wwn = &xop->dst_tid_wwn[0]; - else - dev_wwn = &xop->src_tid_wwn[0]; - mutex_lock(&g_device_mutex); list_for_each_entry(se_dev, &g_device_list, g_dev_node) { @@ -78,15 +73,8 @@ static int target_xcopy_locate_se_dev_e4(struct se_cmd *se_cmd, struct xcopy_op if (rc != 0) continue; - if (src) { - xop->dst_dev = se_dev; - pr_debug("XCOPY 0xe4: Setting xop->dst_dev: %p from located" - " se_dev\n", xop->dst_dev); - } else { - xop->src_dev = se_dev; - pr_debug("XCOPY 0xe4: Setting xop->src_dev: %p from located" - " se_dev\n", xop->src_dev); - } + *found_dev = se_dev; + pr_debug("XCOPY 0xe4: located se_dev: %p\n", se_dev); rc = target_depend_item(&se_dev->dev_group.cg_item); if (rc != 0) { @@ -247,9 +235,11 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, } if (xop->op_origin == XCOL_SOURCE_RECV_OP) - rc = target_xcopy_locate_se_dev_e4(se_cmd, xop, true); + rc = target_xcopy_locate_se_dev_e4(xop->dst_tid_wwn, + &xop->dst_dev); else - rc = target_xcopy_locate_se_dev_e4(se_cmd, xop, false); + rc = target_xcopy_locate_se_dev_e4(xop->src_tid_wwn, + &xop->src_dev); /* * If a matching IEEE NAA 0x83 descriptor for the requested device * is not located on this node, return COPY_ABORTED with ASQ/ASQC From f184210bca6c9d0091ff5e5629dea4cbb8a17c0f Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 2 Jan 2017 18:04:06 +0100 Subject: [PATCH 07/11] target: check XCOPY segment descriptor CSCD IDs Ensure that the segment descriptor CSCD descriptor ID values correspond to CSCD descriptor entries located in the XCOPY command parameter list. SPC4r37 6.4.6.1 Table 150 specifies this range as 0000h to 07FFh, where the CSCD descriptor location in the parameter list can be located via: 16 + (id * 32) Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig [ bvanassche: inserted "; " in the format string of an error message and also moved a "||" operator from the start of a line to the end of the previous line ] Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 8 ++++++++ drivers/target/target_core_xcopy.h | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 155db18ee4e7..41a2a8ad1046 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -278,6 +278,14 @@ static int target_xcopy_parse_segdesc_02(struct se_cmd *se_cmd, struct xcopy_op xop->stdi = get_unaligned_be16(&desc[4]); xop->dtdi = get_unaligned_be16(&desc[6]); + + if (xop->stdi > XCOPY_CSCD_DESC_ID_LIST_OFF_MAX || + xop->dtdi > XCOPY_CSCD_DESC_ID_LIST_OFF_MAX) { + pr_err("XCOPY segment desc 0x02: unsupported CSCD ID > 0x%x; stdi: %hu dtdi: %hu\n", + XCOPY_CSCD_DESC_ID_LIST_OFF_MAX, xop->stdi, xop->dtdi); + return -EINVAL; + } + pr_debug("XCOPY seg desc 0x02: desc_len: %hu stdi: %hu dtdi: %hu, DC: %d\n", desc_len, xop->stdi, xop->dtdi, dc); diff --git a/drivers/target/target_core_xcopy.h b/drivers/target/target_core_xcopy.h index 4d3d4dd060f2..e2d141140342 100644 --- a/drivers/target/target_core_xcopy.h +++ b/drivers/target/target_core_xcopy.h @@ -5,6 +5,12 @@ #define XCOPY_NAA_IEEE_REGEX_LEN 16 #define XCOPY_MAX_SECTORS 1024 +/* + * SPC4r37 6.4.6.1 + * Table 150 — CSCD descriptor ID values + */ +#define XCOPY_CSCD_DESC_ID_LIST_OFF_MAX 0x07FF + enum xcopy_origin_list { XCOL_SOURCE_RECV_OP = 0x01, XCOL_DEST_RECV_OP = 0x02, From 66640d35c1e4ef3c96ba5edb3c5e2ff8ab812e7a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 2 Jan 2017 18:04:07 +0100 Subject: [PATCH 08/11] target: use XCOPY segment descriptor CSCD IDs The XCOPY specification in SPC4r37 states that the XCOPY source and destination device(s) should be derived from the copy source and copy destination (CSCD) descriptor IDs in the XCOPY segment descriptor. The CSCD IDs are generally (for block -> block copies), indexes into the corresponding CSCD descriptor list, e.g. ================================= EXTENDED COPY Header ================================= CSCD Descriptor List - entry 0 + LU ID <--------------<------------------\ - entry 1 | + LU ID <______________<_____________ | ================================= | | Segment Descriptor List | | - segment 0 | | + src CSCD ID = 0 --------->---------+----/ + dest CSCD ID = 1 ___________>______| + len + src lba + dest lba ================================= Currently LIO completely ignores the src and dest CSCD IDs in the Segment Descriptor List, and instead assumes that the first entry in the CSCD list corresponds to the source, and the second to the destination. This commit removes this assumption, by ensuring that the Segment Descriptor List is parsed prior to processing the CSCD Descriptor List. CSCD Descriptor List processing is modified to compare the current list index with the previously obtained src and dest CSCD IDs. Additionally, XCOPY requests where the src and dest CSCD IDs refer to the CSCD Descriptor List entry can now be successfully processed. Fixes: cbf031f ("target: Add support for EXTENDED_COPY copy offload") Link: https://bugzilla.kernel.org/show_bug.cgi?id=191381 Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 79 ++++++++++++++++++------------ 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 41a2a8ad1046..2595c1eb9e91 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -98,7 +98,7 @@ static int target_xcopy_locate_se_dev_e4(const unsigned char *dev_wwn, } static int target_xcopy_parse_tiddesc_e4(struct se_cmd *se_cmd, struct xcopy_op *xop, - unsigned char *p, bool src) + unsigned char *p, unsigned short cscd_index) { unsigned char *desc = p; unsigned short ript; @@ -143,7 +143,13 @@ static int target_xcopy_parse_tiddesc_e4(struct se_cmd *se_cmd, struct xcopy_op return -EINVAL; } - if (src) { + if (cscd_index != xop->stdi && cscd_index != xop->dtdi) { + pr_debug("XCOPY 0xe4: ignoring CSCD entry %d - neither src nor " + "dest\n", cscd_index); + return 0; + } + + if (cscd_index == xop->stdi) { memcpy(&xop->src_tid_wwn[0], &desc[8], XCOPY_NAA_IEEE_REGEX_LEN); /* * Determine if the source designator matches the local device @@ -155,10 +161,15 @@ static int target_xcopy_parse_tiddesc_e4(struct se_cmd *se_cmd, struct xcopy_op pr_debug("XCOPY 0xe4: Set xop->src_dev %p from source" " received xop\n", xop->src_dev); } - } else { + } + + if (cscd_index == xop->dtdi) { memcpy(&xop->dst_tid_wwn[0], &desc[8], XCOPY_NAA_IEEE_REGEX_LEN); /* - * Determine if the destination designator matches the local device + * Determine if the destination designator matches the local + * device. If @cscd_index corresponds to both source (stdi) and + * destination (dtdi), or dtdi comes after stdi, then + * XCOL_DEST_RECV_OP wins. */ if (!memcmp(&xop->local_dev_wwn[0], &xop->dst_tid_wwn[0], XCOPY_NAA_IEEE_REGEX_LEN)) { @@ -178,9 +189,9 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, { struct se_device *local_dev = se_cmd->se_dev; unsigned char *desc = p; - int offset = tdll % XCOPY_TARGET_DESC_LEN, rc, ret = 0; + int offset = tdll % XCOPY_TARGET_DESC_LEN, rc; + unsigned short cscd_index = 0; unsigned short start = 0; - bool src = true; *sense_ret = TCM_INVALID_PARAMETER_LIST; @@ -206,25 +217,19 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, while (start < tdll) { /* - * Check target descriptor identification with 0xE4 type with - * use VPD 0x83 WWPN matching .. + * Check target descriptor identification with 0xE4 type, and + * compare the current index with the CSCD descriptor IDs in + * the segment descriptor. Use VPD 0x83 WWPN matching .. */ switch (desc[0]) { case 0xe4: rc = target_xcopy_parse_tiddesc_e4(se_cmd, xop, - &desc[0], src); + &desc[0], cscd_index); if (rc != 0) goto out; - /* - * Assume target descriptors are in source -> destination order.. - */ - if (src) - src = false; - else - src = true; start += XCOPY_TARGET_DESC_LEN; desc += XCOPY_TARGET_DESC_LEN; - ret++; + cscd_index++; break; default: pr_err("XCOPY unsupported descriptor type code:" @@ -234,12 +239,21 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, } } - if (xop->op_origin == XCOL_SOURCE_RECV_OP) + switch (xop->op_origin) { + case XCOL_SOURCE_RECV_OP: rc = target_xcopy_locate_se_dev_e4(xop->dst_tid_wwn, &xop->dst_dev); - else + break; + case XCOL_DEST_RECV_OP: rc = target_xcopy_locate_se_dev_e4(xop->src_tid_wwn, &xop->src_dev); + break; + default: + pr_err("XCOPY CSCD descriptor IDs not found in CSCD list - " + "stdi: %hu dtdi: %hu\n", xop->stdi, xop->dtdi); + rc = -EINVAL; + break; + } /* * If a matching IEEE NAA 0x83 descriptor for the requested device * is not located on this node, return COPY_ABORTED with ASQ/ASQC @@ -256,7 +270,7 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, pr_debug("XCOPY TGT desc: Dest dev: %p NAA IEEE WWN: 0x%16phN\n", xop->dst_dev, &xop->dst_tid_wwn[0]); - return ret; + return cscd_index; out: return -EINVAL; @@ -913,6 +927,20 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) " tdll: %hu sdll: %u inline_dl: %u\n", list_id, list_id_usage, tdll, sdll, inline_dl); + /* + * skip over the target descriptors until segment descriptors + * have been passed - CSCD ids are needed to determine src and dest. + */ + seg_desc = &p[16] + tdll; + + rc = target_xcopy_parse_segment_descriptors(se_cmd, xop, seg_desc, + sdll, &ret); + if (rc <= 0) + goto out; + + pr_debug("XCOPY: Processed %d segment descriptors, length: %u\n", rc, + rc * XCOPY_SEGMENT_DESC_LEN); + rc = target_xcopy_parse_target_descriptors(se_cmd, xop, &p[16], tdll, &ret); if (rc <= 0) goto out; @@ -930,19 +958,8 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) pr_debug("XCOPY: Processed %d target descriptors, length: %u\n", rc, rc * XCOPY_TARGET_DESC_LEN); - seg_desc = &p[16]; - seg_desc += (rc * XCOPY_TARGET_DESC_LEN); - - rc = target_xcopy_parse_segment_descriptors(se_cmd, xop, seg_desc, - sdll, &ret); - if (rc <= 0) { - xcopy_pt_undepend_remotedev(xop); - goto out; - } transport_kunmap_data_sg(se_cmd); - pr_debug("XCOPY: Processed %d segment descriptors, length: %u\n", rc, - rc * XCOPY_SEGMENT_DESC_LEN); INIT_WORK(&xop->xop_work, target_xcopy_do_work); queue_work(xcopy_wq, &xop->xop_work); return TCM_NO_SENSE; From f94fd098f674b78c29f482da1999d8de0c93c74e Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 2 Jan 2017 18:04:08 +0100 Subject: [PATCH 09/11] target: check for XCOPY parameter truncation Check for XCOPY header, CSCD descriptor and segment descriptor list truncation, and respond accordingly. SPC4r37 6.4.1 EXTENDED COPY(LID4) states (also applying to LID1 reqs): If the parameter list length causes truncation of the parameter list, then the copy manager shall transfer no data and shall terminate the EXTENDED COPY command with CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to PARAMETER LIST LENGTH ERROR. This behaviour can be tested using the libiscsi ExtendedCopy.ParamHdr test. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 14 ++++++++++++++ drivers/target/target_core_xcopy.h | 1 + 2 files changed, 15 insertions(+) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 2595c1eb9e91..a9a6462c66d1 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -888,6 +888,12 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) return TCM_UNSUPPORTED_SCSI_OPCODE; } + if (se_cmd->data_length < XCOPY_HDR_LEN) { + pr_err("XCOPY parameter truncation: length %u < hdr_len %u\n", + se_cmd->data_length, XCOPY_HDR_LEN); + return TCM_PARAMETER_LIST_LENGTH_ERROR; + } + xop = kzalloc(sizeof(struct xcopy_op), GFP_KERNEL); if (!xop) { pr_err("Unable to allocate xcopy_op\n"); @@ -923,6 +929,14 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) goto out; } + if (se_cmd->data_length < (XCOPY_HDR_LEN + tdll + sdll + inline_dl)) { + pr_err("XCOPY parameter truncation: data length %u too small " + "for tdll: %hu sdll: %u inline_dl: %u\n", + se_cmd->data_length, tdll, sdll, inline_dl); + ret = TCM_PARAMETER_LIST_LENGTH_ERROR; + goto out; + } + pr_debug("Processing XCOPY with list_id: 0x%02x list_id_usage: 0x%02x" " tdll: %hu sdll: %u inline_dl: %u\n", list_id, list_id_usage, tdll, sdll, inline_dl); diff --git a/drivers/target/target_core_xcopy.h b/drivers/target/target_core_xcopy.h index e2d141140342..7c0b105cbe1b 100644 --- a/drivers/target/target_core_xcopy.h +++ b/drivers/target/target_core_xcopy.h @@ -1,5 +1,6 @@ #include +#define XCOPY_HDR_LEN 16 #define XCOPY_TARGET_DESC_LEN 32 #define XCOPY_SEGMENT_DESC_LEN 28 #define XCOPY_NAA_IEEE_REGEX_LEN 16 From 87156518da94a696f2b27ab8945d531af2f1d339 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 2 Jan 2017 18:04:09 +0100 Subject: [PATCH 10/11] target: support XCOPY requests without parameters SPC4r37 6.4.1 EXTENDED COPY(LID4) states (also applying to LID1 reqs): A parameter list length of zero specifies that the copy manager shall not transfer any data or alter any internal state, and this shall not be considered an error. This behaviour can be tested using the libiscsi ExtendedCopy.ParamHdr test. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/target/target_core_xcopy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index a9a6462c66d1..d828b3b5000b 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -888,6 +888,10 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) return TCM_UNSUPPORTED_SCSI_OPCODE; } + if (se_cmd->data_length == 0) { + target_complete_cmd(se_cmd, SAM_STAT_GOOD); + return TCM_NO_SENSE; + } if (se_cmd->data_length < XCOPY_HDR_LEN) { pr_err("XCOPY parameter truncation: length %u < hdr_len %u\n", se_cmd->data_length, XCOPY_HDR_LEN); From 7c9d8d0c41b3e24473ac7648a7fc2d644ccf08ff Mon Sep 17 00:00:00 2001 From: "Bryant G. Ly" Date: Mon, 9 Jan 2017 10:21:20 -0600 Subject: [PATCH 11/11] ibmvscsis: Fix srp_transfer_data fail return code If srp_transfer_data fails within ibmvscsis_write_pending, then the most likely scenario is that the client timed out the op and removed the TCE mapping. Thus it will loop forever retrying the op that is pretty much guaranteed to fail forever. A better return code would be EIO instead of EAGAIN. Cc: stable@vger.kernel.org Reported-by: Steven Royer Tested-by: Steven Royer Signed-off-by: Bryant G. Ly Signed-off-by: Bart Van Assche --- drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c index 3d3768aaab4f..8fb5c54c7dd3 100644 --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c @@ -3585,7 +3585,7 @@ static int ibmvscsis_write_pending(struct se_cmd *se_cmd) 1, 1); if (rc) { pr_err("srp_transfer_data() failed: %d\n", rc); - return -EAGAIN; + return -EIO; } /* * We now tell TCM to add this WRITE CDB directly into the TCM storage