From 0322913cab79e47282fa98910559cbf6f3660b52 Mon Sep 17 00:00:00 2001 From: Alan Adamson Date: Fri, 1 Mar 2019 14:44:20 -0800 Subject: scsi: target: Add device product id and revision configfs attributes The product_id and revision attributes will allow for the modification of the T10 Model and Revision strings returned in inquiry responses. Its value can be viewed and modified via the ConfigFS path at: target/core/$backstore/$name/wwn/product_id target/core/$backstore/$name/wwn/revision [mkp: dropped parentheses as requested by Bart] Signed-off-by: Alan Adamson Reviewed-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_configfs.c | 157 ++++++++++++++++++++++++++++++---- 1 file changed, 142 insertions(+), 15 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index fc5ef31f5ba8..8f3faef235b5 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1227,6 +1227,29 @@ static struct t10_wwn *to_t10_wwn(struct config_item *item) return container_of(to_config_group(item), struct t10_wwn, t10_wwn_group); } +static ssize_t target_check_inquiry_data(char *buf) +{ + size_t len; + int i; + + len = strlen(buf); + + /* + * SPC 4.3.1: + * ASCII data fields shall contain only ASCII printable characters + * (i.e., code values 20h to 7Eh) and may be terminated with one or + * more ASCII null (00h) characters. + */ + for (i = 0; i < len; i++) { + if (buf[i] < 0x20 || buf[i] > 0x7E) { + pr_err("Emulated T10 Inquiry Data contains non-ASCII-printable characters\n"); + return -EINVAL; + } + } + + return len; +} + /* * STANDARD and VPD page 0x83 T10 Vendor Identification */ @@ -1244,8 +1267,7 @@ static ssize_t target_wwn_vendor_id_store(struct config_item *item, /* +2 to allow for a trailing (stripped) '\n' and null-terminator */ unsigned char buf[INQUIRY_VENDOR_LEN + 2]; char *stripped = NULL; - size_t len; - int i; + size_t len, ret; len = strlcpy(buf, page, sizeof(buf)); if (len < sizeof(buf)) { @@ -1260,19 +1282,10 @@ static ssize_t target_wwn_vendor_id_store(struct config_item *item, return -EOVERFLOW; } - /* - * SPC 4.3.1: - * ASCII data fields shall contain only ASCII printable characters (i.e., - * code values 20h to 7Eh) and may be terminated with one or more ASCII - * null (00h) characters. - */ - for (i = 0; i < len; i++) { - if ((stripped[i] < 0x20) || (stripped[i] > 0x7E)) { - pr_err("Emulated T10 Vendor Identification contains" - " non-ASCII-printable characters\n"); - return -EINVAL; - } - } + ret = target_check_inquiry_data(stripped); + + if (ret < 0) + return ret; /* * Check to see if any active exports exist. If they do exist, fail @@ -1295,6 +1308,116 @@ static ssize_t target_wwn_vendor_id_store(struct config_item *item, return count; } +static ssize_t target_wwn_product_id_show(struct config_item *item, + char *page) +{ + return sprintf(page, "%s\n", &to_t10_wwn(item)->model[0]); +} + +static ssize_t target_wwn_product_id_store(struct config_item *item, + const char *page, size_t count) +{ + struct t10_wwn *t10_wwn = to_t10_wwn(item); + struct se_device *dev = t10_wwn->t10_dev; + /* +2 to allow for a trailing (stripped) '\n' and null-terminator */ + unsigned char buf[INQUIRY_MODEL_LEN + 2]; + char *stripped = NULL; + size_t len, ret; + + len = strlcpy(buf, page, sizeof(buf)); + if (len < sizeof(buf)) { + /* Strip any newline added from userspace. */ + stripped = strstrip(buf); + len = strlen(stripped); + } + if (len > INQUIRY_MODEL_LEN) { + pr_err("Emulated T10 Vendor exceeds INQUIRY_MODEL_LEN: " + __stringify(INQUIRY_MODEL_LEN) + "\n"); + return -EOVERFLOW; + } + + ret = target_check_inquiry_data(stripped); + + if (ret < 0) + return ret; + + /* + * Check to see if any active exports exist. If they do exist, fail + * here as changing this information on the fly (underneath the + * initiator side OS dependent multipath code) could cause negative + * effects. + */ + if (dev->export_count) { + pr_err("Unable to set T10 Model while active %d exports exist\n", + dev->export_count); + return -EINVAL; + } + + BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1); + strlcpy(dev->t10_wwn.model, stripped, sizeof(dev->t10_wwn.model)); + + pr_debug("Target_Core_ConfigFS: Set emulated T10 Model Identification: %s\n", + dev->t10_wwn.model); + + return count; +} + +static ssize_t target_wwn_revision_show(struct config_item *item, + char *page) +{ + return sprintf(page, "%s\n", &to_t10_wwn(item)->revision[0]); +} + +static ssize_t target_wwn_revision_store(struct config_item *item, + const char *page, size_t count) +{ + struct t10_wwn *t10_wwn = to_t10_wwn(item); + struct se_device *dev = t10_wwn->t10_dev; + /* +2 to allow for a trailing (stripped) '\n' and null-terminator */ + unsigned char buf[INQUIRY_REVISION_LEN + 2]; + char *stripped = NULL; + size_t len, ret; + + len = strlcpy(buf, page, sizeof(buf)); + if (len < sizeof(buf)) { + /* Strip any newline added from userspace. */ + stripped = strstrip(buf); + len = strlen(stripped); + } + if (len > INQUIRY_REVISION_LEN) { + pr_err("Emulated T10 Revision exceeds INQUIRY_REVISION_LEN: " + __stringify(INQUIRY_REVISION_LEN) + "\n"); + return -EOVERFLOW; + } + + ret = target_check_inquiry_data(stripped); + + if (ret < 0) + return ret; + + /* + * Check to see if any active exports exist. If they do exist, fail + * here as changing this information on the fly (underneath the + * initiator side OS dependent multipath code) could cause negative + * effects. + */ + if (dev->export_count) { + pr_err("Unable to set T10 Revision while active %d exports exist\n", + dev->export_count); + return -EINVAL; + } + + BUILD_BUG_ON(sizeof(dev->t10_wwn.revision) != INQUIRY_REVISION_LEN + 1); + strlcpy(dev->t10_wwn.revision, stripped, sizeof(dev->t10_wwn.revision)); + + pr_debug("Target_Core_ConfigFS: Set emulated T10 Revision: %s\n", + dev->t10_wwn.revision); + + return count; +} + /* * VPD page 0x80 Unit serial */ @@ -1442,6 +1565,8 @@ DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_target_port, 0x10); DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_scsi_target_device, 0x20); CONFIGFS_ATTR(target_wwn_, vendor_id); +CONFIGFS_ATTR(target_wwn_, product_id); +CONFIGFS_ATTR(target_wwn_, revision); CONFIGFS_ATTR(target_wwn_, vpd_unit_serial); CONFIGFS_ATTR_RO(target_wwn_, vpd_protocol_identifier); CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_logical_unit); @@ -1450,6 +1575,8 @@ CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_scsi_target_device); static struct configfs_attribute *target_core_dev_wwn_attrs[] = { &target_wwn_attr_vendor_id, + &target_wwn_attr_product_id, + &target_wwn_attr_revision, &target_wwn_attr_vpd_unit_serial, &target_wwn_attr_vpd_protocol_identifier, &target_wwn_attr_vpd_assoc_logical_unit, -- cgit v1.2.3-55-g7522 From ee26724af6c71a9db786c5809395b4853edd4d54 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 20 Mar 2019 16:37:09 +0000 Subject: scsi: target: fix unsigned comparision with less than zero Currently an error return is being assigned to an unsigned size_t varianle and then checked if the result is less than zero which will always be false. Fix this by making ret ssize_t rather than a size_t. Fixes: 0322913cab79 ("scsi: target: Add device product id and revision configfs attributes") Signed-off-by: Colin Ian King Reviewed-by: Mike Christie Signed-off-by: Martin K. Petersen --- drivers/target/target_core_configfs.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index 8f3faef235b5..3fe79875b3ac 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1267,7 +1267,8 @@ static ssize_t target_wwn_vendor_id_store(struct config_item *item, /* +2 to allow for a trailing (stripped) '\n' and null-terminator */ unsigned char buf[INQUIRY_VENDOR_LEN + 2]; char *stripped = NULL; - size_t len, ret; + size_t len; + ssize_t ret; len = strlcpy(buf, page, sizeof(buf)); if (len < sizeof(buf)) { @@ -1322,7 +1323,8 @@ static ssize_t target_wwn_product_id_store(struct config_item *item, /* +2 to allow for a trailing (stripped) '\n' and null-terminator */ unsigned char buf[INQUIRY_MODEL_LEN + 2]; char *stripped = NULL; - size_t len, ret; + size_t len; + ssize_t ret; len = strlcpy(buf, page, sizeof(buf)); if (len < sizeof(buf)) { @@ -1377,7 +1379,8 @@ static ssize_t target_wwn_revision_store(struct config_item *item, /* +2 to allow for a trailing (stripped) '\n' and null-terminator */ unsigned char buf[INQUIRY_REVISION_LEN + 2]; char *stripped = NULL; - size_t len, ret; + size_t len; + ssize_t ret; len = strlcpy(buf, page, sizeof(buf)); if (len < sizeof(buf)) { -- cgit v1.2.3-55-g7522 From 1ea9b4633cda461f09b4476ff612d32d78e81c92 Mon Sep 17 00:00:00 2001 From: tangwenji Date: Wed, 13 Mar 2019 22:56:38 +0800 Subject: scsi: target: iscsi: Fix np_ip_proto and np_sock_type in iscsit_setup_np In the switch, np_ip_proto and np_sock_type set different values according to np_network_transport, and then uniformly assign values, so the previous values are overwritten. Signed-off-by: tangwenji Reviewed-by: Mike Christie Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_login.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index ae3209efd0e0..c526e665f022 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -883,9 +883,6 @@ int iscsit_setup_np( return -EINVAL; } - np->np_ip_proto = IPPROTO_TCP; - np->np_sock_type = SOCK_STREAM; - ret = sock_create(sockaddr->ss_family, np->np_sock_type, np->np_ip_proto, &sock); if (ret < 0) { -- cgit v1.2.3-55-g7522 From f55d0b40eccfcfb50f93cc7d03c95e25ab19917e Mon Sep 17 00:00:00 2001 From: tangwenji Date: Wed, 20 Mar 2019 22:14:44 +0800 Subject: scsi: target: iscsi: Free conn_ops when zalloc_cpumask_var failed It should not free cpumask but free conn->conn_ops when zalloc_cpumask_var failed. Signed-off-by: tangwenji Reviewed-by: Mike Christie Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_login.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index c526e665f022..683d04580eb3 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -1156,13 +1156,13 @@ static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np) if (!zalloc_cpumask_var(&conn->conn_cpumask, GFP_KERNEL)) { pr_err("Unable to allocate conn->conn_cpumask\n"); - goto free_mask; + goto free_conn_ops; } return conn; -free_mask: - free_cpumask_var(conn->conn_cpumask); +free_conn_ops: + kfree(conn->conn_ops); put_transport: iscsit_put_transport(conn->conn_transport); free_conn: -- cgit v1.2.3-55-g7522 From 82129697df9d90afd22736acd89d24f98bde2522 Mon Sep 17 00:00:00 2001 From: tangwenji Date: Wed, 27 Mar 2019 21:59:06 +0800 Subject: scsi: target: alua: fix the tg_pt_gps_count Reducing the count should be alua_tg_pt_gps_count instead of alua_tg_pt_gps_counter when free alua group. Signed-off-by: tangwenji Reviewed-by: Mike Christie Signed-off-by: Martin K. Petersen --- drivers/target/target_core_alua.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index e09f0cf86bed..893f1fe8e373 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -1760,8 +1760,10 @@ void core_alua_free_tg_pt_gp( * can be made while we are releasing struct t10_alua_tg_pt_gp. */ spin_lock(&dev->t10_alua.tg_pt_gps_lock); - list_del(&tg_pt_gp->tg_pt_gp_list); - dev->t10_alua.alua_tg_pt_gps_counter--; + if (tg_pt_gp->tg_pt_gp_valid_id) { + list_del(&tg_pt_gp->tg_pt_gp_list); + dev->t10_alua.alua_tg_pt_gps_count--; + } spin_unlock(&dev->t10_alua.tg_pt_gps_lock); /* -- cgit v1.2.3-55-g7522 From 63f7479439c95bcd49b7dd4af809862c316c71a3 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:05 -0700 Subject: scsi: target/core: Fix a race condition in the LUN lookup code The rcu_dereference(deve->se_lun) expression occurs twice in the LUN lookup functions. Since these expressions are not serialized against deve->se_lun assignments each of these expressions may yield a different result. Avoid that the wrong LUN pointer is stored in se_cmd by reading deve->se_lun only once. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Fixes: 29a05deebf6c ("target: Convert se_node_acl->device_list[] to RCU hlist") # v4.10 Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 1f8482b6473b..7eae1c823c4e 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -85,7 +85,7 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd, u64 unpacked_lun) goto out_unlock; } - se_cmd->se_lun = rcu_dereference(deve->se_lun); + se_cmd->se_lun = se_lun; se_cmd->pr_res_key = deve->pr_res_key; se_cmd->orig_fe_lun = unpacked_lun; se_cmd->se_cmd_flags |= SCF_SE_LUN_CMD; @@ -176,7 +176,7 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u64 unpacked_lun) goto out_unlock; } - se_cmd->se_lun = rcu_dereference(deve->se_lun); + se_cmd->se_lun = se_lun; se_cmd->pr_res_key = deve->pr_res_key; se_cmd->orig_fe_lun = unpacked_lun; se_cmd->se_cmd_flags |= SCF_SE_LUN_CMD; -- cgit v1.2.3-55-g7522 From fae43461f8f227a83f8edc3b15325188b56aa023 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:06 -0700 Subject: scsi: target/core: Rework the SPC-2 reservation handling code Instead of tracking the initiator that established an SPC-2 reservation, track the session through which the SPC-2 reservation has been established. This patch does not change any functionality. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_configfs.c | 5 +++-- drivers/target/target_core_pr.c | 33 +++++++++++++++++++-------------- drivers/target/target_core_pr.h | 1 + drivers/target/target_core_tmr.c | 2 +- include/target/target_core_base.h | 4 ++-- include/target/target_core_fabric.h | 1 + 6 files changed, 27 insertions(+), 19 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index 3fe79875b3ac..db2558fe8d46 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1624,11 +1624,12 @@ static ssize_t target_core_dev_pr_show_spc3_res(struct se_device *dev, static ssize_t target_core_dev_pr_show_spc2_res(struct se_device *dev, char *page) { + struct se_session *sess = dev->reservation_holder; struct se_node_acl *se_nacl; ssize_t len; - se_nacl = dev->dev_reserved_node_acl; - if (se_nacl) { + if (sess) { + se_nacl = sess->se_node_acl; len = sprintf(page, "SPC-2 Reservation: %s Initiator: %s\n", se_nacl->se_tpg->se_tpg_tfo->fabric_name, diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 1597a9ebadca..03767693f580 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -111,10 +111,10 @@ target_scsi2_reservation_check(struct se_cmd *cmd) break; } - if (!dev->dev_reserved_node_acl || !sess) + if (!dev->reservation_holder || !sess) return 0; - if (dev->dev_reserved_node_acl != sess->se_node_acl) + if (dev->reservation_holder->se_node_acl != sess->se_node_acl) return TCM_RESERVATION_CONFLICT; if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS_WITH_ISID) { @@ -200,6 +200,16 @@ static int target_check_scsi2_reservation_conflict(struct se_cmd *cmd) return 0; } +void target_release_reservation(struct se_device *dev) +{ + dev->reservation_holder = NULL; + dev->dev_reservation_flags &= ~DRF_SPC2_RESERVATIONS; + if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS_WITH_ISID) { + dev->dev_res_bin_isid = 0; + dev->dev_reservation_flags &= ~DRF_SPC2_RESERVATIONS_WITH_ISID; + } +} + sense_reason_t target_scsi2_reservation_release(struct se_cmd *cmd) { @@ -217,21 +227,16 @@ target_scsi2_reservation_release(struct se_cmd *cmd) return TCM_RESERVATION_CONFLICT; spin_lock(&dev->dev_reservation_lock); - if (!dev->dev_reserved_node_acl || !sess) + if (!dev->reservation_holder || !sess) goto out_unlock; - if (dev->dev_reserved_node_acl != sess->se_node_acl) + if (dev->reservation_holder->se_node_acl != sess->se_node_acl) goto out_unlock; if (dev->dev_res_bin_isid != sess->sess_bin_isid) goto out_unlock; - dev->dev_reserved_node_acl = NULL; - dev->dev_reservation_flags &= ~DRF_SPC2_RESERVATIONS; - if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS_WITH_ISID) { - dev->dev_res_bin_isid = 0; - dev->dev_reservation_flags &= ~DRF_SPC2_RESERVATIONS_WITH_ISID; - } + target_release_reservation(dev); tpg = sess->se_tpg; pr_debug("SCSI-2 Released reservation for %s LUN: %llu ->" " MAPPED LUN: %llu for %s\n", @@ -275,13 +280,13 @@ target_scsi2_reservation_reserve(struct se_cmd *cmd) tpg = sess->se_tpg; spin_lock(&dev->dev_reservation_lock); - if (dev->dev_reserved_node_acl && - (dev->dev_reserved_node_acl != sess->se_node_acl)) { + if (dev->reservation_holder && + dev->reservation_holder->se_node_acl != sess->se_node_acl) { pr_err("SCSI-2 RESERVATION CONFLIFT for %s fabric\n", tpg->se_tpg_tfo->fabric_name); pr_err("Original reserver LUN: %llu %s\n", cmd->se_lun->unpacked_lun, - dev->dev_reserved_node_acl->initiatorname); + dev->reservation_holder->se_node_acl->initiatorname); pr_err("Current attempt - LUN: %llu -> MAPPED LUN: %llu" " from %s \n", cmd->se_lun->unpacked_lun, cmd->orig_fe_lun, @@ -290,7 +295,7 @@ target_scsi2_reservation_reserve(struct se_cmd *cmd) goto out_unlock; } - dev->dev_reserved_node_acl = sess->se_node_acl; + dev->reservation_holder = sess; dev->dev_reservation_flags |= DRF_SPC2_RESERVATIONS; if (sess->sess_bin_isid != 0) { dev->dev_res_bin_isid = sess->sess_bin_isid; diff --git a/drivers/target/target_core_pr.h b/drivers/target/target_core_pr.h index 198fad5c89dc..a31c93e4e19c 100644 --- a/drivers/target/target_core_pr.h +++ b/drivers/target/target_core_pr.h @@ -58,6 +58,7 @@ extern struct kmem_cache *t10_pr_reg_cache; extern void core_pr_dump_initiator_port(struct t10_pr_registration *, char *, u32); +extern void target_release_reservation(struct se_device *dev); extern sense_reason_t target_scsi2_reservation_release(struct se_cmd *); extern sense_reason_t target_scsi2_reservation_reserve(struct se_cmd *); extern int core_scsi3_alloc_aptpl_registration( diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 3a1bb799a9ab..344df737f3a3 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -390,7 +390,7 @@ int core_tmr_lun_reset( if (!preempt_and_abort_list && (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)) { spin_lock(&dev->dev_reservation_lock); - dev->dev_reserved_node_acl = NULL; + dev->reservation_holder = NULL; dev->dev_reservation_flags &= ~DRF_SPC2_RESERVATIONS; spin_unlock(&dev->dev_reservation_lock); pr_debug("LUN_RESET: SCSI-2 Released reservation\n"); diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 19a5bf4214fc..7c9716fe731e 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -795,8 +795,8 @@ struct se_device { spinlock_t se_tmr_lock; spinlock_t qf_cmd_lock; struct semaphore caw_sem; - /* Used for legacy SPC-2 reservationsa */ - struct se_node_acl *dev_reserved_node_acl; + /* Used for legacy SPC-2 reservations */ + struct se_session *reservation_holder; /* Used for ALUA Logical Unit Group membership */ struct t10_alua_lu_gp_member *dev_alua_lu_gp_mem; /* Used for SPC-3 Persistent Reservations */ diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h index 8ed90407f062..063f133e47c2 100644 --- a/include/target/target_core_fabric.h +++ b/include/target/target_core_fabric.h @@ -142,6 +142,7 @@ void transport_register_session(struct se_portal_group *, struct se_node_acl *, struct se_session *, void *); ssize_t target_show_dynamic_sessions(struct se_portal_group *, char *); void transport_free_session(struct se_session *); +void target_spc2_release(struct se_node_acl *nacl); void target_put_nacl(struct se_node_acl *); void transport_deregister_session_configfs(struct se_session *); void transport_deregister_session(struct se_session *); -- cgit v1.2.3-55-g7522 From 82b76b4476e3a8938ad84f8ad1c2d6fc44af0522 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:08 -0700 Subject: scsi: target/core: Remove a set-but-not-used member variable from the XCOPY implementation This patch does not change any functionality. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_xcopy.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 9be1418e919f..d97be766e4ac 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -389,7 +389,6 @@ out: */ struct xcopy_pt_cmd { - bool remote_port; struct se_cmd se_cmd; struct completion xpt_passthrough_sem; unsigned char sense_buffer[TRANSPORT_SENSE_BUFFER]; @@ -520,9 +519,7 @@ static void target_xcopy_setup_pt_port( * when CDB is received on local source port, and READs blocks to * WRITE on remote destination port. */ - if (remote_port) { - xpt_cmd->remote_port = remote_port; - } else { + if (!remote_port) { pt_cmd->se_lun = ec_cmd->se_lun; pt_cmd->se_dev = ec_cmd->se_dev; @@ -539,9 +536,7 @@ static void target_xcopy_setup_pt_port( * blocks from the remote source port to WRITE on local * destination port. */ - if (remote_port) { - xpt_cmd->remote_port = remote_port; - } else { + if (!remote_port) { pt_cmd->se_lun = ec_cmd->se_lun; pt_cmd->se_dev = ec_cmd->se_dev; -- cgit v1.2.3-55-g7522 From 0f57cf5ce7661c9803064d8d1b4e7f3814e84649 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:09 -0700 Subject: scsi: target/core: Simplify LUN initialization in XCOPY implementation Other than removing a few pr_debug() statements, this patch does not change any functionality. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_xcopy.c | 43 +++----------------------------------- 1 file changed, 3 insertions(+), 40 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index d97be766e4ac..39f4490a3ebe 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -513,53 +513,16 @@ static void target_xcopy_setup_pt_port( struct se_cmd *ec_cmd = xop->xop_se_cmd; struct se_cmd *pt_cmd = &xpt_cmd->se_cmd; - if (xop->op_origin == XCOL_SOURCE_RECV_OP) { - /* - * Honor destination port reservations for X-COPY PUSH emulation - * when CDB is received on local source port, and READs blocks to - * WRITE on remote destination port. - */ - if (!remote_port) { - pt_cmd->se_lun = ec_cmd->se_lun; - pt_cmd->se_dev = ec_cmd->se_dev; - - pr_debug("Honoring local SRC port from ec_cmd->se_dev:" - " %p\n", pt_cmd->se_dev); - pt_cmd->se_lun = ec_cmd->se_lun; - pr_debug("Honoring local SRC port from ec_cmd->se_lun: %p\n", - pt_cmd->se_lun); - } - } else { - /* - * Honor source port reservation for X-COPY PULL emulation - * when CDB is received on local desintation port, and READs - * blocks from the remote source port to WRITE on local - * destination port. - */ - if (!remote_port) { - pt_cmd->se_lun = ec_cmd->se_lun; - pt_cmd->se_dev = ec_cmd->se_dev; - - pr_debug("Honoring local DST port from ec_cmd->se_dev:" - " %p\n", pt_cmd->se_dev); - pt_cmd->se_lun = ec_cmd->se_lun; - pr_debug("Honoring local DST port from ec_cmd->se_lun: %p\n", - pt_cmd->se_lun); - } + if (!remote_port) { + pt_cmd->se_lun = ec_cmd->se_lun; + pt_cmd->se_dev = ec_cmd->se_dev; } } static void target_xcopy_init_pt_lun(struct se_device *se_dev, struct se_cmd *pt_cmd, bool remote_port) { - /* - * Don't allocate + init an pt_cmd->se_lun if honoring local port for - * reservations. The pt_cmd->se_lun pointer will be setup from within - * target_xcopy_setup_pt_port() - */ if (remote_port) { - pr_debug("Setup emulated se_dev: %p from se_dev\n", - pt_cmd->se_dev); pt_cmd->se_lun = &se_dev->xcopy_lun; pt_cmd->se_dev = se_dev; } -- cgit v1.2.3-55-g7522 From be71530aa9742afb02714ae7eab4d3c5ad107590 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:10 -0700 Subject: scsi: target/core: Make the XCOPY setup code easier to read by inlining two functions The target_xcopy_setup_pt_port() and target_xcopy_init_pt_lun() functions obfuscate what is really going on. Hence inline these two functions. This patch does not change any functionality. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_xcopy.c | 50 ++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 27 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 39f4490a3ebe..e59a896ac58a 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -505,31 +505,20 @@ void target_xcopy_release_pt(void) destroy_workqueue(xcopy_wq); } -static void target_xcopy_setup_pt_port( - struct xcopy_pt_cmd *xpt_cmd, - struct xcopy_op *xop, - bool remote_port) -{ - struct se_cmd *ec_cmd = xop->xop_se_cmd; - struct se_cmd *pt_cmd = &xpt_cmd->se_cmd; - - if (!remote_port) { - pt_cmd->se_lun = ec_cmd->se_lun; - pt_cmd->se_dev = ec_cmd->se_dev; - } -} - -static void target_xcopy_init_pt_lun(struct se_device *se_dev, - struct se_cmd *pt_cmd, bool remote_port) -{ - if (remote_port) { - pt_cmd->se_lun = &se_dev->xcopy_lun; - pt_cmd->se_dev = se_dev; - } - - pt_cmd->se_cmd_flags |= SCF_SE_LUN_CMD; -} - +/* + * target_xcopy_setup_pt_cmd - set up a pass-through command + * @xpt_cmd: Data structure to initialize. + * @xop: Describes the XCOPY operation received from an initiator. + * @se_dev: Backend device to associate with @xpt_cmd if + * @remote_port == true. + * @cdb: SCSI CDB to be copied into @xpt_cmd. + * @remote_port: If false, use the LUN through which the XCOPY command has + * been received. If true, use @se_dev->xcopy_lun. + * @alloc_mem: Whether or not to allocate an SGL list. + * + * Set up a SCSI command (READ or WRITE) that will be used to execute an + * XCOPY command. + */ static int target_xcopy_setup_pt_cmd( struct xcopy_pt_cmd *xpt_cmd, struct xcopy_op *xop, @@ -541,12 +530,19 @@ static int target_xcopy_setup_pt_cmd( struct se_cmd *cmd = &xpt_cmd->se_cmd; sense_reason_t sense_rc; int ret = 0, rc; + /* * Setup LUN+port to honor reservations based upon xop->op_origin for * X-COPY PUSH or X-COPY PULL based upon where the CDB was received. */ - target_xcopy_init_pt_lun(se_dev, cmd, remote_port); - target_xcopy_setup_pt_port(xpt_cmd, xop, remote_port); + if (remote_port) { + cmd->se_lun = &se_dev->xcopy_lun; + cmd->se_dev = se_dev; + } else { + cmd->se_lun = xop->xop_se_cmd->se_lun; + cmd->se_dev = xop->xop_se_cmd->se_dev; + } + cmd->se_cmd_flags |= SCF_SE_LUN_CMD; cmd->tag = 0; sense_rc = target_setup_cmd_from_cdb(cmd, cdb); -- cgit v1.2.3-55-g7522 From b0055acaedf56a2717a6e2a4b700f1959a1b60df Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:11 -0700 Subject: scsi: target/iscsi: Detect conn_cmd_list corruption early Certain behavior of an initiator can cause the target driver to send both a reject and a SCSI response. If that happens two target_put_sess_cmd() calls will occur without the command having been removed from conn_cmd_list. In other words, conn_cmd_list will get corrupted once the freed memory is reused. Although the Linux kernel can detect list corruption if list debugging is enabled, in this case the context in which list corruption is detected is not related to the context that caused list corruption. Hence add WARN_ON() statements that report the context that is causing list corruption. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_util.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 3ac494f63a0b..3da062ccd2ab 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -769,6 +769,8 @@ void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown) struct se_cmd *se_cmd = cmd->se_cmd.se_tfo ? &cmd->se_cmd : NULL; int rc; + WARN_ON(!list_empty(&cmd->i_conn_node)); + __iscsit_free_cmd(cmd, shutdown); if (se_cmd) { rc = transport_generic_free_cmd(se_cmd, shutdown); -- cgit v1.2.3-55-g7522 From 96e8e26dd8dd8a60ef1d0dc3ef0d952ffa70a39f Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:12 -0700 Subject: scsi: target/iscsi: Only send R2T if needed If an initiator submits more immediate data than the size derived from the SCSI CDB, do not send any R2T to the initiator. This scenario is triggered by the libiscsi test ALL.iSCSIResiduals.WriteVerify16Residuals if the iSCSI target driver is modified to discard too large immediate data buffers instead of trying to parse these as an iSCSI PDU. This patch avoids that a negative xfer_len value is passed to iscsit_add_r2t_to_list() if too large immediate data buffers are handled correctly. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 6 ++++++ drivers/target/iscsi/iscsi_target_util.c | 2 ++ 2 files changed, 8 insertions(+) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 5ce6e2a40e00..828697015759 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -3121,6 +3121,12 @@ int iscsit_build_r2ts_for_cmd( else xfer_len = conn->sess->sess_ops->MaxBurstLength; } + + if ((s32)xfer_len < 0) { + cmd->cmd_flags |= ICF_SENT_LAST_R2T; + break; + } + cmd->r2t_offset += xfer_len; if (cmd->r2t_offset == cmd->se_cmd.data_length) diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 3da062ccd2ab..5b26bc23016a 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -67,6 +67,8 @@ int iscsit_add_r2t_to_list( lockdep_assert_held(&cmd->r2t_lock); + WARN_ON_ONCE((s32)xfer_len < 0); + r2t = kmem_cache_zalloc(lio_r2t_cache, GFP_ATOMIC); if (!r2t) { pr_err("Unable to allocate memory for struct iscsi_r2t.\n"); -- cgit v1.2.3-55-g7522 From 0ca650c13ba2f53cd3592d1a1d054adcd4164ca4 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:13 -0700 Subject: scsi: target/iscsi: Handle too large immediate data buffers correctly Since target_alloc_sgl() and iscsit_allocate_iovecs() allocate buffer space for se_cmd.data_length bytes and since that number can be smaller than the iSCSI Expected Data Transfer Length (EDTL), ensure that the iSCSI target driver does not attempt to receive more bytes than what fits in the receive buffer. Always receive the full immediate data buffer such that the iSCSI target driver does not attempt to parse immediate data as an iSCSI PDU. Note: the current code base only calls iscsit_get_dataout() if the size of the immediate data buffer does not exceed the buffer size derived from the SCSI CDB. See also target_cmd_size_check(). Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 27 ++++++++++++++++++++++++--- drivers/target/iscsi/iscsi_target_util.c | 1 + include/target/iscsi/iscsi_target_core.h | 1 + 3 files changed, 26 insertions(+), 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 828697015759..8cdea25f1377 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -1568,9 +1568,11 @@ iscsit_get_dataout(struct iscsi_conn *conn, struct iscsi_cmd *cmd, { struct kvec *iov; u32 checksum, iov_count = 0, padding = 0, rx_got = 0, rx_size = 0; - u32 payload_length = ntoh24(hdr->dlength); + u32 payload_length; int iov_ret, data_crc_failed = 0; + payload_length = min_t(u32, cmd->se_cmd.data_length, + ntoh24(hdr->dlength)); rx_size += payload_length; iov = &cmd->iov_data[0]; @@ -2575,14 +2577,33 @@ static int iscsit_handle_immediate_data( u32 checksum, iov_count = 0, padding = 0; struct iscsi_conn *conn = cmd->conn; struct kvec *iov; + void *overflow_buf = NULL; - iov_ret = iscsit_map_iovec(cmd, cmd->iov_data, cmd->write_data_done, length); + BUG_ON(cmd->write_data_done > cmd->se_cmd.data_length); + rx_size = min(cmd->se_cmd.data_length - cmd->write_data_done, length); + iov_ret = iscsit_map_iovec(cmd, cmd->iov_data, cmd->write_data_done, + rx_size); if (iov_ret < 0) return IMMEDIATE_DATA_CANNOT_RECOVER; - rx_size = length; iov_count = iov_ret; iov = &cmd->iov_data[0]; + if (rx_size < length) { + /* + * Special case: length of immediate data exceeds the data + * buffer size derived from the CDB. + */ + overflow_buf = kmalloc(length - rx_size, GFP_KERNEL); + if (!overflow_buf) { + iscsit_unmap_iovec(cmd); + return IMMEDIATE_DATA_CANNOT_RECOVER; + } + cmd->overflow_buf = overflow_buf; + iov[iov_count].iov_base = overflow_buf; + iov[iov_count].iov_len = length - rx_size; + iov_count++; + rx_size = length; + } padding = ((-length) & 3); if (padding != 0) { diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 5b26bc23016a..fae85bfd790e 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -737,6 +737,7 @@ void iscsit_release_cmd(struct iscsi_cmd *cmd) kfree(cmd->pdu_list); kfree(cmd->seq_list); kfree(cmd->tmr_req); + kfree(cmd->overflow_buf); kfree(cmd->iov_data); kfree(cmd->text_in_ptr); diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h index 24c398f4a68f..a49d37140a64 100644 --- a/include/target/iscsi/iscsi_target_core.h +++ b/include/target/iscsi/iscsi_target_core.h @@ -473,6 +473,7 @@ struct iscsi_cmd { struct timer_list dataout_timer; /* Iovecs for SCSI data payload RX/TX w/ kernel level sockets */ struct kvec *iov_data; + void *overflow_buf; /* Iovecs for miscellaneous purposes */ #define ISCSI_MISC_IOVECS 5 struct kvec iov_misc[ISCSI_MISC_IOVECS]; -- cgit v1.2.3-55-g7522 From 2e39f1c9064d54e51e939ce5f3cf34527ef5603f Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:14 -0700 Subject: scsi: target/iscsi: Make iscsit_map_iovec() more robust Make the code for mapping an iovec more robust by checking the bounds of the allocated iovec. This patch avoids that the following crash occurs if a map attempt is made that exceeds the bounds of the iovec that is being mapped: BUG: unable to handle kernel NULL pointer dereference at 00000000 00000014 RIP: 0010:iscsit_map_iovec+0x120/0x190 [iscsi_target_mod] Call Trace: iscsit_get_rx_pdu+0x8a2/0xe00 [iscsi_target_mod] iscsi_target_rx_thread+0x6e/0xa0 [iscsi_target_mod] kthread+0x109/0x140 Cc: Mike Christie Cc: Hannes Reinecke Cc: Christoph Hellwig Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 50 ++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 14 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 8cdea25f1377..f01cdae54277 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -573,7 +573,8 @@ iscsit_xmit_nondatain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd, return 0; } -static int iscsit_map_iovec(struct iscsi_cmd *, struct kvec *, u32, u32); +static int iscsit_map_iovec(struct iscsi_cmd *cmd, struct kvec *iov, int nvec, + u32 data_offset, u32 data_length); static void iscsit_unmap_iovec(struct iscsi_cmd *); static u32 iscsit_do_crypto_hash_sg(struct ahash_request *, struct iscsi_cmd *, u32, u32, u32, u8 *); @@ -604,7 +605,8 @@ iscsit_xmit_datain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd, *header_digest); } - iov_ret = iscsit_map_iovec(cmd, &cmd->iov_data[1], + iov_ret = iscsit_map_iovec(cmd, &cmd->iov_data[iov_count], + cmd->orig_iov_data_count - (iov_count + 2), datain->offset, datain->length); if (iov_ret < 0) return -1; @@ -886,13 +888,10 @@ EXPORT_SYMBOL(iscsit_reject_cmd); * Map some portion of the allocated scatterlist to an iovec, suitable for * kernel sockets to copy data in/out. */ -static int iscsit_map_iovec( - struct iscsi_cmd *cmd, - struct kvec *iov, - u32 data_offset, - u32 data_length) +static int iscsit_map_iovec(struct iscsi_cmd *cmd, struct kvec *iov, int nvec, + u32 data_offset, u32 data_length) { - u32 i = 0; + u32 i = 0, orig_data_length = data_length; struct scatterlist *sg; unsigned int page_off; @@ -901,9 +900,12 @@ static int iscsit_map_iovec( */ u32 ent = data_offset / PAGE_SIZE; + if (!data_length) + return 0; + if (ent >= cmd->se_cmd.t_data_nents) { pr_err("Initial page entry out-of-bounds\n"); - return -1; + goto overflow; } sg = &cmd->se_cmd.t_data_sg[ent]; @@ -913,7 +915,12 @@ static int iscsit_map_iovec( cmd->first_data_sg_off = page_off; while (data_length) { - u32 cur_len = min_t(u32, data_length, sg->length - page_off); + u32 cur_len; + + if (WARN_ON_ONCE(!sg || i >= nvec)) + goto overflow; + + cur_len = min_t(u32, data_length, sg->length - page_off); iov[i].iov_base = kmap(sg_page(sg)) + sg->offset + page_off; iov[i].iov_len = cur_len; @@ -927,6 +934,16 @@ static int iscsit_map_iovec( cmd->kmapped_nents = i; return i; + +overflow: + pr_err("offset %d + length %d overflow; %d/%d; sg-list:\n", + data_offset, orig_data_length, i, nvec); + for_each_sg(cmd->se_cmd.t_data_sg, sg, + cmd->se_cmd.t_data_nents, i) { + pr_err("[%d] off %d len %d\n", + i, sg->offset, sg->length); + } + return -1; } static void iscsit_unmap_iovec(struct iscsi_cmd *cmd) @@ -1576,8 +1593,8 @@ iscsit_get_dataout(struct iscsi_conn *conn, struct iscsi_cmd *cmd, rx_size += payload_length; iov = &cmd->iov_data[0]; - iov_ret = iscsit_map_iovec(cmd, iov, be32_to_cpu(hdr->offset), - payload_length); + iov_ret = iscsit_map_iovec(cmd, iov, cmd->orig_iov_data_count - 2, + be32_to_cpu(hdr->offset), payload_length); if (iov_ret < 0) return -1; @@ -1597,6 +1614,7 @@ iscsit_get_dataout(struct iscsi_conn *conn, struct iscsi_cmd *cmd, rx_size += ISCSI_CRC_LEN; } + WARN_ON_ONCE(iov_count > cmd->orig_iov_data_count); rx_got = rx_data(conn, &cmd->iov_data[0], iov_count, rx_size); iscsit_unmap_iovec(cmd); @@ -1862,6 +1880,7 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd, rx_size += ISCSI_CRC_LEN; } + WARN_ON_ONCE(niov > ARRAY_SIZE(cmd->iov_misc)); rx_got = rx_data(conn, &cmd->iov_misc[0], niov, rx_size); if (rx_got != rx_size) { ret = -1; @@ -2267,6 +2286,7 @@ iscsit_handle_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, rx_size += ISCSI_CRC_LEN; } + WARN_ON_ONCE(niov > ARRAY_SIZE(iov)); rx_got = rx_data(conn, &iov[0], niov, rx_size); if (rx_got != rx_size) goto reject; @@ -2581,8 +2601,9 @@ static int iscsit_handle_immediate_data( BUG_ON(cmd->write_data_done > cmd->se_cmd.data_length); rx_size = min(cmd->se_cmd.data_length - cmd->write_data_done, length); - iov_ret = iscsit_map_iovec(cmd, cmd->iov_data, cmd->write_data_done, - rx_size); + iov_ret = iscsit_map_iovec(cmd, cmd->iov_data, + cmd->orig_iov_data_count - 2, + cmd->write_data_done, rx_size); if (iov_ret < 0) return IMMEDIATE_DATA_CANNOT_RECOVER; @@ -2618,6 +2639,7 @@ static int iscsit_handle_immediate_data( rx_size += ISCSI_CRC_LEN; } + WARN_ON_ONCE(iov_count > cmd->orig_iov_data_count); rx_got = rx_data(conn, &cmd->iov_data[0], iov_count, rx_size); iscsit_unmap_iovec(cmd); -- cgit v1.2.3-55-g7522 From 4b3766ec0e1840f45bc9238e7e749922bdcb7016 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 2 Apr 2019 12:58:15 -0700 Subject: scsi: target/iscsi: Make sure PDU processing continues if parsing a command fails Currently the iSCSI target driver sends a CHECK CONDITION code back to the initiator if the immediate data buffer is too large but it does not discard that immediate data buffer. The result is that the iSCSI target driver attempts to parse the immediate data itself as iSCSI PDUs and that all further iSCSI communication fails. Fix this by receiving and discarding too large immediate data buffers. Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 39 +++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 21 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index f01cdae54277..59d32453b891 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -1285,27 +1285,27 @@ iscsit_get_immediate_data(struct iscsi_cmd *cmd, struct iscsi_scsi_req *hdr, bool dump_payload) { int cmdsn_ret = 0, immed_ret = IMMEDIATE_DATA_NORMAL_OPERATION; + int rc; + /* * Special case for Unsupported SAM WRITE Opcodes and ImmediateData=Yes. */ - if (dump_payload) - goto after_immediate_data; - /* - * Check for underflow case where both EDTL and immediate data payload - * exceeds what is presented by CDB's TRANSFER LENGTH, and what has - * already been set in target_cmd_size_check() as se_cmd->data_length. - * - * For this special case, fail the command and dump the immediate data - * payload. - */ - if (cmd->first_burst_len > cmd->se_cmd.data_length) { - cmd->sense_reason = TCM_INVALID_CDB_FIELD; - goto after_immediate_data; + if (dump_payload) { + u32 length = min(cmd->se_cmd.data_length - cmd->write_data_done, + cmd->first_burst_len); + + pr_debug("Dumping min(%d - %d, %d) = %d bytes of immediate data\n", + cmd->se_cmd.data_length, cmd->write_data_done, + cmd->first_burst_len, length); + rc = iscsit_dump_data_payload(cmd->conn, length, 1); + pr_debug("Finished dumping immediate data\n"); + if (rc < 0) + immed_ret = IMMEDIATE_DATA_CANNOT_RECOVER; + } else { + immed_ret = iscsit_handle_immediate_data(cmd, hdr, + cmd->first_burst_len); } - immed_ret = iscsit_handle_immediate_data(cmd, hdr, - cmd->first_burst_len); -after_immediate_data: if (immed_ret == IMMEDIATE_DATA_NORMAL_OPERATION) { /* * A PDU/CmdSN carrying Immediate Data passed @@ -1318,12 +1318,9 @@ after_immediate_data: return -1; if (cmd->sense_reason || cmdsn_ret == CMDSN_LOWER_THAN_EXP) { - int rc; - - rc = iscsit_dump_data_payload(cmd->conn, - cmd->first_burst_len, 1); target_put_sess_cmd(&cmd->se_cmd); - return rc; + + return 0; } else if (cmd->unsolicited_data) iscsit_set_unsolicited_dataout(cmd); -- cgit v1.2.3-55-g7522