From 2b16e96e6b63d0419d857f53e4cc67f0adb383fd Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 16 Nov 2020 12:25:32 +0100 Subject: [PATCH 1/4] tcmur: fail cross-device XCOPY requests tcmu-runner can't determine whether the device(s) referred to in XCOPY Copy Source/Copy Destination (CSCD) descriptors should be accessible to the initiator via transport settings, ACLs, etc. Consequently, fail XCOPY requests with CSCD descriptors which refer to any device other than where the XCOPY request is processed. References: CVE-2020-28374 Fixes: 9c86bd0 ("tcmur: Add emulate XCOPY command support") Signed-off-by: David Disseldorp Reviewed-by: Lee Duncan --- tcmur_cmd_handler.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tcmur_cmd_handler.c b/tcmur_cmd_handler.c index ca5e21cc..dfffc300 100644 --- a/tcmur_cmd_handler.c +++ b/tcmur_cmd_handler.c @@ -1349,6 +1349,18 @@ static int xcopy_parse_parameter_list(struct tcmu_device *dev, if (ret != TCMU_STS_OK) goto err; + /* + * tcmu-runner can't determine whether the device(s) referred to in an + * XCOPY request should be accessible to the initiator via transport + * settings, ACLs, etc. XXX Consequently, we need to fail any + * cross-device requests for safety reasons. + */ + if (dev != xcopy->src_dev || dev != xcopy->dst_dev) { + tcmu_dev_err(dev, "Cross-device XCOPY not supported\n"); + ret = TCMU_STS_CP_TGT_DEV_NOTCONN; + goto err; + } + if (tcmu_dev_get_block_size(xcopy->src_dev) != tcmu_dev_get_block_size(xcopy->dst_dev)) { tcmu_dev_err(dev, "The block size of src dev %u != dst dev %u\n", From b202dc06ef391c6ab9a7561856238a258de04663 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 13 Nov 2020 14:49:15 +0100 Subject: [PATCH 2/4] tcmur: fail XCOPY requests with inline data EXTENDED COPY inline data is currently unprocessed, so fail requests if the INLINE DATA LENGTH field indicates presence of any. Fixes: 9c86bd0 ("tcmur: Add emulate XCOPY command support") Signed-off-by: David Disseldorp --- tcmur_cmd_handler.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tcmur_cmd_handler.c b/tcmur_cmd_handler.c index dfffc300..db99e523 100644 --- a/tcmur_cmd_handler.c +++ b/tcmur_cmd_handler.c @@ -1308,6 +1308,12 @@ static int xcopy_parse_parameter_list(struct tcmu_device *dev, * data, after the last segment descriptor. * */ inline_dl = be32toh(*(uint32_t *)&par[12]); + if (inline_dl != 0) { + tcmu_dev_err(dev, "non-zero xcopy inline_dl %u unsupported\n", + inline_dl); + ret = TCMU_STS_INVALID_PARAM_LIST_LEN; + goto err; + } /* From spc4r31, section 6.3.1 EXTENDED COPY command introduction * From 170bfa63288a399b38c35eb646b2835d4ba7c08a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 13 Nov 2020 19:38:19 +0100 Subject: [PATCH 3/4] tcmur: don't assume two XCOPY CSCDs The XCOPY Copy Source / Copy Destination (CSCD) parse loop currently assumes that two descriptors are present. Remove this assumption and instead use the CSCD DESCRIPTOR LIST LENGTH in case only a single CSCD descriptor is carried, referred to by both segment descriptor CSCD descriptor IDs. Fixes: 9c86bd0 ("tcmur: Add emulate XCOPY command support") Signed-off-by: David Disseldorp --- tcmur_cmd_handler.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tcmur_cmd_handler.c b/tcmur_cmd_handler.c index db99e523..f914d9fa 100644 --- a/tcmur_cmd_handler.c +++ b/tcmur_cmd_handler.c @@ -1161,6 +1161,12 @@ static int xcopy_parse_target_descs(struct tcmu_device *udev, { int i, ret; + if (tdll % XCOPY_TARGET_DESC_LEN) { + tcmu_dev_err(udev, + "CSCD descriptor list length %u not a multiple of %u\n", + (unsigned int)tdll, XCOPY_TARGET_DESC_LEN); + return TCMU_STS_NOTSUPP_TGT_DESC_TYPE; + } /* From spc4r36q,section 6.4.3.4 CSCD DESCRIPTOR LIST LENGTH field * If the number of CSCD descriptors exceeds the allowed number, the copy * manager shall terminate the command with CHECK CONDITION status, with @@ -1173,7 +1179,7 @@ static int xcopy_parse_target_descs(struct tcmu_device *udev, return TCMU_STS_TOO_MANY_TGT_DESC; } - for (i = 0; i < RCR_OP_MAX_TARGET_DESC_COUNT; i++) { + for (i = 0; tdll >= XCOPY_TARGET_DESC_LEN; i++) { /* * Only Identification Descriptor Target Descriptor support * for now. @@ -1184,6 +1190,7 @@ static int xcopy_parse_target_descs(struct tcmu_device *udev, return ret; tgt_desc += XCOPY_TARGET_DESC_LEN; + tdll -= XCOPY_TARGET_DESC_LEN; } else { tcmu_dev_err(udev, "Unsupport target descriptor type code 0x%x\n", tgt_desc[0]); From 01685b2ab8c430c0fb9ce397e7e76b60fe6cbde5 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Sat, 14 Nov 2020 01:20:53 +0100 Subject: [PATCH 4/4] tcmur: error if both src/dst_dev are unset after CSCD parsing A null src_dev and dst_dev pointer appears to be quite possible after CSCD list parsing, so this error condition should be handled. Fixes: 9c86bd0 ("tcmur: Add emulate XCOPY command support") Signed-off-by: David Disseldorp --- tcmur_cmd_handler.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tcmur_cmd_handler.c b/tcmur_cmd_handler.c index f914d9fa..3a07ddac 100644 --- a/tcmur_cmd_handler.c +++ b/tcmur_cmd_handler.c @@ -1198,6 +1198,7 @@ static int xcopy_parse_target_descs(struct tcmu_device *udev, } } + ret = TCMU_STS_CP_TGT_DEV_NOTCONN; if (xcopy->src_dev) ret = xcopy_locate_udev(udev->ctx, xcopy->dst_tid_wwn, &xcopy->dst_dev);