[RFC PATCH] scsi: target: detect XCOPY NAA descriptor conflicts

[David Disseldorp <ddiss@suse.de>]

LIO's XCOPY implementation currently only accepts IEEE NAA 0x83 type
device descriptors for copy source and destination IDs. These IDs are
automatically generated by spc_parse_naa_6h_vendor_specific() using
*only* hexadecimal characters present in the user-configured
vpd_unit_serial string, and advertised in the Device ID Page INQUIRY
response.

spc_parse_naa_6h_vendor_specific() mapping can quite easily result in
two devices with differing vpd_unit_serial strings sharing the same NAA
ID. E.g.
LUN0
-> backstore device=/dev/sda, vpd_unit_serial=unitserialfirst
LUN1
-> backstore device=/dev/sdb, vpd_unit_serial=unitserialforth

In this case, both LUNs would advertise an NAA ID of:
0x01405eaf0000000000000000...
Where 0x01405 corresponds to the OpenFabrics IEEE Company ID and 0xeaf
are hex characters taken from vpd_unit_serial.

With the above example, an initiator wishing to copy data from LUN0 to
LUN1 may issue an XCOPY request with a copy source and copy dest set
to 0x01405eaf... and observe that (despite XCOPY success), no data has
moved from LUN0 to LUN1. Instead LIO has processed the request using
LUN0 as source and destination.

This change sees LIO fail XCOPY requests if the copy source or
destination correspond to a non-unique NAA identifier.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 drivers/target/target_core_xcopy.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
index 44e15d7fb2f0..3ce5da4b3e81 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -68,8 +68,14 @@ static int target_xcopy_locate_se_dev_e4_iter(struct se_device *se_dev,
 	if (rc != 0)
 		return 0;
 
-	info->found_dev = se_dev;
 	pr_debug("XCOPY 0xe4: located se_dev: %p\n", se_dev);
+	if (info->found_dev) {
+		pr_warn("XCOPY 0xe4 descriptor conflict for se_dev %p and %p\n",
+			info->found_dev, se_dev);
+		target_undepend_item(&info->found_dev->dev_group.cg_item);
+		return -ENOTUNIQ;
+	}
+	info->found_dev = se_dev;
 
 	rc = target_depend_item(&se_dev->dev_group.cg_item);
 	if (rc != 0) {
@@ -80,7 +86,8 @@ static int target_xcopy_locate_se_dev_e4_iter(struct se_device *se_dev,
 
 	pr_debug("Called configfs_depend_item for se_dev: %p se_dev->se_dev_group: %p\n",
 		 se_dev, &se_dev->dev_group);
-	return 1;
+	/* continue iteration to check for conflicts */
+	return 0;
 }
 
 static int target_xcopy_locate_se_dev_e4(const unsigned char *dev_wwn,
@@ -93,13 +100,14 @@ static int target_xcopy_locate_se_dev_e4(const unsigned char *dev_wwn,
 	info.dev_wwn = dev_wwn;
 
 	ret = target_for_each_device(target_xcopy_locate_se_dev_e4_iter, &info);
-	if (ret = 1) {
-		*found_dev = info.found_dev;
-		return 0;
-	} else {
+	if (ret < 0) {
+		return ret;
+	} else if (!info.found_dev) {
 		pr_debug_ratelimited("Unable to locate 0xe4 descriptor for EXTENDED_COPY\n");
 		return -EINVAL;
 	}
+	*found_dev = info.found_dev;
+	return 0;
 }
 
 static int target_xcopy_parse_tiddesc_e4(struct se_cmd *se_cmd, struct xcopy_op *xop,
@@ -264,6 +272,9 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd,
 	 * is not located on this node, return COPY_ABORTED with ASQ/ASQC
 	 * 0x0d/0x02 - COPY_TARGET_DEVICE_NOT_REACHABLE to request the
 	 * initiator to fall back to normal copy method.
+	 * Fall back will also be requested if a IEEE NAA 0x83 descriptor
+	 * is not unique across all devices, which can occur due to suboptimal
+	 * vpd_unit_serial mapping in spc_parse_naa_6h_vendor_specific().
 	 */
 	if (rc < 0) {
 		*sense_ret = TCM_COPY_TARGET_DEVICE_NOT_REACHABLE;
-- 
2.26.2