ACK: [SRU focal/linux-oem-5.6, bionic/linux-hwe, bionic/linux-hwe-5.0] scsi: target: Fix XCOPY NAA identifier lookup

Stefan Bader stefan.bader at canonical.com
Thu Jan 28 08:51:02 UTC 2021


On 27.01.21 18:58, Thadeu Lima de Souza Cascardo wrote:
> From: David Disseldorp <ddiss at suse.de>
> 
> When attempting to match EXTENDED COPY CSCD descriptors with corresponding
> se_devices, target_xcopy_locate_se_dev_e4() currently iterates over LIO's
> global devices list which includes all configured backstores.
> 
> This change ensures that only initiator-accessible backstores are
> considered during CSCD descriptor lookup, according to the session's
> se_node_acl LUN list.
> 
> To avoid LUN removal race conditions, device pinning is changed from being
> configfs based to instead using the se_node_acl lun_ref.
> 
> Reference: CVE-2020-28374
> Fixes: cbf031f425fd ("target: Add support for EXTENDED_COPY copy offload emulation")
> Reviewed-by: Lee Duncan <lduncan at suse.com>
> Signed-off-by: David Disseldorp <ddiss at suse.de>
> Signed-off-by: Mike Christie <michael.christie at oracle.com>
> Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
> (cherry picked from commit 2896c93811e39d63a4d9b63ccf12a8fbc226e5e4)
> CVE-2020-28374
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> --->  drivers/target/target_core_xcopy.c | 119 +++++++++++++++++------------
>  drivers/target/target_core_xcopy.h |   1 +
>  2 files changed, 71 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
> index 425c1070de08..4a95e9de7a8a 100644
> --- a/drivers/target/target_core_xcopy.c
> +++ b/drivers/target/target_core_xcopy.c
> @@ -46,60 +46,83 @@ static int target_xcopy_gen_naa_ieee(struct se_device *dev, unsigned char *buf)
>  	return 0;
>  }
>  
> -struct xcopy_dev_search_info {
> -	const unsigned char *dev_wwn;
> -	struct se_device *found_dev;
> -};
> -
> +/**
> + * target_xcopy_locate_se_dev_e4_iter - compare XCOPY NAA device identifiers
> + *
> + * @se_dev: device being considered for match
> + * @dev_wwn: XCOPY requested NAA dev_wwn
> + * @return: 1 on match, 0 on no-match
> + */
>  static int target_xcopy_locate_se_dev_e4_iter(struct se_device *se_dev,
> -					      void *data)
> +					      const unsigned char *dev_wwn)
>  {
> -	struct xcopy_dev_search_info *info = data;
>  	unsigned char tmp_dev_wwn[XCOPY_NAA_IEEE_REGEX_LEN];
>  	int rc;
>  
> -	if (!se_dev->dev_attrib.emulate_3pc)
> +	if (!se_dev->dev_attrib.emulate_3pc) {
> +		pr_debug("XCOPY: emulate_3pc disabled on se_dev %p\n", se_dev);
>  		return 0;
> +	}
>  
>  	memset(&tmp_dev_wwn[0], 0, XCOPY_NAA_IEEE_REGEX_LEN);
>  	target_xcopy_gen_naa_ieee(se_dev, &tmp_dev_wwn[0]);
>  
> -	rc = memcmp(&tmp_dev_wwn[0], info->dev_wwn, XCOPY_NAA_IEEE_REGEX_LEN);
> -	if (rc != 0)
> -		return 0;
> -
> -	info->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);
> +	rc = memcmp(&tmp_dev_wwn[0], dev_wwn, XCOPY_NAA_IEEE_REGEX_LEN);
>  	if (rc != 0) {
> -		pr_err("configfs_depend_item attempt failed: %d for se_dev: %p\n",
> -		       rc, se_dev);
> -		return rc;
> +		pr_debug("XCOPY: skip non-matching: %*ph\n",
> +			 XCOPY_NAA_IEEE_REGEX_LEN, tmp_dev_wwn);
> +		return 0;
>  	}
> +	pr_debug("XCOPY 0xe4: located se_dev: %p\n", 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;
>  }
>  
> -static int target_xcopy_locate_se_dev_e4(const unsigned char *dev_wwn,
> -					struct se_device **found_dev)
> +static int target_xcopy_locate_se_dev_e4(struct se_session *sess,
> +					const unsigned char *dev_wwn,
> +					struct se_device **_found_dev,
> +					struct percpu_ref **_found_lun_ref)
>  {
> -	struct xcopy_dev_search_info info;
> -	int ret;
> -
> -	memset(&info, 0, sizeof(info));
> -	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 {
> -		pr_debug_ratelimited("Unable to locate 0xe4 descriptor for EXTENDED_COPY\n");
> -		return -EINVAL;
> +	struct se_dev_entry *deve;
> +	struct se_node_acl *nacl;
> +	struct se_lun *this_lun = NULL;
> +	struct se_device *found_dev = NULL;
> +
> +	/* cmd with NULL sess indicates no associated $FABRIC_MOD */
> +	if (!sess)
> +		goto err_out;
> +
> +	pr_debug("XCOPY 0xe4: searching for: %*ph\n",
> +		 XCOPY_NAA_IEEE_REGEX_LEN, dev_wwn);
> +
> +	nacl = sess->se_node_acl;
> +	rcu_read_lock();
> +	hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) {
> +		struct se_device *this_dev;
> +		int rc;
> +
> +		this_lun = rcu_dereference(deve->se_lun);
> +		this_dev = rcu_dereference_raw(this_lun->lun_se_dev);
> +
> +		rc = target_xcopy_locate_se_dev_e4_iter(this_dev, dev_wwn);
> +		if (rc) {
> +			if (percpu_ref_tryget_live(&this_lun->lun_ref))
> +				found_dev = this_dev;
> +			break;
> +		}
>  	}
> +	rcu_read_unlock();
> +	if (found_dev == NULL)
> +		goto err_out;
> +
> +	pr_debug("lun_ref held for se_dev: %p se_dev->se_dev_group: %p\n",
> +		 found_dev, &found_dev->dev_group);
> +	*_found_dev = found_dev;
> +	*_found_lun_ref = &this_lun->lun_ref;
> +	return 0;
> +err_out:
> +	pr_debug_ratelimited("Unable to locate 0xe4 descriptor for EXTENDED_COPY\n");
> +	return -EINVAL;
>  }
>  
>  static int target_xcopy_parse_tiddesc_e4(struct se_cmd *se_cmd, struct xcopy_op *xop,
> @@ -246,12 +269,16 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd,
>  
>  	switch (xop->op_origin) {
>  	case XCOL_SOURCE_RECV_OP:
> -		rc = target_xcopy_locate_se_dev_e4(xop->dst_tid_wwn,
> -						&xop->dst_dev);
> +		rc = target_xcopy_locate_se_dev_e4(se_cmd->se_sess,
> +						xop->dst_tid_wwn,
> +						&xop->dst_dev,
> +						&xop->remote_lun_ref);
>  		break;
>  	case XCOL_DEST_RECV_OP:
> -		rc = target_xcopy_locate_se_dev_e4(xop->src_tid_wwn,
> -						&xop->src_dev);
> +		rc = target_xcopy_locate_se_dev_e4(se_cmd->se_sess,
> +						xop->src_tid_wwn,
> +						&xop->src_dev,
> +						&xop->remote_lun_ref);
>  		break;
>  	default:
>  		pr_err("XCOPY CSCD descriptor IDs not found in CSCD list - "
> @@ -396,18 +423,12 @@ static int xcopy_pt_get_cmd_state(struct se_cmd *se_cmd)
>  
>  static void xcopy_pt_undepend_remotedev(struct xcopy_op *xop)
>  {
> -	struct se_device *remote_dev;
> -
>  	if (xop->op_origin == XCOL_SOURCE_RECV_OP)
> -		remote_dev = xop->dst_dev;
> +		pr_debug("putting dst lun_ref for %p\n", xop->dst_dev);
>  	else
> -		remote_dev = xop->src_dev;
> -
> -	pr_debug("Calling configfs_undepend_item for"
> -		  " remote_dev: %p remote_dev->dev_group: %p\n",
> -		  remote_dev, &remote_dev->dev_group.cg_item);
> +		pr_debug("putting src lun_ref for %p\n", xop->src_dev);
>  
> -	target_undepend_item(&remote_dev->dev_group.cg_item);
> +	percpu_ref_put(xop->remote_lun_ref);
>  }
>  
>  static void xcopy_pt_release_cmd(struct se_cmd *se_cmd)
> diff --git a/drivers/target/target_core_xcopy.h b/drivers/target/target_core_xcopy.h
> index 26ba4c3c9cff..974bc1e19ff2 100644
> --- a/drivers/target/target_core_xcopy.h
> +++ b/drivers/target/target_core_xcopy.h
> @@ -29,6 +29,7 @@ struct xcopy_op {
>  	struct se_device *dst_dev;
>  	unsigned char dst_tid_wwn[XCOPY_NAA_IEEE_REGEX_LEN];
>  	unsigned char local_dev_wwn[XCOPY_NAA_IEEE_REGEX_LEN];
> +	struct percpu_ref *remote_lun_ref;
>  
>  	sector_t src_lba;
>  	sector_t dst_lba;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210128/e90cd86b/attachment.sig>


More information about the kernel-team mailing list