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

Michael Christie
2020-09-02T03:18:06Z

> On Aug 12, 2020, at 7:21 PM, David Disseldorp <ddiss@suse.de> wrote:
>
> 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;

Was it valid to copy to/from the same LUN? You would copy from/to different src/destinations on that LUN. Would your patch break that?