[RHEL7,COMMIT] ms/target: Fix race with SCF_SEND_DELAYED_TAS handling

Submitted by Konstantin Khorenko on April 2, 2018, 2:28 p.m.

Details

Message ID 201804021428.w32ESQJs024278@finist_ce7.work
State New
Series "target: backport bug fixes from the upstream kernel"
Headers show

Commit Message

Konstantin Khorenko April 2, 2018, 2:28 p.m.
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 <nab@linux-iscsi.org>
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 <quinn.tran@qlogic.com>
    Cc: Himanshu Madhani <himanshu.madhani@qlogic.com>
    Cc: Sagi Grimberg <sagig@mellanox.com>
    Cc: Christoph Hellwig <hch@lst.de>
    Cc: Hannes Reinecke <hare@suse.de>
    Cc: Andy Grover <agrover@redhat.com>
    Cc: Mike Christie <mchristi@redhat.com>
    Cc: stable@vger.kernel.org # 3.10+
    Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
    Signed-off-by: Andrei Vagin <avagin@openvz.org>
---
 drivers/target/target_core_transport.c | 49 +++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

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);