From patchwork Mon Apr 2 14:28:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7,COMMIT] ms/target: Fix VERIFY_16 handling in sbc_parse_cdb From: Konstantin Khorenko X-Patchwork-Id: 7978 Message-Id: <201804021428.w32ESJC6023667@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:19 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 8c4370283f897faa85c255fa5caddccb314ba8dd Author: Max Lohrmann Date: Mon Apr 2 17:28:19 2018 +0300 ms/target: Fix VERIFY_16 handling in sbc_parse_cdb ML: 13603685c1f12c67a7a2427f00b63f39a2b6f7c9 As reported by Max, the Windows 2008 R2 chkdsk utility expects VERIFY_16 to be supported, and does not handle the returned CHECK_CONDITION properly, resulting in an infinite loop. The kernel will log huge amounts of this error: kernel: TARGET_CORE[iSCSI]: Unsupported SCSI Opcode 0x8f, sending CHECK_CONDITION. Signed-off-by: Max Lohrmann Cc: Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_sbc.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 4be6c69537ae..8a799a8860b7 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -1018,9 +1018,15 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops) return ret; break; case VERIFY: + case VERIFY_16: size = 0; - sectors = transport_get_sectors_10(cdb); - cmd->t_task_lba = transport_lba_32(cdb); + if (cdb[0] == VERIFY) { + sectors = transport_get_sectors_10(cdb); + cmd->t_task_lba = transport_lba_32(cdb); + } else { + sectors = transport_get_sectors_16(cdb); + cmd->t_task_lba = transport_lba_64(cdb); + } cmd->execute_cmd = sbc_emulate_noop; goto check_lba; case REZERO_UNIT: From patchwork Mon Apr 2 14:28:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: reject COMPARE_AND_WRITE if emulate_caw is not set From: Konstantin Khorenko X-Patchwork-Id: 7966 Message-Id: <201804021428.w32ESKRx023722@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:20 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit a5eccba2193d16ddfce4b110fd31300b6dae22b4 Author: Jiang Yi Date: Mon Apr 2 17:28:19 2018 +0300 ms/target: reject COMPARE_AND_WRITE if emulate_caw is not set ML: 12f66e4a0f7b5624901ba4301210e026c9ddf78d In struct se_dev_attrib, there is a field emulate_caw exposed as a /sys/kernel/config/target/core/$HBA/$DEV/attrib/. If this field is set zero, it means the corresponding struct se_device does not support the scsi cmd COMPARE_AND_WRITE In function sbc_parse_cdb(), go ahead and reject scsi COMPARE_AND_WRITE if emulate_caw is not set, because it has been explicitly disabled from user-space. (Make pr_err ratelimited - nab) Signed-off-by: Jiang Yi Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_sbc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 8a799a8860b7..17889bd68a78 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -913,6 +913,12 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops) break; } case COMPARE_AND_WRITE: + if (!dev->dev_attrib.emulate_caw) { + pr_err_ratelimited("se_device %s/%s (vpd_unit_serial %s) reject" + " COMPARE_AND_WRITE\n", dev->transport->name, + dev->dev_group.cg_item.ci_name, dev->t10_wwn.unit_serial); + return TCM_UNSUPPORTED_SCSI_OPCODE; + } sectors = cdb[13]; /* * Currently enforce COMPARE_AND_WRITE for a single sector From patchwork Mon Apr 2 14:28:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7,COMMIT] ms/target: Inline transport_get_sense_codes() From: Konstantin Khorenko X-Patchwork-Id: 7971 Message-Id: <201804021428.w32ESKgw023773@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:20 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit d47712e564562670059fe8a1da7f9f8a308faf26 Author: Bart Van Assche Date: Mon Apr 2 17:28:20 2018 +0300 ms/target: Inline transport_get_sense_codes() ML: 46d5bd62ef9e3d6e2018963cbb725c91f864922d Inline this function in its call site since it performs a trivial task and since it is only called once. Signed-off-by: Bart Van Assche Signed-off-by: Sagi Grimberg Reviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Reviewed-by: Martin K. Petersen Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_transport.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 5d2487b840b1..9b0cacadd46a 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2659,17 +2659,6 @@ bool transport_wait_for_tasks(struct se_cmd *cmd) } EXPORT_SYMBOL(transport_wait_for_tasks); -static int transport_get_sense_codes( - struct se_cmd *cmd, - u8 *asc, - u8 *ascq) -{ - *asc = cmd->scsi_asc; - *ascq = cmd->scsi_ascq; - - return 0; -} - static void transport_err_sector_info(unsigned char *buffer, sector_t bad_sector) { @@ -2863,9 +2852,8 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd, buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; /* Not Ready */ buffer[SPC_SENSE_KEY_OFFSET] = NOT_READY; - transport_get_sense_codes(cmd, &asc, &ascq); - buffer[SPC_ASC_KEY_OFFSET] = asc; - buffer[SPC_ASCQ_KEY_OFFSET] = ascq; + buffer[SPC_ASC_KEY_OFFSET] = cmd->scsi_asc; + buffer[SPC_ASCQ_KEY_OFFSET] = cmd->scsi_ascq; break; case TCM_MISCOMPARE_VERIFY: /* CURRENT ERROR */ From patchwork Tue Mar 27 17:36:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [04/23] target: Split transport_send_check_condition_and_sense() From: Andrei Vagin X-Patchwork-Id: 7915 Message-Id: <1522172237-6960-5-git-send-email-avagin@openvz.org> To: devel@openvz.org Date: Tue, 27 Mar 2018 20:36:58 +0300 From: Bart Van Assche ML: ab78fef4d5f79134042ae0e1e2c259e1226aa5bd Move the code for translating a sense_reason_t code into a SCSI status ASC and ASCQ codes from transport_send_check_condition_and_sense() into the new function translate_sense_reason(). Convert the switch statement that performs the translation into table-driven code. Signed-off-by: Bart Van Assche Signed-off-by: Sagi Grimberg Reviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Reviewed-by: Martin K. Petersen Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_transport.c | 386 +++++++++++++-------------------- 1 file changed, 151 insertions(+), 235 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 66f5438..8af8049 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2682,13 +2682,158 @@ void transport_err_sector_info(unsigned char *buffer, sector_t bad_sector) put_unaligned_be64(bad_sector, &buffer[12]); } +struct sense_info { + u8 key; + u8 asc; + u8 ascq; + bool add_sector_info; +}; + +static const struct sense_info sense_info_table[] = { + [TCM_NO_SENSE] = { + .key = NOT_READY + }, + [TCM_NON_EXISTENT_LUN] = { + .key = ILLEGAL_REQUEST, + .asc = 0x25 /* LOGICAL UNIT NOT SUPPORTED */ + }, + [TCM_UNSUPPORTED_SCSI_OPCODE] = { + .key = ILLEGAL_REQUEST, + .asc = 0x20, /* INVALID COMMAND OPERATION CODE */ + }, + [TCM_SECTOR_COUNT_TOO_MANY] = { + .key = ILLEGAL_REQUEST, + .asc = 0x20, /* INVALID COMMAND OPERATION CODE */ + }, + [TCM_UNKNOWN_MODE_PAGE] = { + .key = ILLEGAL_REQUEST, + .asc = 0x24, /* INVALID FIELD IN CDB */ + }, + [TCM_CHECK_CONDITION_ABORT_CMD] = { + .key = ABORTED_COMMAND, + .asc = 0x29, /* BUS DEVICE RESET FUNCTION OCCURRED */ + .ascq = 0x03, + }, + [TCM_INCORRECT_AMOUNT_OF_DATA] = { + .key = ABORTED_COMMAND, + .asc = 0x0c, /* WRITE ERROR */ + .ascq = 0x0d, /* NOT ENOUGH UNSOLICITED DATA */ + }, + [TCM_INVALID_CDB_FIELD] = { + .key = ILLEGAL_REQUEST, + .asc = 0x24, /* INVALID FIELD IN CDB */ + }, + [TCM_INVALID_PARAMETER_LIST] = { + .key = ILLEGAL_REQUEST, + .asc = 0x26, /* INVALID FIELD IN PARAMETER LIST */ + }, + [TCM_PARAMETER_LIST_LENGTH_ERROR] = { + .key = ILLEGAL_REQUEST, + .asc = 0x1a, /* PARAMETER LIST LENGTH ERROR */ + }, + [TCM_UNEXPECTED_UNSOLICITED_DATA] = { + .key = ILLEGAL_REQUEST, + .asc = 0x0c, /* WRITE ERROR */ + .ascq = 0x0c, /* UNEXPECTED_UNSOLICITED_DATA */ + }, + [TCM_SERVICE_CRC_ERROR] = { + .key = ABORTED_COMMAND, + .asc = 0x47, /* PROTOCOL SERVICE CRC ERROR */ + .ascq = 0x05, /* N/A */ + }, + [TCM_SNACK_REJECTED] = { + .key = ABORTED_COMMAND, + .asc = 0x11, /* READ ERROR */ + .ascq = 0x13, /* FAILED RETRANSMISSION REQUEST */ + }, + [TCM_WRITE_PROTECTED] = { + .key = DATA_PROTECT, + .asc = 0x27, /* WRITE PROTECTED */ + }, + [TCM_ADDRESS_OUT_OF_RANGE] = { + .key = ILLEGAL_REQUEST, + .asc = 0x21, /* LOGICAL BLOCK ADDRESS OUT OF RANGE */ + }, + [TCM_CHECK_CONDITION_UNIT_ATTENTION] = { + .key = UNIT_ATTENTION, + }, + [TCM_CHECK_CONDITION_NOT_READY] = { + .key = NOT_READY, + }, + [TCM_MISCOMPARE_VERIFY] = { + .key = MISCOMPARE, + .asc = 0x1d, /* MISCOMPARE DURING VERIFY OPERATION */ + .ascq = 0x00, + }, + [TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED] = { + .key = ILLEGAL_REQUEST, + .asc = 0x10, + .ascq = 0x01, /* LOGICAL BLOCK GUARD CHECK FAILED */ + .add_sector_info = true, + }, + [TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED] = { + .key = ILLEGAL_REQUEST, + .asc = 0x10, + .ascq = 0x02, /* LOGICAL BLOCK APPLICATION TAG CHECK FAILED */ + .add_sector_info = true, + }, + [TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED] = { + .key = ILLEGAL_REQUEST, + .asc = 0x10, + .ascq = 0x03, /* LOGICAL BLOCK REFERENCE TAG CHECK FAILED */ + .add_sector_info = true, + }, + [TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE] = { + /* + * Returning ILLEGAL REQUEST would cause immediate IO errors on + * Solaris initiators. Returning NOT READY instead means the + * operations will be retried a finite number of times and we + * can survive intermittent errors. + */ + .key = NOT_READY, + .asc = 0x08, /* LOGICAL UNIT COMMUNICATION FAILURE */ + }, +}; + +static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) +{ + const struct sense_info *si; + u8 *buffer = cmd->sense_buffer; + int r = (__force int)reason; + u8 asc, ascq; + + buffer[0] = 0x70; + buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; + + if (r < ARRAY_SIZE(sense_info_table) && sense_info_table[r].key) + si = &sense_info_table[r]; + else + si = &sense_info_table[(__force int) + TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE]; + + buffer[SPC_SENSE_KEY_OFFSET] = si->key; + if (reason == TCM_CHECK_CONDITION_UNIT_ATTENTION) { + core_scsi3_ua_for_check_condition(cmd, &asc, &ascq); + WARN_ON_ONCE(asc == 0); + } else if (si->asc == 0) { + WARN_ON_ONCE(cmd->scsi_asc == 0); + asc = cmd->scsi_asc; + ascq = cmd->scsi_ascq; + } else { + asc = si->asc; + ascq = si->ascq; + } + buffer[SPC_ASC_KEY_OFFSET] = asc; + buffer[SPC_ASCQ_KEY_OFFSET] = ascq; + if (si->add_sector_info) + transport_err_sector_info(cmd->sense_buffer, cmd->bad_sector); +} + int transport_send_check_condition_and_sense(struct se_cmd *cmd, sense_reason_t reason, int from_transport) { - unsigned char *buffer = cmd->sense_buffer; unsigned long flags; - u8 asc = 0, ascq = 0; spin_lock_irqsave(&cmd->t_state_lock, flags); if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION) { @@ -2698,242 +2843,13 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd, cmd->se_cmd_flags |= SCF_SENT_CHECK_CONDITION; spin_unlock_irqrestore(&cmd->t_state_lock, flags); - if (!reason && from_transport) - goto after_reason; - - if (!from_transport) + if (!from_transport) { cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; - - /* - * Actual SENSE DATA, see SPC-3 7.23.2 SPC_SENSE_KEY_OFFSET uses - * SENSE KEY values from include/scsi/scsi.h - */ - switch (reason) { - case TCM_NO_SENSE: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* Not Ready */ - buffer[SPC_SENSE_KEY_OFFSET] = NOT_READY; - /* NO ADDITIONAL SENSE INFORMATION */ - buffer[SPC_ASC_KEY_OFFSET] = 0; - buffer[SPC_ASCQ_KEY_OFFSET] = 0; - break; - case TCM_NON_EXISTENT_LUN: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* LOGICAL UNIT NOT SUPPORTED */ - buffer[SPC_ASC_KEY_OFFSET] = 0x25; - break; - case TCM_UNSUPPORTED_SCSI_OPCODE: - case TCM_SECTOR_COUNT_TOO_MANY: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* INVALID COMMAND OPERATION CODE */ - buffer[SPC_ASC_KEY_OFFSET] = 0x20; - break; - case TCM_UNKNOWN_MODE_PAGE: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* INVALID FIELD IN CDB */ - buffer[SPC_ASC_KEY_OFFSET] = 0x24; - break; - case TCM_CHECK_CONDITION_ABORT_CMD: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ABORTED COMMAND */ - buffer[SPC_SENSE_KEY_OFFSET] = ABORTED_COMMAND; - /* BUS DEVICE RESET FUNCTION OCCURRED */ - buffer[SPC_ASC_KEY_OFFSET] = 0x29; - buffer[SPC_ASCQ_KEY_OFFSET] = 0x03; - break; - case TCM_INCORRECT_AMOUNT_OF_DATA: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ABORTED COMMAND */ - buffer[SPC_SENSE_KEY_OFFSET] = ABORTED_COMMAND; - /* WRITE ERROR */ - buffer[SPC_ASC_KEY_OFFSET] = 0x0c; - /* NOT ENOUGH UNSOLICITED DATA */ - buffer[SPC_ASCQ_KEY_OFFSET] = 0x0d; - break; - case TCM_INVALID_CDB_FIELD: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* INVALID FIELD IN CDB */ - buffer[SPC_ASC_KEY_OFFSET] = 0x24; - break; - case TCM_INVALID_PARAMETER_LIST: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* INVALID FIELD IN PARAMETER LIST */ - buffer[SPC_ASC_KEY_OFFSET] = 0x26; - break; - case TCM_PARAMETER_LIST_LENGTH_ERROR: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* PARAMETER LIST LENGTH ERROR */ - buffer[SPC_ASC_KEY_OFFSET] = 0x1a; - break; - case TCM_UNEXPECTED_UNSOLICITED_DATA: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ABORTED COMMAND */ - buffer[SPC_SENSE_KEY_OFFSET] = ABORTED_COMMAND; - /* WRITE ERROR */ - buffer[SPC_ASC_KEY_OFFSET] = 0x0c; - /* UNEXPECTED_UNSOLICITED_DATA */ - buffer[SPC_ASCQ_KEY_OFFSET] = 0x0c; - break; - case TCM_SERVICE_CRC_ERROR: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ABORTED COMMAND */ - buffer[SPC_SENSE_KEY_OFFSET] = ABORTED_COMMAND; - /* PROTOCOL SERVICE CRC ERROR */ - buffer[SPC_ASC_KEY_OFFSET] = 0x47; - /* N/A */ - buffer[SPC_ASCQ_KEY_OFFSET] = 0x05; - break; - case TCM_SNACK_REJECTED: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ABORTED COMMAND */ - buffer[SPC_SENSE_KEY_OFFSET] = ABORTED_COMMAND; - /* READ ERROR */ - buffer[SPC_ASC_KEY_OFFSET] = 0x11; - /* FAILED RETRANSMISSION REQUEST */ - buffer[SPC_ASCQ_KEY_OFFSET] = 0x13; - break; - case TCM_WRITE_PROTECTED: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* DATA PROTECT */ - buffer[SPC_SENSE_KEY_OFFSET] = DATA_PROTECT; - /* WRITE PROTECTED */ - buffer[SPC_ASC_KEY_OFFSET] = 0x27; - break; - case TCM_ADDRESS_OUT_OF_RANGE: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* LOGICAL BLOCK ADDRESS OUT OF RANGE */ - buffer[SPC_ASC_KEY_OFFSET] = 0x21; - break; - case TCM_CHECK_CONDITION_UNIT_ATTENTION: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* UNIT ATTENTION */ - buffer[SPC_SENSE_KEY_OFFSET] = UNIT_ATTENTION; - core_scsi3_ua_for_check_condition(cmd, &asc, &ascq); - buffer[SPC_ASC_KEY_OFFSET] = asc; - buffer[SPC_ASCQ_KEY_OFFSET] = ascq; - break; - case TCM_CHECK_CONDITION_NOT_READY: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* Not Ready */ - buffer[SPC_SENSE_KEY_OFFSET] = NOT_READY; - buffer[SPC_ASC_KEY_OFFSET] = cmd->scsi_asc; - buffer[SPC_ASCQ_KEY_OFFSET] = cmd->scsi_ascq; - break; - case TCM_MISCOMPARE_VERIFY: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - buffer[SPC_SENSE_KEY_OFFSET] = MISCOMPARE; - /* MISCOMPARE DURING VERIFY OPERATION */ - buffer[SPC_ASC_KEY_OFFSET] = 0x1d; - buffer[SPC_ASCQ_KEY_OFFSET] = 0x00; - break; - case TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* LOGICAL BLOCK GUARD CHECK FAILED */ - buffer[SPC_ASC_KEY_OFFSET] = 0x10; - buffer[SPC_ASCQ_KEY_OFFSET] = 0x01; - transport_err_sector_info(buffer, cmd->bad_sector); - break; - case TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* LOGICAL BLOCK APPLICATION TAG CHECK FAILED */ - buffer[SPC_ASC_KEY_OFFSET] = 0x10; - buffer[SPC_ASCQ_KEY_OFFSET] = 0x02; - transport_err_sector_info(buffer, cmd->bad_sector); - break; - case TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* ILLEGAL REQUEST */ - buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; - /* LOGICAL BLOCK REFERENCE TAG CHECK FAILED */ - buffer[SPC_ASC_KEY_OFFSET] = 0x10; - buffer[SPC_ASCQ_KEY_OFFSET] = 0x03; - transport_err_sector_info(buffer, cmd->bad_sector); - break; - case TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE: - default: - /* CURRENT ERROR */ - buffer[0] = 0x70; - buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; - /* - * Returning ILLEGAL REQUEST would cause immediate IO errors on - * Solaris initiators. Returning NOT READY instead means the - * operations will be retried a finite number of times and we - * can survive intermittent errors. - */ - buffer[SPC_SENSE_KEY_OFFSET] = NOT_READY; - /* LOGICAL UNIT COMMUNICATION FAILURE */ - buffer[SPC_ASC_KEY_OFFSET] = 0x08; - break; + translate_sense_reason(cmd, reason); + cmd->scsi_status = SAM_STAT_CHECK_CONDITION; + cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; } - /* - * This code uses linux/include/scsi/scsi.h SAM status codes! - */ - cmd->scsi_status = SAM_STAT_CHECK_CONDITION; - /* - * Automatically padded, this value is encoded in the fabric's - * data_length response PDU containing the SCSI defined sense data. - */ - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; -after_reason: trace_target_cmd_complete(cmd); return cmd->se_tfo->queue_status(cmd); } From patchwork Mon Apr 2 14:28:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Make EXTENDED_COPY 0xe4 failure return COPY TARGET DEVICE NOT REACHABLE From: Konstantin Khorenko X-Patchwork-Id: 7977 Message-Id: <201804021428.w32ESLtc023875@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:21 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit e88c04a22ad5034135f77ae0a8fffba0d6eb2fb0 Author: Nicholas Bellinger Date: Mon Apr 2 17:28:21 2018 +0300 ms/target: Make EXTENDED_COPY 0xe4 failure return COPY TARGET DEVICE NOT REACHABLE ML: 449a137846c84829a328757cd21fd9ca65c08519 This patch addresses a bug where EXTENDED_COPY across multiple LUNs results in a CHECK_CONDITION when the source + destination are not located on the same physical node. ESX Host environments expect sense COPY_ABORTED w/ COPY TARGET DEVICE NOT REACHABLE to be returned when this occurs, in order to signal fallback to local copy method. As described in section 6.3.3 of spc4r22: "If it is not possible to complete processing of a segment because the copy manager is unable to establish communications with a copy target device, because the copy target device does not respond to INQUIRY, or because the data returned in response to INQUIRY indicates an unsupported logical unit, then the EXTENDED COPY command shall be terminated with CHECK CONDITION status, with the sense key set to COPY ABORTED, and the additional sense code set to COPY TARGET DEVICE NOT REACHABLE." Tested on v4.1.y with ESX v5.5u2+ with BlockCopy across multiple nodes. Reported-by: Nixon Vincent Tested-by: Nixon Vincent Cc: Nixon Vincent Tested-by: Dinesh Israni Signed-off-by: Dinesh Israni Cc: Dinesh Israni Cc: stable@vger.kernel.org # 3.14+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_transport.c | 7 +++++++ drivers/target/target_core_xcopy.c | 22 ++++++++++++++++------ include/target/target_core_base.h | 1 + 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 2a01d8e88fbb..5bc36e428cf1 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1656,6 +1656,7 @@ void transport_generic_request_failure(struct se_cmd *cmd, case TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED: case TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED: case TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED: + case TCM_COPY_TARGET_DEVICE_NOT_REACHABLE: break; case TCM_OUT_OF_RESOURCES: sense_reason = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; @@ -2773,6 +2774,12 @@ static const struct sense_info sense_info_table[] = { .ascq = 0x03, /* LOGICAL BLOCK REFERENCE TAG CHECK FAILED */ .add_sector_info = true, }, + [TCM_COPY_TARGET_DEVICE_NOT_REACHABLE] = { + .key = COPY_ABORTED, + .asc = 0x0d, + .ascq = 0x02, /* COPY TARGET DEVICE NOT REACHABLE */ + + }, [TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE] = { /* * Returning ILLEGAL REQUEST would cause immediate IO errors on diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index a7c89740feb0..5cfb3829c2d6 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -116,7 +116,7 @@ static int target_xcopy_locate_se_dev_e4(struct se_cmd *se_cmd, struct xcopy_op } mutex_unlock(&g_device_mutex); - pr_err("Unable to locate 0xe4 descriptor for EXTENDED_COPY\n"); + pr_debug_ratelimited("Unable to locate 0xe4 descriptor for EXTENDED_COPY\n"); return -EINVAL; } @@ -197,7 +197,7 @@ static int target_xcopy_parse_tiddesc_e4(struct se_cmd *se_cmd, struct xcopy_op static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, struct xcopy_op *xop, unsigned char *p, - unsigned short tdll) + unsigned short tdll, sense_reason_t *sense_ret) { struct se_device *local_dev = se_cmd->se_dev; unsigned char *desc = p; @@ -205,6 +205,8 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, unsigned short start = 0; bool src = true; + *sense_ret = TCM_INVALID_PARAMETER_LIST; + if (offset != 0) { pr_err("XCOPY target descriptor list length is not" " multiple of %d\n", XCOPY_TARGET_DESC_LEN); @@ -255,9 +257,16 @@ static int target_xcopy_parse_target_descriptors(struct se_cmd *se_cmd, rc = target_xcopy_locate_se_dev_e4(se_cmd, xop, true); else rc = target_xcopy_locate_se_dev_e4(se_cmd, xop, false); - - if (rc < 0) + /* + * If a matching IEEE NAA 0x83 descriptor for the requested device + * 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. + */ + if (rc < 0) { + *sense_ret = TCM_COPY_TARGET_DEVICE_NOT_REACHABLE; goto out; + } pr_debug("XCOPY TGT desc: Source dev: %p NAA IEEE WWN: 0x%16phN\n", xop->src_dev, &xop->src_tid_wwn[0]); @@ -861,7 +870,8 @@ static void target_xcopy_do_work(struct work_struct *work) xcopy_pt_undepend_remotedev(xop); kfree(xop); - pr_warn("target_xcopy_do_work: Setting X-COPY CHECK_CONDITION -> sending response\n"); + pr_warn_ratelimited("target_xcopy_do_work: rc: %d, Setting X-COPY CHECK_CONDITION" + " -> sending response\n", rc); ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION; target_complete_cmd(ec_cmd, SAM_STAT_CHECK_CONDITION); } @@ -920,7 +930,7 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) " tdll: %hu sdll: %u inline_dl: %u\n", list_id, list_id_usage, tdll, sdll, inline_dl); - rc = target_xcopy_parse_target_descriptors(se_cmd, xop, &p[16], tdll); + rc = target_xcopy_parse_target_descriptors(se_cmd, xop, &p[16], tdll, &ret); if (rc <= 0) goto out; diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 288935002951..55c66cdbb414 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -214,6 +214,7 @@ enum tcm_sense_reason_table { TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED = R(0x15), TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED = R(0x16), TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED = R(0x17), + TCM_COPY_TARGET_DEVICE_NOT_REACHABLE = R(0x18), #undef R }; From patchwork Mon Apr 2 14:28:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Don't override EXTENDED_COPY xcopy_pt_cmd SCSI status code From: Konstantin Khorenko X-Patchwork-Id: 7973 Message-Id: <201804021428.w32ESMMX023926@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:22 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit f03b7c21a02e2426416abbcd85b1cdba93c0e1c0 Author: Dinesh Israni Date: Mon Apr 2 17:28:22 2018 +0300 ms/target: Don't override EXTENDED_COPY xcopy_pt_cmd SCSI status code ML: 926317de33998c112c5510301868ea9aa34097e2 This patch addresses a bug where a local EXTENDED_COPY WRITE or READ backend I/O request would always return SAM_STAT_CHECK_CONDITION, even if underlying xcopy_pt_cmd->se_cmd generated a different SCSI status code. ESX host environments expect to hit SAM_STAT_RESERVATION_CONFLICT for certain scenarios, and SAM_STAT_CHECK_CONDITION results in non-retriable status for these cases. Tested on v4.1.y with ESX v5.5u2+ with local IBLOCK backend copy. Reported-by: Nixon Vincent Tested-by: Nixon Vincent Cc: Nixon Vincent Tested-by: Dinesh Israni Signed-off-by: Dinesh Israni Cc: Dinesh Israni Cc: stable@vger.kernel.org # 3.14+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_xcopy.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 5cfb3829c2d6..9f5a002bfa04 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -707,6 +707,7 @@ static int target_xcopy_read_source( rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, src_dev, &cdb[0], remote_port, true); if (rc < 0) { + ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status; transport_generic_free_cmd(se_cmd, 0); return rc; } @@ -718,6 +719,7 @@ static int target_xcopy_read_source( rc = target_xcopy_issue_pt_cmd(xpt_cmd); if (rc < 0) { + ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status; transport_generic_free_cmd(se_cmd, 0); return rc; } @@ -768,6 +770,7 @@ static int target_xcopy_write_destination( remote_port, false); if (rc < 0) { struct se_cmd *src_cmd = &xop->src_pt_cmd->se_cmd; + ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status; /* * If the failure happened before the t_mem_list hand-off in * target_xcopy_setup_pt_cmd(), Reset memory + clear flag so that @@ -783,6 +786,7 @@ static int target_xcopy_write_destination( rc = target_xcopy_issue_pt_cmd(xpt_cmd); if (rc < 0) { + ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status; se_cmd->se_cmd_flags &= ~SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC; transport_generic_free_cmd(se_cmd, 0); return rc; @@ -869,10 +873,14 @@ static void target_xcopy_do_work(struct work_struct *work) out: xcopy_pt_undepend_remotedev(xop); kfree(xop); - - pr_warn_ratelimited("target_xcopy_do_work: rc: %d, Setting X-COPY CHECK_CONDITION" - " -> sending response\n", rc); - ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION; + /* + * Don't override an error scsi status if it has already been set + */ + if (ec_cmd->scsi_status == SAM_STAT_GOOD) { + pr_warn_ratelimited("target_xcopy_do_work: rc: %d, Setting X-COPY" + " CHECK_CONDITION -> sending response\n", rc); + ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION; + } target_complete_cmd(ec_cmd, SAM_STAT_CHECK_CONDITION); } From patchwork Mon Apr 2 14:28:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Use correct SCSI status during EXTENDED_COPY exception From: Konstantin Khorenko X-Patchwork-Id: 7980 Message-Id: <201804021428.w32ESMgv023976@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:22 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 18f9c25c5b1613911af11e42b4a7e739629d3dd4 Author: Nicholas Bellinger Date: Mon Apr 2 17:28:22 2018 +0300 ms/target: Use correct SCSI status during EXTENDED_COPY exception ML: 0583c261e6325f392c1f7a1b9112e31298e1a4bd This patch adds the missing target_complete_cmd() SCSI status parameter change in target_xcopy_do_work(), that was originally missing in commit 926317de33. It correctly propigates up the correct SCSI status during EXTENDED_COPY exception cases, instead of always using the hardcoded SAM_STAT_CHECK_CONDITION from original code. This is required for ESX host environments that expect to hit SAM_STAT_RESERVATION_CONFLICT for certain scenarios, and SAM_STAT_CHECK_CONDITION results in non-retriable status for these cases. Reported-by: Nixon Vincent Tested-by: Nixon Vincent Cc: Nixon Vincent Reviewed-by: Christoph Hellwig Cc: stable@vger.kernel.org # 3.14+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_xcopy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 9f5a002bfa04..94e95748e815 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -881,7 +881,7 @@ static void target_xcopy_do_work(struct work_struct *work) " CHECK_CONDITION -> sending response\n", rc); ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION; } - target_complete_cmd(ec_cmd, SAM_STAT_CHECK_CONDITION); + target_complete_cmd(ec_cmd, ec_cmd->scsi_status); } sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) From patchwork Mon Apr 2 14:28:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target/iscsi: Fix double free of a TUR followed by a solicited NOPOUT From: Konstantin Khorenko X-Patchwork-Id: 7965 Message-Id: <201804021428.w32ESNVd024026@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:23 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit a2d16a61f380825a9c9b25abb8d6bfc3e6ce1314 Author: Alexei Potashnik Date: Mon Apr 2 17:28:23 2018 +0300 ms/target/iscsi: Fix double free of a TUR followed by a solicited NOPOUT ML: 9547308bda296b6f69876c840a0291fcfbeddbb8 Make sure all non-READ SCSI commands get targ_xfer_tag initialized to 0xffffffff, not just WRITEs. Double-free of a TUR cmd object occurs under the following scenario: 1. TUR received (targ_xfer_tag is uninitialized and left at 0) 2. TUR status sent 3. First unsolicited NOPIN is sent to initiator (gets targ_xfer_tag of 0) 4. NOPOUT for NOPIN (with TTT=0) arrives - its ExpStatSN acks TUR status, TUR is queued for removal - LIO tries to find NOPIN with TTT=0, but finds the same TUR instead, TUR is queued for removal for the 2nd time (Drop unbalanced conditional bracket usage - nab) Signed-off-by: Alexei Potashnik Signed-off-by: Spencer Baugh Cc: # v3.1+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/iscsi/iscsi_target.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 35209f0a469a..ea8573e9e129 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -1117,9 +1117,9 @@ int iscsit_setup_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, cmd->cmd_flags |= ICF_NON_IMMEDIATE_UNSOLICITED_DATA; conn->sess->init_task_tag = cmd->init_task_tag = hdr->itt; - if (hdr->flags & ISCSI_FLAG_CMD_READ) { + if (hdr->flags & ISCSI_FLAG_CMD_READ) cmd->targ_xfer_tag = session_get_next_ttt(conn->sess); - } else if (hdr->flags & ISCSI_FLAG_CMD_WRITE) + else cmd->targ_xfer_tag = 0xFFFFFFFF; cmd->cmd_sn = be32_to_cpu(hdr->cmdsn); cmd->exp_stat_sn = be32_to_cpu(hdr->exp_statsn); From patchwork Mon Apr 2 14:28:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: fix COMPARE_AND_WRITE non zero SGL offset data corruption From: Konstantin Khorenko X-Patchwork-Id: 7983 Message-Id: <201804021428.w32ESNBa024077@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:23 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 6ee7a5696a9b835cc68b0468cf15382f98639321 Author: Jan Engelhardt Date: Mon Apr 2 17:28:23 2018 +0300 ms/target: fix COMPARE_AND_WRITE non zero SGL offset data corruption ML: d94e5a61357a04938ce14d6033b4d33a3c5fd780 target_core_sbc's compare_and_write functionality suffers from taking data at the wrong memory location when writing a CAW request to disk when a SGL offset is non-zero. This can happen with loopback and vhost-scsi fabric drivers when SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC is used to map existing user-space SGL memory into COMPARE_AND_WRITE READ/WRITE payload buffers. Given the following sample LIO subtopology, % targetcli ls /loopback/ o- loopback ................................. [1 Target] o- naa.6001405ebb8df14a ....... [naa.60014059143ed2b3] o- luns ................................... [2 LUNs] o- lun0 ................ [iblock/ram0 (/dev/ram0)] o- lun1 ................ [iblock/ram1 (/dev/ram1)] % lsscsi -g [3:0:1:0] disk LIO-ORG IBLOCK 4.0 /dev/sdc /dev/sg3 [3:0:1:1] disk LIO-ORG IBLOCK 4.0 /dev/sdd /dev/sg4 the following bug can be observed in Linux 4.3 and 4.4~rc1: % perl -e 'print chr$_ for 0..255,reverse 0..255' >rand % perl -e 'print "\0" x 512' >zero % cat rand >/dev/sdd % sg_compare_and_write -i rand -D zero --lba 0 /dev/sdd % sg_compare_and_write -i zero -D rand --lba 0 /dev/sdd Miscompare reported % hexdump -Cn 512 /dev/sdd 00000000 0f 0e 0d 0c 0b 0a 09 08 07 06 05 04 03 02 01 00 00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 00000200 Rather than writing all-zeroes as instructed with the -D file, it corrupts the data in the sector by splicing some of the original bytes in. The page of the first entry of cmd->t_data_sg includes the CDB, and sg->offset is set to a position past the CDB. I presume that sg->offset is also the right choice to use for subsequent sglist members. Signed-off-by: Jan Engelhardt Tested-by: Douglas Gilbert Cc: # v3.12+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_sbc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 17889bd68a78..1448dce5fde8 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -503,11 +503,11 @@ static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes if (block_size < PAGE_SIZE) { sg_set_page(&write_sg[i], m.page, block_size, - block_size); + m.piter.sg->offset + block_size); } else { sg_miter_next(&m); sg_set_page(&write_sg[i], m.page, block_size, - 0); + m.piter.sg->offset); } len -= block_size; i++; From patchwork Mon Apr 2 14:28:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/iscsi-target: Fix potential dead-lock during node acl delete From: Konstantin Khorenko X-Patchwork-Id: 7967 Message-Id: <201804021428.w32ESOUW024128@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:24 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 8d78cb0af827bc1694b4ac512a8fef7680171f7f Author: Nicholas Bellinger Date: Mon Apr 2 17:28:24 2018 +0300 ms/iscsi-target: Fix potential dead-lock during node acl delete ML: 26a99c19f810b2593410899a5b304b21b47428a6 This patch is a iscsi-target specific bug-fix for a dead-lock that can occur during explicit struct se_node_acl->acl_group se_session deletion via configfs rmdir(2), when iscsi-target time2retain timer is still active. It changes iscsi-target to obtain se_portal_group->session_lock internally using spin_in_locked() to check for the specific se_node_acl configfs shutdown rmdir(2) case. Note this patch is intended for stable, and the subsequent v4.5-rc patch converts target_core_tpg.c to use proper se_sess->sess_kref reference counting for both se_node_acl deletion + se_node_acl->queue_depth se_session restart. Reported-by:: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/iscsi/iscsi_target_configfs.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c index 6d754cfe3fc4..f43e9757366d 100644 --- a/drivers/target/iscsi/iscsi_target_configfs.c +++ b/drivers/target/iscsi/iscsi_target_configfs.c @@ -2016,7 +2016,8 @@ static void lio_tpg_release_fabric_acl( } /* - * Called with spin_lock_bh(struct se_portal_group->session_lock) held.. + * Called with spin_lock_irq(struct se_portal_group->session_lock) held + * or not held. * * Also, this function calls iscsit_inc_session_usage_count() on the * struct iscsi_session in question. @@ -2024,19 +2025,32 @@ static void lio_tpg_release_fabric_acl( static int lio_tpg_shutdown_session(struct se_session *se_sess) { struct iscsi_session *sess = se_sess->fabric_sess_ptr; + struct se_portal_group *se_tpg = se_sess->se_tpg; + bool local_lock = false; + + if (!spin_is_locked(&se_tpg->session_lock)) { + spin_lock_irq(&se_tpg->session_lock); + local_lock = true; + } spin_lock(&sess->conn_lock); if (atomic_read(&sess->session_fall_back_to_erl0) || atomic_read(&sess->session_logout) || (sess->time2retain_timer_flags & ISCSI_TF_EXPIRED)) { spin_unlock(&sess->conn_lock); + if (local_lock) + spin_unlock_irq(&sess->conn_lock); return 0; } atomic_set(&sess->session_reinstatement, 1); spin_unlock(&sess->conn_lock); iscsit_stop_time2retain_timer(sess); + spin_unlock_irq(&se_tpg->session_lock); + iscsit_stop_session(sess, 1, 1); + if (!local_lock) + spin_lock_irq(&se_tpg->session_lock); return 1; } From patchwork Mon Apr 2 14:28:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target/sbc: Add LBPRZ attribute + control CDB emulation From: Konstantin Khorenko X-Patchwork-Id: 7962 Message-Id: <201804021428.w32ESPO3024178@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:25 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 6670637e00d204e25c3d7b67f022b825d93520e4 Author: Jamie Pocas Date: Mon Apr 2 17:28:24 2018 +0300 ms/target/sbc: Add LBPRZ attribute + control CDB emulation ML: e6f41633cb79b55ead84b023c02035322c7827e7 This change sets the LBPRZ flag in EVPD page b2h and READ CAPACITY (16) based on a new unmap_zeroes_data device attribute. This flag is set automatically for iblock based on underlying block device queue's discard_zeroes_data flag. Signed-off-by: Jamie Pocas Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_iblock.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index c68e2555edf4..02c5ab0a56e8 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -144,6 +144,8 @@ static int iblock_configure_device(struct se_device *dev) q->limits.discard_granularity >> 9; dev->dev_attrib.unmap_granularity_alignment = q->limits.discard_alignment; + dev->dev_attrib.unmap_zeroes_data = + q->limits.discard_zeroes_data; pr_debug("IBLOCK: BLOCK Discard support available," " disabled by default\n"); From patchwork Mon Apr 2 14:28:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Fix WRITE_SAME/DISCARD conversion to linux 512b sectors From: Konstantin Khorenko X-Patchwork-Id: 7968 Message-Id: <201804021428.w32ESPPv024228@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:25 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit a22a8a84c0226f7f2696e7e42f1786b2f61b0884 Author: Mike Christie Date: Mon Apr 2 17:28:25 2018 +0300 ms/target: Fix WRITE_SAME/DISCARD conversion to linux 512b sectors ML: 8a9ebe717a133ba7bc90b06047f43cc6b8bcb8b3 In a couple places we are not converting to/from the Linux block layer 512 bytes sectors. 1. The request queue values and what we do are a mismatch of things: max_discard_sectors - This is in linux block layer 512 byte sectors. We are just copying this to max_unmap_lba_count. discard_granularity - This is in bytes. We are converting it to Linux block layer 512 byte sectors. discard_alignment - This is in bytes. We are just copying this over. The problem is that the core LIO code exports these values in spc_emulate_evpd_b0 and we use them to test request arguments in sbc_execute_unmap, but we never convert to the block size we export to the initiator. If we are not using 512 byte sectors then we are exporting the wrong values or are checks are off. And, for the discard_alignment/bytes case we are just plain messed up. 2. blkdev_issue_discard's start and number of sector arguments are supposed to be in linux block layer 512 byte sectors. We are currently passing in the values we get from the initiator which might be based on some other sector size. There is a similar problem in iblock_execute_write_same where the bio functions want values in 512 byte sectors but we are passing in what we got from the initiator. Signed-off-by: Mike Christie Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_device.c | 44 +++++++++++++++++++++++++++ drivers/target/target_core_file.c | 29 ++++++------------ drivers/target/target_core_iblock.c | 58 +++++++++--------------------------- include/target/target_core_backend.h | 4 +++ 4 files changed, 71 insertions(+), 64 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 3621e6444f3d..81156de492a3 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -1583,6 +1583,50 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) return dev; } +/* + * Check if the underlying struct block_device request_queue supports + * the QUEUE_FLAG_DISCARD bit for UNMAP/WRITE_SAME in SCSI + TRIM + * in ATA and we need to set TPE=1 + */ +bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib, + struct request_queue *q, int block_size) +{ + if (!blk_queue_discard(q)) + return false; + + attrib->max_unmap_lba_count = (q->limits.max_discard_sectors << 9) / + block_size; + /* + * Currently hardcoded to 1 in Linux/SCSI code.. + */ + attrib->max_unmap_block_desc_count = 1; + attrib->unmap_granularity = q->limits.discard_granularity / block_size; + attrib->unmap_granularity_alignment = q->limits.discard_alignment / + block_size; + attrib->unmap_zeroes_data = q->limits.discard_zeroes_data; + return true; +} +EXPORT_SYMBOL(target_configure_unmap_from_queue); + +/* + * Convert from blocksize advertised to the initiator to the 512 byte + * units unconditionally used by the Linux block layer. + */ +sector_t target_to_linux_sector(struct se_device *dev, sector_t lb) +{ + switch (dev->dev_attrib.block_size) { + case 4096: + return lb << 3; + case 2048: + return lb << 2; + case 1024: + return lb << 1; + default: + return lb; + } +} +EXPORT_SYMBOL(target_to_linux_sector); + int target_configure_device(struct se_device *dev) { struct se_hba *hba = dev->se_hba; diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index de165f5c3aee..5c92a0814e24 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -165,25 +165,11 @@ static int fd_configure_device(struct se_device *dev) " block_device blocks: %llu logical_block_size: %d\n", dev_size, div_u64(dev_size, fd_dev->fd_block_size), fd_dev->fd_block_size); - /* - * Check if the underlying struct block_device request_queue supports - * the QUEUE_FLAG_DISCARD bit for UNMAP/WRITE_SAME in SCSI + TRIM - * in ATA and we need to set TPE=1 - */ - if (blk_queue_discard(q)) { - dev->dev_attrib.max_unmap_lba_count = - q->limits.max_discard_sectors; - /* - * Currently hardcoded to 1 in Linux/SCSI code.. - */ - dev->dev_attrib.max_unmap_block_desc_count = 1; - dev->dev_attrib.unmap_granularity = - q->limits.discard_granularity >> 9; - dev->dev_attrib.unmap_granularity_alignment = - q->limits.discard_alignment; + + if (target_configure_unmap_from_queue(&dev->dev_attrib, q, + fd_dev->fd_block_size)) pr_debug("IFILE: BLOCK Discard support available," - " disabled by default\n"); - } + " disabled by default\n"); /* * Enable write same emulation for IBLOCK and use 0xFFFF as * the smaller WRITE_SAME(10) only has a two-byte block count. @@ -607,9 +593,12 @@ fd_do_unmap(struct se_cmd *cmd, void *priv, sector_t lba, sector_t nolb) if (S_ISBLK(inode->i_mode)) { /* The backend is block device, use discard */ struct block_device *bdev = inode->i_bdev; + struct se_device *dev = cmd->se_dev; - ret = blkdev_issue_discard(bdev, lba, - nolb, GFP_KERNEL, 0); + ret = blkdev_issue_discard(bdev, + target_to_linux_sector(dev, lba), + target_to_linux_sector(dev, nolb), + GFP_KERNEL, 0); if (ret < 0) { pr_warn("FILEIO: blkdev_issue_discard() failed: %d\n", ret); diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index 02c5ab0a56e8..00bfe975ac72 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -127,29 +127,11 @@ static int iblock_configure_device(struct se_device *dev) dev->dev_attrib.hw_max_sectors = queue_max_hw_sectors(q); dev->dev_attrib.hw_queue_depth = q->nr_requests; - /* - * Check if the underlying struct block_device request_queue supports - * the QUEUE_FLAG_DISCARD bit for UNMAP/WRITE_SAME in SCSI + TRIM - * in ATA and we need to set TPE=1 - */ - if (blk_queue_discard(q)) { - dev->dev_attrib.max_unmap_lba_count = - q->limits.max_discard_sectors; - - /* - * Currently hardcoded to 1 in Linux/SCSI code.. - */ - dev->dev_attrib.max_unmap_block_desc_count = 1; - dev->dev_attrib.unmap_granularity = - q->limits.discard_granularity >> 9; - dev->dev_attrib.unmap_granularity_alignment = - q->limits.discard_alignment; - dev->dev_attrib.unmap_zeroes_data = - q->limits.discard_zeroes_data; - + if (target_configure_unmap_from_queue(&dev->dev_attrib, q, + dev->dev_attrib.hw_block_size)) pr_debug("IBLOCK: BLOCK Discard support available," - " disabled by default\n"); - } + " disabled by default\n"); + /* * Enable write same emulation for IBLOCK and use 0xFFFF as * the smaller WRITE_SAME(10) only has a two-byte block count. @@ -427,9 +409,13 @@ iblock_do_unmap(struct se_cmd *cmd, void *priv, sector_t lba, sector_t nolb) { struct block_device *bdev = priv; + struct se_device *dev = cmd->se_dev; int ret; - ret = blkdev_issue_discard(bdev, lba, nolb, GFP_KERNEL, 0); + ret = blkdev_issue_discard(bdev, + target_to_linux_sector(dev, lba), + target_to_linux_sector(dev, nolb), + GFP_KERNEL, 0); if (ret < 0) { pr_err("blkdev_issue_discard() failed: %d\n", ret); return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; @@ -469,8 +455,10 @@ iblock_execute_write_same(struct se_cmd *cmd) struct scatterlist *sg; struct bio *bio; struct bio_list list; - sector_t block_lba = cmd->t_task_lba; - sector_t sectors = sbc_get_write_same_sectors(cmd); + struct se_device *dev = cmd->se_dev; + sector_t block_lba = target_to_linux_sector(dev, cmd->t_task_lba); + sector_t sectors = target_to_linux_sector(dev, + sbc_get_write_same_sectors(cmd)); if (cmd->prot_op) { pr_err("WRITE_SAME: Protection information with IBLOCK" @@ -684,12 +672,12 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, enum dma_data_direction data_direction) { struct se_device *dev = cmd->se_dev; + sector_t block_lba = target_to_linux_sector(dev, cmd->t_task_lba); struct iblock_req *ibr; struct bio *bio, *bio_start; struct bio_list list; struct scatterlist *sg; u32 sg_num = sgl_nents; - sector_t block_lba; unsigned bio_cnt; int rw = 0; int i; @@ -715,24 +703,6 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, rw = READ; } - /* - * Convert the blocksize advertised to the initiator to the 512 byte - * units unconditionally used by the Linux block layer. - */ - if (dev->dev_attrib.block_size == 4096) - block_lba = (cmd->t_task_lba << 3); - else if (dev->dev_attrib.block_size == 2048) - block_lba = (cmd->t_task_lba << 2); - else if (dev->dev_attrib.block_size == 1024) - block_lba = (cmd->t_task_lba << 1); - else if (dev->dev_attrib.block_size == 512) - block_lba = cmd->t_task_lba; - else { - pr_err("Unsupported SCSI -> BLOCK LBA conversion:" - " %u\n", dev->dev_attrib.block_size); - return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; - } - ibr = kzalloc(sizeof(struct iblock_req), GFP_KERNEL); if (!ibr) goto fail; diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h index 4d5c23a10a08..3905c1c7ab75 100644 --- a/include/target/target_core_backend.h +++ b/include/target/target_core_backend.h @@ -143,4 +143,8 @@ int se_dev_set_max_sectors(struct se_device *, u32); int se_dev_set_optimal_sectors(struct se_device *, u32); int se_dev_set_block_size(struct se_device *, u32); +sector_t target_to_linux_sector(struct se_device *dev, sector_t lb); +bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib, + struct request_queue *q, int block_size); + #endif /* TARGET_CORE_BACKEND_H */ From patchwork Mon Apr 2 14:28:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Fix race with SCF_SEND_DELAYED_TAS handling From: Konstantin Khorenko X-Patchwork-Id: 7981 Message-Id: <201804021428.w32ESQJs024278@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:26 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit d718ed42df49f0908160da35ff091f70b75595c0 Author: Nicholas Bellinger Date: Mon Apr 2 17:28:25 2018 +0300 ms/target: Fix race with SCF_SEND_DELAYED_TAS handling ML: 310d3d314be7f0a84011ebdc4bdccbcae9755a87 This patch fixes a race between setting of SCF_SEND_DELAYED_TAS in transport_send_task_abort(), and check of the same bit in transport_check_aborted_status(). It adds a __transport_check_aborted_status() version that is used by target_execute_cmd() when se_cmd->t_state_lock is held, and a transport_check_aborted_status() wrapper for all other existing callers. Also, it handles the case where the check happens before transport_send_task_abort() gets called. For this, go ahead and set SCF_SEND_DELAYED_TAS early when necessary, and have transport_send_task_abort() send the abort. Cc: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_transport.c | 49 +++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 5bc36e428cf1..8ae6748831f3 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1779,19 +1779,21 @@ static bool target_handle_task_attr(struct se_cmd *cmd) return true; } +static int __transport_check_aborted_status(struct se_cmd *, int); + void target_execute_cmd(struct se_cmd *cmd) { - /* - * If the received CDB has aleady been aborted stop processing it here. - */ - if (transport_check_aborted_status(cmd, 1)) - return; - /* * Determine if frontend context caller is requesting the stopping of * this command for frontend exceptions. + * + * If the received CDB has aleady been aborted stop processing it here. */ spin_lock_irq(&cmd->t_state_lock); + if (__transport_check_aborted_status(cmd, 1)) { + spin_unlock_irq(&cmd->t_state_lock); + return; + } if (cmd->transport_state & CMD_T_STOP) { pr_debug("%s:%d CMD_T_STOP for ITT: 0x%08x\n", __func__, __LINE__, @@ -2852,17 +2854,24 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd, } EXPORT_SYMBOL(transport_send_check_condition_and_sense); -int transport_check_aborted_status(struct se_cmd *cmd, int send_status) +static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status) + __releases(&cmd->t_state_lock) + __acquires(&cmd->t_state_lock) { + assert_spin_locked(&cmd->t_state_lock); + WARN_ON_ONCE(!irqs_disabled()); + if (!(cmd->transport_state & CMD_T_ABORTED)) return 0; - /* * If cmd has been aborted but either no status is to be sent or it has * already been sent, just return */ - if (!send_status || !(cmd->se_cmd_flags & SCF_SEND_DELAYED_TAS)) + if (!send_status || !(cmd->se_cmd_flags & SCF_SEND_DELAYED_TAS)) { + if (send_status) + cmd->se_cmd_flags |= SCF_SEND_DELAYED_TAS; return 1; + } pr_debug("Sending delayed SAM_STAT_TASK_ABORTED status for CDB: 0x%02x ITT: 0x%08x\n", cmd->t_task_cdb[0], cmd->se_tfo->get_task_tag(cmd)); @@ -2870,10 +2879,24 @@ int transport_check_aborted_status(struct se_cmd *cmd, int send_status) cmd->se_cmd_flags &= ~SCF_SEND_DELAYED_TAS; cmd->scsi_status = SAM_STAT_TASK_ABORTED; trace_target_cmd_complete(cmd); + + spin_unlock_irq(&cmd->t_state_lock); cmd->se_tfo->queue_status(cmd); + spin_lock_irq(&cmd->t_state_lock); return 1; } + +int transport_check_aborted_status(struct se_cmd *cmd, int send_status) +{ + int ret; + + spin_lock_irq(&cmd->t_state_lock); + ret = __transport_check_aborted_status(cmd, send_status); + spin_unlock_irq(&cmd->t_state_lock); + + return ret; +} EXPORT_SYMBOL(transport_check_aborted_status); void transport_send_task_abort(struct se_cmd *cmd) @@ -2896,11 +2919,17 @@ void transport_send_task_abort(struct se_cmd *cmd) */ if (cmd->data_direction == DMA_TO_DEVICE) { if (cmd->se_tfo->write_pending_status(cmd) != 0) { - cmd->transport_state |= CMD_T_ABORTED; + spin_lock_irqsave(&cmd->t_state_lock, flags); + if (cmd->se_cmd_flags & SCF_SEND_DELAYED_TAS) { + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + goto send_abort; + } cmd->se_cmd_flags |= SCF_SEND_DELAYED_TAS; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); return; } } +send_abort: cmd->scsi_status = SAM_STAT_TASK_ABORTED; transport_lun_remove_cmd(cmd); From patchwork Mon Apr 2 14:28:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7,COMMIT] ms/target: fix DPO and FUA bit checks From: Konstantin Khorenko X-Patchwork-Id: 7975 Message-Id: <201804021428.w32ESQna024329@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:26 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit deea49e73219891239924b50190ebcbb486c220f Author: Christoph Hellwig Date: Mon Apr 2 17:28:26 2018 +0300 ms/target: fix DPO and FUA bit checks ML: 814e5b45182f4aaf6c0b0deac7104bc2cba5109e Drivers may override the WCE flag, in which case the DPOFUA flag in MODE SENSE might differ from the check used to reject invalid FUA bits in sbc_check_dpofua. Also now that we reject invalid FUA bits early there is no need to duplicate the same buggy check down in the fileio code. As the DPOFUA flag controls th support for FUA bits on read and write commands as well as DPO key off all the checks off a single helper, and deprecate the emulate_dpo and emulate_fua_read attributs. This fixes various failures in the libiscsi testsuite. Personally I'd prefer to also remove the emulate_fua_write attribute as there is no good reason to disable it, but I'll leave that for a separate discussion. Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_device.c | 30 +++++++----------------------- drivers/target/target_core_file.c | 4 +--- drivers/target/target_core_internal.h | 2 ++ drivers/target/target_core_sbc.c | 5 +++-- drivers/target/target_core_spc.c | 12 ++++++++---- drivers/target/target_core_transport.c | 19 +++++++++++++++++++ include/target/target_core_base.h | 6 ------ 7 files changed, 40 insertions(+), 38 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 81156de492a3..7dfe9641b640 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -794,16 +794,8 @@ EXPORT_SYMBOL(se_dev_set_emulate_model_alias); int se_dev_set_emulate_dpo(struct se_device *dev, int flag) { - if (flag != 0 && flag != 1) { - pr_err("Illegal value %d\n", flag); - return -EINVAL; - } - - if (flag) { - pr_err("dpo_emulated not supported\n"); - return -EINVAL; - } - + printk_once(KERN_WARNING + "ignoring deprecated emulate_dpo attribute\n"); return 0; } EXPORT_SYMBOL(se_dev_set_emulate_dpo); @@ -833,16 +825,8 @@ EXPORT_SYMBOL(se_dev_set_emulate_fua_write); int se_dev_set_emulate_fua_read(struct se_device *dev, int flag) { - if (flag != 0 && flag != 1) { - pr_err("Illegal value %d\n", flag); - return -EINVAL; - } - - if (flag) { - pr_err("ua read emulated not supported\n"); - return -EINVAL; - } - + printk_once(KERN_WARNING + "ignoring deprecated emulate_fua_read attribute\n"); return 0; } EXPORT_SYMBOL(se_dev_set_emulate_fua_read); @@ -1547,9 +1531,9 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) dev->dev_attrib.da_dev = dev; dev->dev_attrib.emulate_model_alias = DA_EMULATE_MODEL_ALIAS; - dev->dev_attrib.emulate_dpo = DA_EMULATE_DPO; - dev->dev_attrib.emulate_fua_write = DA_EMULATE_FUA_WRITE; - dev->dev_attrib.emulate_fua_read = DA_EMULATE_FUA_READ; + dev->dev_attrib.emulate_dpo = 1; + dev->dev_attrib.emulate_fua_write = 1; + dev->dev_attrib.emulate_fua_read = 1; dev->dev_attrib.emulate_write_cache = DA_EMULATE_WRITE_CACHE; dev->dev_attrib.emulate_ua_intlck_ctrl = DA_EMULATE_UA_INTLLCK_CTRL; dev->dev_attrib.emulate_tas = DA_EMULATE_TAS; diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index 5c92a0814e24..4e87701792a8 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -726,9 +726,7 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, * for SCSI WRITEs with Forced Unit Access (FUA) set. * Allow this to happen independent of WCE=0 setting. */ - if (ret > 0 && - dev->dev_attrib.emulate_fua_write > 0 && - (cmd->se_cmd_flags & SCF_FUA)) { + if (ret > 0 && (cmd->se_cmd_flags & SCF_FUA)) { struct fd_dev *fd_dev = FD_DEV(dev); loff_t start = cmd->t_task_lba * dev->dev_attrib.block_size; diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h index 60381db90026..06f372c71a3d 100644 --- a/drivers/target/target_core_internal.h +++ b/drivers/target/target_core_internal.h @@ -78,6 +78,8 @@ int transport_clear_lun_ref(struct se_lun *); void transport_send_task_abort(struct se_cmd *); sense_reason_t target_cmd_size_check(struct se_cmd *cmd, unsigned int size); void target_qf_do_work(struct work_struct *work); +bool target_check_wce(struct se_device *dev); +bool target_check_fua(struct se_device *dev); /* target_core_stat.c */ void target_stat_setup_dev_default_groups(struct se_device *); diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 1448dce5fde8..5578e4d21226 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -714,14 +714,15 @@ static int sbc_check_dpofua(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb) { if (cdb[1] & 0x10) { - if (!dev->dev_attrib.emulate_dpo) { + /* see explanation in spc_emulate_modesense */ + if (!target_check_fua(dev)) { pr_err("Got CDB: 0x%02x with DPO bit set, but device" " does not advertise support for DPO\n", cdb[0]); return -EINVAL; } } if (cdb[1] & 0x8) { - if (!dev->dev_attrib.emulate_fua_write || !se_dev_check_wce(dev)) { + if (!target_check_fua(dev)) { pr_err("Got CDB: 0x%02x with FUA bit set, but device" " does not advertise support for FUA write\n", cdb[0]); diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c index 2dd2887a2598..1206ba77a4d3 100644 --- a/drivers/target/target_core_spc.c +++ b/drivers/target/target_core_spc.c @@ -477,7 +477,7 @@ spc_emulate_evpd_86(struct se_cmd *cmd, unsigned char *buf) buf[5] = 0x07; /* If WriteCache emulation is enabled, set V_SUP */ - if (se_dev_check_wce(dev)) + if (target_check_wce(dev)) buf[6] = 0x01; /* If an LBA map is present set R_SUP */ spin_lock(&cmd->se_dev->t10_alua.lba_map_lock); @@ -896,7 +896,7 @@ static int spc_modesense_caching(struct se_cmd *cmd, u8 pc, u8 *p) if (pc == 1) goto out; - if (se_dev_check_wce(dev)) + if (target_check_wce(dev)) p[2] = 0x04; /* Write Cache Enable */ p[12] = 0x20; /* Disabled Read Ahead */ @@ -1008,8 +1008,12 @@ static sense_reason_t spc_emulate_modesense(struct se_cmd *cmd) (cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY))) spc_modesense_write_protect(&buf[length], type); - if ((se_dev_check_wce(dev)) && - (dev->dev_attrib.emulate_fua_write > 0)) + /* + * SBC only allows us to enable FUA and DPO together. Fortunately + * DPO is explicitly specified as a hint, so a noop is a perfectly + * valid implementation. + */ + if (target_check_fua(dev)) spc_modesense_dpofua(&buf[length], type); ++length; diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 8ae6748831f3..1a6ebd20e3bc 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2998,3 +2998,22 @@ int transport_generic_handle_tmr( return 0; } EXPORT_SYMBOL(transport_generic_handle_tmr); + +bool +target_check_wce(struct se_device *dev) +{ + bool wce = false; + + if (dev->transport->get_write_cache) + wce = dev->transport->get_write_cache(dev); + else if (dev->dev_attrib.emulate_write_cache > 0) + wce = true; + + return wce; +} + +bool +target_check_fua(struct se_device *dev) +{ + return target_check_wce(dev) && dev->dev_attrib.emulate_fua_write > 0; +} diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 55c66cdbb414..3a7bbf7de29f 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -81,12 +81,6 @@ #define DA_MAX_WRITE_SAME_LEN 0 /* Use a model alias based on the configfs backend device name */ #define DA_EMULATE_MODEL_ALIAS 0 -/* Emulation for Direct Page Out */ -#define DA_EMULATE_DPO 0 -/* Emulation for Forced Unit Access WRITEs */ -#define DA_EMULATE_FUA_WRITE 1 -/* Emulation for Forced Unit Access READs */ -#define DA_EMULATE_FUA_READ 0 /* Emulation for WriteCache and SYNCHRONIZE_CACHE */ #define DA_EMULATE_WRITE_CACHE 0 /* Emulation for UNIT ATTENTION Interlock Control */ From patchwork Mon Apr 2 14:28:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Fix LUN_RESET active I/O handling for ACK_KREF From: Konstantin Khorenko X-Patchwork-Id: 7982 Message-Id: <201804021428.w32ESREw024380@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:27 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 77a02567ce7a0c8a7cea3e76a675af745e4d3e0c Author: Nicholas Bellinger Date: Mon Apr 2 17:28:26 2018 +0300 ms/target: Fix LUN_RESET active I/O handling for ACK_KREF ML: febe562c20dfa8f33bee7d419c6b517986a5aa33 This patch fixes a NULL pointer se_cmd->cmd_kref < 0 refcount bug during TMR LUN_RESET with active se_cmd I/O, that can be triggered during se_cmd descriptor shutdown + release via core_tmr_drain_state_list() code. To address this bug, add common __target_check_io_state() helper for ABORT_TASK + LUN_RESET w/ CMD_T_COMPLETE checking, and set CMD_T_ABORTED + obtain ->cmd_kref for both cases ahead of last target_put_sess_cmd() after TFO->aborted_task() -> transport_cmd_finish_abort() callback has completed. It also introduces SCF_ACK_KREF to determine when transport_cmd_finish_abort() needs to drop the second extra reference, ahead of calling target_put_sess_cmd() for the final kref_put(&se_cmd->cmd_kref). It also updates transport_cmd_check_stop() to avoid holding se_cmd->t_state_lock while dropping se_cmd device state via target_remove_from_state_list(), now that core_tmr_drain_state_list() is holding the se_device lock while checking se_cmd state from within TMR logic. Finally, move transport_put_cmd() release of SGL + TMR + extended CDB memory into target_free_cmd_mem() in order to avoid potential resource leaks in TMR ABORT_TASK + LUN_RESET code-paths. Also update target_release_cmd_kref() accordingly. Reviewed-by: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_tmr.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 4fe985ae18d8..e2c96725a7bf 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -177,7 +177,6 @@ void core_tmr_abort_task( cancel_work_sync(&se_cmd->work); transport_wait_for_tasks(se_cmd); - if (se_cmd->se_cmd_flags & SCF_SE_LUN_CMD) atomic_long_inc(&se_cmd->se_lun->lun_stats.aborts); From patchwork Mon Apr 2 14:28:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Fix LUN_RESET active TMR descriptor handling From: Konstantin Khorenko X-Patchwork-Id: 7964 Message-Id: <201804021428.w32ESRXI024430@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:27 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 73d0dab0653d855435df9492b0724e480d5415ab Author: Nicholas Bellinger Date: Mon Apr 2 17:28:27 2018 +0300 ms/target: Fix LUN_RESET active TMR descriptor handling ML: a6d9bb1c9605cd4f44e2d8290dc4d0e88f20292d This patch fixes a NULL pointer se_cmd->cmd_kref < 0 refcount bug during TMR LUN_RESET with active TMRs, triggered during se_cmd + se_tmr_req descriptor shutdown + release via core_tmr_drain_tmr_list(). To address this bug, go ahead and obtain a local kref_get_unless_zero(&se_cmd->cmd_kref) for active I/O to set CMD_T_ABORTED, and transport_wait_for_tasks() followed by the final target_put_sess_cmd() to drop the local ->cmd_kref. Also add two new checks within target_tmr_work() to avoid CMD_T_ABORTED -> TFO->queue_tm_rsp() callbacks ahead of invoking the backend -> fabric put in transport_cmd_check_stop_to_fabric(). For good measure, also change core_tmr_release_req() to use list_del_init() ahead of se_tmr_req memory free. Reviewed-by: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_tmr.c | 22 +++++++++++++++++++++- drivers/target/target_core_transport.c | 17 +++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index e2c96725a7bf..99bc3d4db737 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -71,7 +71,7 @@ void core_tmr_release_req(struct se_tmr_req *tmr) if (dev) { spin_lock_irqsave(&dev->se_tmr_lock, flags); - list_del(&tmr->tmr_list); + list_del_init(&tmr->tmr_list); spin_unlock_irqrestore(&dev->se_tmr_lock, flags); } @@ -202,9 +202,11 @@ static void core_tmr_drain_tmr_list( struct list_head *preempt_and_abort_list) { LIST_HEAD(drain_tmr_list); + struct se_session *sess; struct se_tmr_req *tmr_p, *tmr_pp; struct se_cmd *cmd; unsigned long flags; + bool rc; /* * Release all pending and outgoing TMRs aside from the received * LUN_RESET tmr.. @@ -230,17 +232,31 @@ static void core_tmr_drain_tmr_list( if (target_check_cdb_and_preempt(preempt_and_abort_list, cmd)) continue; + sess = cmd->se_sess; + if (WARN_ON_ONCE(!sess)) + continue; + + spin_lock(&sess->sess_cmd_lock); spin_lock(&cmd->t_state_lock); if (!(cmd->transport_state & CMD_T_ACTIVE)) { spin_unlock(&cmd->t_state_lock); + spin_unlock(&sess->sess_cmd_lock); continue; } if (cmd->t_state == TRANSPORT_ISTATE_PROCESSING) { spin_unlock(&cmd->t_state_lock); + spin_unlock(&sess->sess_cmd_lock); continue; } + cmd->transport_state |= CMD_T_ABORTED; spin_unlock(&cmd->t_state_lock); + rc = kref_get_unless_zero(&cmd->cmd_kref); + spin_unlock(&sess->sess_cmd_lock); + if (!rc) { + printk("LUN_RESET TMR: non-zero kref_get_unless_zero\n"); + continue; + } list_move_tail(&tmr_p->tmr_list, &drain_tmr_list); } spin_unlock_irqrestore(&dev->se_tmr_lock, flags); @@ -254,7 +270,11 @@ static void core_tmr_drain_tmr_list( (preempt_and_abort_list) ? "Preempt" : "", tmr_p, tmr_p->function, tmr_p->response, cmd->t_state); + cancel_work_sync(&cmd->work); + transport_wait_for_tasks(cmd); + transport_cmd_finish_abort(cmd, 1); + target_put_sess_cmd(cmd); } } diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 1a6ebd20e3bc..92dad1cc4034 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2949,8 +2949,17 @@ static void target_tmr_work(struct work_struct *work) struct se_cmd *cmd = container_of(work, struct se_cmd, work); struct se_device *dev = cmd->se_dev; struct se_tmr_req *tmr = cmd->se_tmr_req; + unsigned long flags; int ret; + spin_lock_irqsave(&cmd->t_state_lock, flags); + if (cmd->transport_state & CMD_T_ABORTED) { + tmr->response = TMR_FUNCTION_REJECTED; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + goto check_stop; + } + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + switch (tmr->function) { case TMR_ABORT_TASK: core_tmr_abort_task(dev, tmr, cmd->se_sess); @@ -2978,9 +2987,17 @@ static void target_tmr_work(struct work_struct *work) break; } + spin_lock_irqsave(&cmd->t_state_lock, flags); + if (cmd->transport_state & CMD_T_ABORTED) { + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + goto check_stop; + } cmd->t_state = TRANSPORT_ISTATE_PROCESSING; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + cmd->se_tfo->queue_tm_rsp(cmd); +check_stop: transport_cmd_check_stop_to_fabric(cmd); } From patchwork Mon Apr 2 14:28:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Fix TAS handling for multi-session se_node_acls From: Konstantin Khorenko X-Patchwork-Id: 7979 Message-Id: <201804021428.w32ESSwo024480@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:28 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit e0712c21dd35bcc357848ef9c2388e3ff6e88f42 Author: Nicholas Bellinger Date: Mon Apr 2 17:28:28 2018 +0300 ms/target: Fix TAS handling for multi-session se_node_acls ML: ebde1ca5a908b10312db4ecd7553e3ba039319ab This patch fixes a bug in TMR task aborted status (TAS) handling when multiple sessions are connected to the same target WWPN endpoint and se_node_acl descriptor, resulting in TASK_ABORTED status to not be generated for aborted se_cmds on the remote port. This is due to core_tmr_handle_tas_abort() incorrectly comparing se_node_acl instead of se_session, for which the multi-session case is expected to be sharing the same se_node_acl. Instead, go ahead and update core_tmr_handle_tas_abort() to compare tmr_sess + cmd->se_sess in order to determine if the LUN_RESET was received on a different I_T nexus, and TASK_ABORTED status response needs to be generated. Reviewed-by: Christoph Hellwig Cc: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_tmr.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 99bc3d4db737..8f935fb2734d 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -79,7 +79,7 @@ void core_tmr_release_req(struct se_tmr_req *tmr) } static void core_tmr_handle_tas_abort( - struct se_node_acl *tmr_nacl, + struct se_session *tmr_sess, struct se_cmd *cmd, int tas) { @@ -87,7 +87,7 @@ static void core_tmr_handle_tas_abort( /* * TASK ABORTED status (TAS) bit support */ - if ((tmr_nacl && (tmr_nacl != cmd->se_sess->se_node_acl)) && tas) { + if (tmr_sess && tmr_sess != cmd->se_sess && tas) { remove = false; transport_send_task_abort(cmd); } @@ -281,7 +281,7 @@ static void core_tmr_drain_tmr_list( static void core_tmr_drain_state_list( struct se_device *dev, struct se_cmd *prout_cmd, - struct se_node_acl *tmr_nacl, + struct se_session *tmr_sess, int tas, struct list_head *preempt_and_abort_list) { @@ -372,7 +372,7 @@ static void core_tmr_drain_state_list( cancel_work_sync(&cmd->work); transport_wait_for_tasks(cmd); - core_tmr_handle_tas_abort(tmr_nacl, cmd, tas); + core_tmr_handle_tas_abort(tmr_sess, cmd, tas); target_put_sess_cmd(cmd); } } @@ -385,6 +385,7 @@ int core_tmr_lun_reset( { struct se_node_acl *tmr_nacl = NULL; struct se_portal_group *tmr_tpg = NULL; + struct se_session *tmr_sess = NULL; int tas; /* * TASK_ABORTED status bit, this is configurable via ConfigFS @@ -403,8 +404,9 @@ int core_tmr_lun_reset( * or struct se_device passthrough.. */ if (tmr && tmr->task_cmd && tmr->task_cmd->se_sess) { - tmr_nacl = tmr->task_cmd->se_sess->se_node_acl; - tmr_tpg = tmr->task_cmd->se_sess->se_tpg; + tmr_sess = tmr->task_cmd->se_sess; + tmr_nacl = tmr_sess->se_node_acl; + tmr_tpg = tmr_sess->se_tpg; if (tmr_nacl && tmr_tpg) { pr_debug("LUN_RESET: TMR caller fabric: %s" " initiator port %s\n", @@ -417,7 +419,7 @@ int core_tmr_lun_reset( dev->transport->name, tas); core_tmr_drain_tmr_list(dev, tmr, preempt_and_abort_list); - core_tmr_drain_state_list(dev, prout_cmd, tmr_nacl, tas, + core_tmr_drain_state_list(dev, prout_cmd, tmr_sess, tas, preempt_and_abort_list); /* From patchwork Mon Apr 2 14:28:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Fix remote-port TMR ABORT + se_cmd fabric stop From: Konstantin Khorenko X-Patchwork-Id: 7970 Message-Id: <201804021428.w32ESSHs024530@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:28 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit d0ec62ded23e32338ee4687a62750f484bc4a77a Author: Nicholas Bellinger Date: Mon Apr 2 17:28:28 2018 +0300 ms/target: Fix remote-port TMR ABORT + se_cmd fabric stop ML: 0f4a943168f31d29a1701908931acaba518b131a To address the bug where fabric driver level shutdown of se_cmd occurs at the same time when TMR CMD_T_ABORTED is happening resulting in a -1 ->cmd_kref, this patch adds a CMD_T_FABRIC_STOP bit that is used to determine when TMR + driver I_T nexus shutdown is happening concurrently. It changes target_sess_cmd_list_set_waiting() to obtain se_cmd->cmd_kref + set CMD_T_FABRIC_STOP, and drop local reference in target_wait_for_sess_cmds() and invoke extra target_put_sess_cmd() during Task Aborted Status (TAS) when necessary. Also, it adds a new target_wait_free_cmd() wrapper around transport_wait_for_tasks() for the special case within transport_generic_free_cmd() to set CMD_T_FABRIC_STOP, and is now aware of CMD_T_ABORTED + CMD_T_TAS status bits to know when an extra transport_put_cmd() during TAS is required. Note transport_generic_free_cmd() is expected to block on cmd->cmd_wait_comp in order to follow what iscsi-target expects during iscsi_conn context se_cmd shutdown. Cc: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_tmr.c | 57 +++++++++---- drivers/target/target_core_transport.c | 150 +++++++++++++++++++++++++-------- include/target/target_core_base.h | 2 + 3 files changed, 154 insertions(+), 55 deletions(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 8f935fb2734d..956de70539c4 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -78,16 +78,18 @@ void core_tmr_release_req(struct se_tmr_req *tmr) kfree(tmr); } -static void core_tmr_handle_tas_abort( - struct se_session *tmr_sess, - struct se_cmd *cmd, - int tas) +static void core_tmr_handle_tas_abort(struct se_cmd *cmd, int tas) { - bool remove = true; + unsigned long flags; + bool remove = true, send_tas; /* * TASK ABORTED status (TAS) bit support */ - if (tmr_sess && tmr_sess != cmd->se_sess && tas) { + spin_lock_irqsave(&cmd->t_state_lock, flags); + send_tas = (cmd->transport_state & CMD_T_TAS); + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + + if (send_tas) { remove = false; transport_send_task_abort(cmd); } @@ -110,31 +112,42 @@ static int target_check_cdb_and_preempt(struct list_head *list, return 1; } -static bool __target_check_io_state(struct se_cmd *se_cmd) +static bool __target_check_io_state(struct se_cmd *se_cmd, + struct se_session *tmr_sess, int tas) { struct se_session *sess = se_cmd->se_sess; + int ref_tag = se_cmd->se_tfo->get_task_tag(se_cmd); assert_spin_locked(&sess->sess_cmd_lock); WARN_ON_ONCE(!irqs_disabled()); /* * If command already reached CMD_T_COMPLETE state within - * target_complete_cmd(), this se_cmd has been passed to - * fabric driver and will not be aborted. + * target_complete_cmd() or CMD_T_FABRIC_STOP due to shutdown, + * this se_cmd has been passed to fabric driver and will + * not be aborted. * * Otherwise, obtain a local se_cmd->cmd_kref now for TMR * ABORT_TASK + LUN_RESET for CMD_T_ABORTED processing as * long as se_cmd->cmd_kref is still active unless zero. */ spin_lock(&se_cmd->t_state_lock); - if (se_cmd->transport_state & CMD_T_COMPLETE) { - int ref_tag = se_cmd->se_tfo->get_task_tag(se_cmd); - - pr_debug("Attempted to abort io tag: %u already complete," + if (se_cmd->transport_state & (CMD_T_COMPLETE | CMD_T_FABRIC_STOP)) { + pr_debug("Attempted to abort io tag: %u already complete or" + " fabric stop, skipping\n", ref_tag); + spin_unlock(&se_cmd->t_state_lock); + return false; + } + if (sess->sess_tearing_down || se_cmd->cmd_wait_set) { + pr_debug("Attempted to abort io tag: %u already shutdown," " skipping\n", ref_tag); spin_unlock(&se_cmd->t_state_lock); return false; } se_cmd->transport_state |= CMD_T_ABORTED; + + if ((tmr_sess != se_cmd->se_sess) && tas) + se_cmd->transport_state |= CMD_T_TAS; + spin_unlock(&se_cmd->t_state_lock); return kref_get_unless_zero(&se_cmd->cmd_kref); @@ -166,7 +179,7 @@ void core_tmr_abort_task( printk("ABORT_TASK: Found referenced %s task_tag: %u\n", se_cmd->se_tfo->get_fabric_name(), ref_tag); - if (!__target_check_io_state(se_cmd)) { + if (!__target_check_io_state(se_cmd, se_sess, 0)) { spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); target_put_sess_cmd(se_cmd); goto out; @@ -238,7 +251,8 @@ static void core_tmr_drain_tmr_list( spin_lock(&sess->sess_cmd_lock); spin_lock(&cmd->t_state_lock); - if (!(cmd->transport_state & CMD_T_ACTIVE)) { + if (!(cmd->transport_state & CMD_T_ACTIVE) || + (cmd->transport_state & CMD_T_FABRIC_STOP)) { spin_unlock(&cmd->t_state_lock); spin_unlock(&sess->sess_cmd_lock); continue; @@ -248,15 +262,22 @@ static void core_tmr_drain_tmr_list( spin_unlock(&sess->sess_cmd_lock); continue; } + if (sess->sess_tearing_down || cmd->cmd_wait_set) { + spin_unlock(&cmd->t_state_lock); + spin_unlock(&sess->sess_cmd_lock); + continue; + } cmd->transport_state |= CMD_T_ABORTED; spin_unlock(&cmd->t_state_lock); rc = kref_get_unless_zero(&cmd->cmd_kref); - spin_unlock(&sess->sess_cmd_lock); if (!rc) { printk("LUN_RESET TMR: non-zero kref_get_unless_zero\n"); + spin_unlock(&sess->sess_cmd_lock); continue; } + spin_unlock(&sess->sess_cmd_lock); + list_move_tail(&tmr_p->tmr_list, &drain_tmr_list); } spin_unlock_irqrestore(&dev->se_tmr_lock, flags); @@ -333,7 +354,7 @@ static void core_tmr_drain_state_list( continue; spin_lock(&sess->sess_cmd_lock); - rc = __target_check_io_state(cmd); + rc = __target_check_io_state(cmd, tmr_sess, tas); spin_unlock(&sess->sess_cmd_lock); if (!rc) continue; @@ -372,7 +393,7 @@ static void core_tmr_drain_state_list( cancel_work_sync(&cmd->work); transport_wait_for_tasks(cmd); - core_tmr_handle_tas_abort(tmr_sess, cmd, tas); + core_tmr_handle_tas_abort(cmd, tas); target_put_sess_cmd(cmd); } } diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 92dad1cc4034..dc10e162ef93 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2409,18 +2409,33 @@ static void transport_write_pending_qf(struct se_cmd *cmd) } } +static bool +__transport_wait_for_tasks(struct se_cmd *, bool, bool *, bool *, + unsigned long *flags); + +static void target_wait_free_cmd(struct se_cmd *cmd, bool *aborted, bool *tas) +{ + unsigned long flags; + + spin_lock_irqsave(&cmd->t_state_lock, flags); + __transport_wait_for_tasks(cmd, true, aborted, tas, &flags); + spin_unlock_irqrestore(&cmd->t_state_lock, flags); +} + int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks) { int ret = 0; + bool aborted = false, tas = false; if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) { if (wait_for_tasks && (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) - transport_wait_for_tasks(cmd); + target_wait_free_cmd(cmd, &aborted, &tas); - ret = transport_put_cmd(cmd); + if (!aborted || tas) + ret = transport_put_cmd(cmd); } else { if (wait_for_tasks) - transport_wait_for_tasks(cmd); + target_wait_free_cmd(cmd, &aborted, &tas); /* * Handle WRITE failure case where transport_generic_new_cmd() * has already added se_cmd to state_list, but fabric has @@ -2432,7 +2447,22 @@ int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks) if (cmd->se_lun) transport_lun_remove_cmd(cmd); - ret = transport_put_cmd(cmd); + if (!aborted || tas) + ret = transport_put_cmd(cmd); + } + /* + * If the task has been internally aborted due to TMR ABORT_TASK + * or LUN_RESET, target_core_tmr.c is responsible for performing + * the remaining calls to target_put_sess_cmd(), and not the + * callers of this function. + */ + if (aborted) { + int ref_tag = cmd->se_tfo->get_task_tag(cmd); + + pr_debug("Detected CMD_T_ABORTED for ITT: %u\n", ref_tag); + wait_for_completion(&cmd->cmd_wait_comp); + cmd->se_tfo->release_cmd(cmd); + ret = 1; } return ret; } @@ -2489,6 +2519,7 @@ static void target_release_cmd_kref(struct kref *kref) struct se_cmd *se_cmd = container_of(kref, struct se_cmd, cmd_kref); struct se_session *se_sess = se_cmd->se_sess; unsigned long flags; + bool fabric_stop; spin_lock_irqsave(&se_sess->sess_cmd_lock, flags); if (list_empty(&se_cmd->se_cmd_list)) { @@ -2497,13 +2528,19 @@ static void target_release_cmd_kref(struct kref *kref) se_cmd->se_tfo->release_cmd(se_cmd); return; } - if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) { + + spin_lock(&se_cmd->t_state_lock); + fabric_stop = (se_cmd->transport_state & CMD_T_FABRIC_STOP); + spin_unlock(&se_cmd->t_state_lock); + + if (se_cmd->cmd_wait_set || fabric_stop) { + list_del_init(&se_cmd->se_cmd_list); spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); target_free_cmd_mem(se_cmd); complete(&se_cmd->cmd_wait_comp); return; } - list_del(&se_cmd->se_cmd_list); + list_del_init(&se_cmd->se_cmd_list); spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); target_free_cmd_mem(se_cmd); @@ -2535,6 +2572,7 @@ void target_sess_cmd_list_set_waiting(struct se_session *se_sess) { struct se_cmd *se_cmd; unsigned long flags; + int rc; spin_lock_irqsave(&se_sess->sess_cmd_lock, flags); if (se_sess->sess_tearing_down) { @@ -2544,8 +2582,15 @@ void target_sess_cmd_list_set_waiting(struct se_session *se_sess) se_sess->sess_tearing_down = 1; list_splice_init(&se_sess->sess_cmd_list, &se_sess->sess_wait_list); - list_for_each_entry(se_cmd, &se_sess->sess_wait_list, se_cmd_list) - se_cmd->cmd_wait_set = 1; + list_for_each_entry(se_cmd, &se_sess->sess_wait_list, se_cmd_list) { + rc = kref_get_unless_zero(&se_cmd->cmd_kref); + if (rc) { + se_cmd->cmd_wait_set = 1; + spin_lock(&se_cmd->t_state_lock); + se_cmd->transport_state |= CMD_T_FABRIC_STOP; + spin_unlock(&se_cmd->t_state_lock); + } + } spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); } @@ -2558,15 +2603,25 @@ void target_wait_for_sess_cmds(struct se_session *se_sess) { struct se_cmd *se_cmd, *tmp_cmd; unsigned long flags; + bool tas; list_for_each_entry_safe(se_cmd, tmp_cmd, &se_sess->sess_wait_list, se_cmd_list) { - list_del(&se_cmd->se_cmd_list); + list_del_init(&se_cmd->se_cmd_list); pr_debug("Waiting for se_cmd: %p t_state: %d, fabric state:" " %d\n", se_cmd, se_cmd->t_state, se_cmd->se_tfo->get_cmd_state(se_cmd)); + spin_lock_irqsave(&se_cmd->t_state_lock, flags); + tas = (se_cmd->transport_state & CMD_T_TAS); + spin_unlock_irqrestore(&se_cmd->t_state_lock, flags); + + if (!target_put_sess_cmd(se_cmd)) { + if (tas) + target_put_sess_cmd(se_cmd); + } + wait_for_completion(&se_cmd->cmd_wait_comp); pr_debug("After cmd_wait_comp: se_cmd: %p t_state: %d" " fabric state: %d\n", se_cmd, se_cmd->t_state, @@ -2609,56 +2664,77 @@ int transport_clear_lun_ref(struct se_lun *lun) return 0; } -/** - * transport_wait_for_tasks - wait for completion to occur - * @cmd: command to wait - * - * Called from frontend fabric context to wait for storage engine - * to pause and/or release frontend generated struct se_cmd. - */ -bool transport_wait_for_tasks(struct se_cmd *cmd) +static bool +__transport_wait_for_tasks(struct se_cmd *cmd, bool fabric_stop, + bool *aborted, bool *tas, unsigned long *flags) + __releases(&cmd->t_state_lock) + __acquires(&cmd->t_state_lock) { - unsigned long flags; - spin_lock_irqsave(&cmd->t_state_lock, flags); + assert_spin_locked(&cmd->t_state_lock); + WARN_ON_ONCE(!irqs_disabled()); + + if (fabric_stop) + cmd->transport_state |= CMD_T_FABRIC_STOP; + + if (cmd->transport_state & CMD_T_ABORTED) + *aborted = true; + + if (cmd->transport_state & CMD_T_TAS) + *tas = true; + if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD) && - !(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) { - spin_unlock_irqrestore(&cmd->t_state_lock, flags); + !(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) return false; - } if (!(cmd->se_cmd_flags & SCF_SUPPORTED_SAM_OPCODE) && - !(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) { - spin_unlock_irqrestore(&cmd->t_state_lock, flags); + !(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) return false; - } - if (!(cmd->transport_state & CMD_T_ACTIVE)) { - spin_unlock_irqrestore(&cmd->t_state_lock, flags); + if (!(cmd->transport_state & CMD_T_ACTIVE)) + return false; + + if (fabric_stop && *aborted) return false; - } cmd->transport_state |= CMD_T_STOP; - pr_debug("wait_for_tasks: Stopping %p ITT: 0x%08x" - " i_state: %d, t_state: %d, CMD_T_STOP\n", - cmd, cmd->se_tfo->get_task_tag(cmd), - cmd->se_tfo->get_cmd_state(cmd), cmd->t_state); + pr_debug("wait_for_tasks: Stopping %p ITT: 0x%08x i_state: %d," + " t_state: %d, CMD_T_STOP\n", + cmd, cmd->se_tfo->get_task_tag(cmd), + cmd->se_tfo->get_cmd_state(cmd), cmd->t_state); - spin_unlock_irqrestore(&cmd->t_state_lock, flags); + spin_unlock_irqrestore(&cmd->t_state_lock, *flags); wait_for_completion(&cmd->t_transport_stop_comp); - spin_lock_irqsave(&cmd->t_state_lock, flags); + spin_lock_irqsave(&cmd->t_state_lock, *flags); cmd->transport_state &= ~(CMD_T_ACTIVE | CMD_T_STOP); - pr_debug("wait_for_tasks: Stopped wait_for_completion(" - "&cmd->t_transport_stop_comp) for ITT: 0x%08x\n", + pr_debug("wait_for_tasks: Stopped wait_for_completion(&cmd->" + "t_transport_stop_comp) for ITT: 0x%08x\n", cmd->se_tfo->get_task_tag(cmd)); + return true; +} + +/** + * transport_wait_for_tasks - wait for completion to occur + * @cmd: command to wait + * + * Called from frontend fabric context to wait for storage engine + * to pause and/or release frontend generated struct se_cmd. + */ +bool transport_wait_for_tasks(struct se_cmd *cmd) +{ + unsigned long flags; + bool ret, aborted = false, tas = false; + + spin_lock_irqsave(&cmd->t_state_lock, flags); + ret = __transport_wait_for_tasks(cmd, false, &aborted, &tas, &flags); spin_unlock_irqrestore(&cmd->t_state_lock, flags); - return true; + return ret; } EXPORT_SYMBOL(transport_wait_for_tasks); diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 3a7bbf7de29f..00ec289bccaa 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -533,6 +533,8 @@ struct se_cmd { #define CMD_T_DEV_ACTIVE (1 << 7) #define CMD_T_REQUEST_STOP (1 << 8) #define CMD_T_BUSY (1 << 9) +#define CMD_T_TAS (1 << 10) +#define CMD_T_FABRIC_STOP (1 << 11) #define CMD_T_STANDBY (1 << 31) spinlock_t t_state_lock; struct completion t_transport_stop_comp; From patchwork Mon Apr 2 14:28:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Drop incorrect ABORT_TASK put for completed commands From: Konstantin Khorenko X-Patchwork-Id: 7963 Message-Id: <201804021428.w32ESTIn024580@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:29 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 4e6e822b3f606f534eaa841511a2c055e89c32a0 Author: Nicholas Bellinger Date: Mon Apr 2 17:28:29 2018 +0300 ms/target: Drop incorrect ABORT_TASK put for completed commands ML: 7f54ab5ff52fb0b91569bc69c4a6bc5cac1b768d This patch fixes a recent ABORT_TASK regression associated with commit febe562c, where a left-over target_put_sess_cmd() would still be called when __target_check_io_state() detected a command has already been completed, and explicit ABORT must be avoided. Note commit febe562c dropped the local kref_get_unless_zero() check in core_tmr_abort_task(), but did not drop this extra corresponding target_put_sess_cmd() in the failure path. So go ahead and drop this now bogus target_put_sess_cmd(), and avoid this potential use-after-free. Reported-by: Dan Lane Cc: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Cc: stable@vger.kernel.org # 3.14+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_tmr.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 956de70539c4..78562d93d02b 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -181,7 +181,6 @@ void core_tmr_abort_task( if (!__target_check_io_state(se_cmd, se_sess, 0)) { spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); - target_put_sess_cmd(se_cmd); goto out; } list_del_init(&se_cmd->se_cmd_list); From patchwork Mon Apr 2 14:28:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/target: Fix target_release_cmd_kref shutdown comp leak From: Konstantin Khorenko X-Patchwork-Id: 7972 Message-Id: <201804021428.w32ESTqv024630@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:29 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 60377be1b944ab89acaa627e4a19327c810151f9 Author: Himanshu Madhani Date: Mon Apr 2 17:28:29 2018 +0300 ms/target: Fix target_release_cmd_kref shutdown comp leak ML: 5e47f1985d7107331c3f64fb3ec83d66fd73577e This patch fixes an active I/O shutdown bug for fabric drivers using target_wait_for_sess_cmds(), where se_cmd descriptor shutdown would result in hung tasks waiting indefinitely for se_cmd->cmd_wait_comp to complete(). To address this bug, drop the incorrect list_del_init() usage in target_wait_for_sess_cmds() and always complete() during se_cmd target_release_cmd_kref() put, in order to let caller invoke the final fabric release callback into se_cmd->se_tfo->release_cmd() code. Reported-by: Himanshu Madhani Tested-by: Himanshu Madhani Signed-off-by: Himanshu Madhani Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_transport.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index dc10e162ef93..9b187cf1c5b0 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2607,8 +2607,6 @@ void target_wait_for_sess_cmds(struct se_session *se_sess) list_for_each_entry_safe(se_cmd, tmp_cmd, &se_sess->sess_wait_list, se_cmd_list) { - list_del_init(&se_cmd->se_cmd_list); - pr_debug("Waiting for se_cmd: %p t_state: %d, fabric state:" " %d\n", se_cmd, se_cmd->t_state, se_cmd->se_tfo->get_cmd_state(se_cmd)); From patchwork Mon Apr 2 14:28:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7, COMMIT] ms/iscsi-target: fix memory leak in iscsit_setup_text_cmd() From: Konstantin Khorenko X-Patchwork-Id: 7969 Message-Id: <201804021428.w32ESUh0024680@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:30 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 36ec28ad5177e3c5cbfa8a86f625d4f0760c85c5 Author: Varun Prakash Date: Mon Apr 2 17:28:30 2018 +0300 ms/iscsi-target: fix memory leak in iscsit_setup_text_cmd() ML: ea8dc5b4cd2195ee582cae28afa4164c6dea1738 On receiving text request iscsi-target allocates buffer for payload in iscsit_handle_text_cmd() and assigns buffer pointer to cmd->text_in_ptr, this buffer is currently freed in iscsit_release_cmd(), if iscsi-target sets 'C' bit in text response then it will receive another text request from the initiator with ttt != 0xffffffff in this case iscsi-target will find cmd using itt and call iscsit_setup_text_cmd() which will set cmd->text_in_ptr to NULL without freeing previously allocated buffer. This patch fixes this issue by calling kfree(cmd->text_in_ptr) in iscsit_setup_text_cmd() before assigning NULL to it. For the first text request cmd->text_in_ptr is NULL as cmd is memset to 0 in iscsit_allocate_cmd(). Signed-off-by: Varun Prakash Cc: # 4.0+ Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/iscsi/iscsi_target.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index ea8573e9e129..aeba04aee4bb 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -2164,6 +2164,7 @@ iscsit_setup_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, cmd->cmd_sn = be32_to_cpu(hdr->cmdsn); cmd->exp_stat_sn = be32_to_cpu(hdr->exp_statsn); cmd->data_direction = DMA_NONE; + kfree(cmd->text_in_ptr); cmd->text_in_ptr = NULL; return 0; From patchwork Mon Apr 2 14:28:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7,COMMIT] ms/iscsi-target: fix invalid flags in text response From: Konstantin Khorenko X-Patchwork-Id: 7974 Message-Id: <201804021428.w32ESUHg024730@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:30 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit 8e7148d94eff27bd1eccc825c94df93d0ea43f52 Author: Varun Prakash Date: Mon Apr 2 17:28:30 2018 +0300 ms/iscsi-target: fix invalid flags in text response ML: 310d40a973c560a24c79f84cb5f16dc540a05686 In case of multiple text responses iscsi-target sets both 'F' and 'C' bit for the final text response pdu, this issue happens because hdr->flags is not zeroed out before ORing with 'F' bit. This patch removes the | operator to fix this issue. Signed-off-by: Varun Prakash Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/iscsi/iscsi_target.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index aeba04aee4bb..2ec68f26e4d5 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -3490,9 +3490,9 @@ iscsit_build_text_rsp(struct iscsi_cmd *cmd, struct iscsi_conn *conn, return text_length; if (completed) { - hdr->flags |= ISCSI_FLAG_CMD_FINAL; + hdr->flags = ISCSI_FLAG_CMD_FINAL; } else { - hdr->flags |= ISCSI_FLAG_TEXT_CONTINUE; + hdr->flags = ISCSI_FLAG_TEXT_CONTINUE; cmd->read_data_done += text_length; if (cmd->targ_xfer_tag == 0xFFFFFFFF) cmd->targ_xfer_tag = session_get_next_ttt(conn->sess); From patchwork Mon Apr 2 14:28:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RHEL7,COMMIT] ms/target: Fix max_unmap_lba_count calc overflow From: Konstantin Khorenko X-Patchwork-Id: 7976 Message-Id: <201804021428.w32ESV47024780@finist_ce7.work> To: Andrei Vagin Cc: OpenVZ devel Date: Mon, 2 Apr 2018 17:28:31 +0300 The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.46.2 ------> commit bab559039e31f0ed70e17b39c93a11e320bce84a Author: Mike Christie Date: Mon Apr 2 17:28:31 2018 +0300 ms/target: Fix max_unmap_lba_count calc overflow ML: ea263c7fada4af8ec7fe5fcfd6e7d7705a89351b max_discard_sectors only 32bits, and some non scsi backend devices will set this to the max 0xffffffff, so we can end up overflowing during the max_unmap_lba_count calculation. This fixes a regression caused by my patch: commit 8a9ebe717a133ba7bc90b06047f43cc6b8bcb8b3 Author: Mike Christie Date: Mon Jan 18 14:09:27 2016 -0600 target: Fix WRITE_SAME/DISCARD conversion to linux 512b sectors which can result in extra discards being sent to due the overflow causing max_unmap_lba_count to be smaller than what the backing device can actually support. Signed-off-by: Mike Christie Reviewed-by: Bart Van Assche Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger Signed-off-by: Andrei Vagin --- drivers/target/target_core_device.c | 8 +++++--- drivers/target/target_core_file.c | 3 +-- drivers/target/target_core_iblock.c | 3 +-- include/target/target_core_backend.h | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 7dfe9641b640..cb17aeb1d7ac 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -1573,13 +1573,15 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) * in ATA and we need to set TPE=1 */ bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib, - struct request_queue *q, int block_size) + struct request_queue *q) { + int block_size = queue_logical_block_size(q); + if (!blk_queue_discard(q)) return false; - attrib->max_unmap_lba_count = (q->limits.max_discard_sectors << 9) / - block_size; + attrib->max_unmap_lba_count = + q->limits.max_discard_sectors >> (ilog2(block_size) - 9); /* * Currently hardcoded to 1 in Linux/SCSI code.. */ diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index 4e87701792a8..ef5ea7105248 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -166,8 +166,7 @@ static int fd_configure_device(struct se_device *dev) dev_size, div_u64(dev_size, fd_dev->fd_block_size), fd_dev->fd_block_size); - if (target_configure_unmap_from_queue(&dev->dev_attrib, q, - fd_dev->fd_block_size)) + if (target_configure_unmap_from_queue(&dev->dev_attrib, q)) pr_debug("IFILE: BLOCK Discard support available," " disabled by default\n"); /* diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index 00bfe975ac72..55be98bb964b 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -127,8 +127,7 @@ static int iblock_configure_device(struct se_device *dev) dev->dev_attrib.hw_max_sectors = queue_max_hw_sectors(q); dev->dev_attrib.hw_queue_depth = q->nr_requests; - if (target_configure_unmap_from_queue(&dev->dev_attrib, q, - dev->dev_attrib.hw_block_size)) + if (target_configure_unmap_from_queue(&dev->dev_attrib, q)) pr_debug("IBLOCK: BLOCK Discard support available," " disabled by default\n"); diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h index 3905c1c7ab75..5e1124d24587 100644 --- a/include/target/target_core_backend.h +++ b/include/target/target_core_backend.h @@ -145,6 +145,6 @@ int se_dev_set_block_size(struct se_device *, u32); sector_t target_to_linux_sector(struct se_device *dev, sector_t lb); bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib, - struct request_queue *q, int block_size); + struct request_queue *q); #endif /* TARGET_CORE_BACKEND_H */