Skip to contentSkip to conversation

Patch "target: Fix race with SCF_SEND_DELAYED_TAS handling" has been added to the 4.4-stable tree

PATCH [email protected] 1 participant 1 message Mar 1, 2016

Subsystems (2)
Stable branch SCSI target
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

target: Fix race with SCF_SEND_DELAYED_TAS handling

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
target-fix-race-with-scf_send_delayed_tas-handling.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 310d3d314be7f0a84011ebdc4bdccbcae9755a87 Mon Sep 17 00:00:00 2001

From: Nicholas Bellinger <[email protected]>
Date: Fri, 5 Feb 2016 14:51:36 -0800
Subject: target: Fix race with SCF_SEND_DELAYED_TAS handling

From: Nicholas Bellinger <[email protected]>

commit 310d3d314be7f0a84011ebdc4bdccbcae9755a87 upstream.

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 <[email protected]>
Cc Himanshu Madhani <[email protected]>
Cc Sagi Grimberg <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Hannes Reinecke <[email protected]>
Cc Andy Grover <[email protected]>
Cc Mike Christie <[email protected]>
Signed-off-by Nicholas Bellinger <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/target/target_core_transport.c |   53 +++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 12 deletions(-)
drivers/target/target_core_transport.c +41 -12
@@ -1849,19 +1849,21 @@ static bool target_handle_task_attr(stru
 	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%08llx\n",
 			__func__, __LINE__, cmd->tag);

@@ -2902,28 +2904,49 @@ transport_send_check_condition_and_sense
 }
 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%08llx\n",
-		 cmd->t_task_cdb[0], cmd->tag);
+	pr_debug("Sending delayed SAM_STAT_TASK_ABORTED status for CDB:"
+		" 0x%02x ITT: 0x%08llx\n", cmd->t_task_cdb[0], cmd->tag);
 
 	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)

@@ -2945,11 +2968,17 @@ void transport_send_task_abort(struct se
 	 */
 	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);

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/qla2xxx-fix-stale-pointer-access.patch queue-4.4/target-fix-remote-port-tmr-abort-se_cmd-fabric-stop.patch queue-4.4/target-fix-lun_reset-active-tmr-descriptor-handling.patch queue-4.4/target-fix-race-with-scf_send_delayed_tas-handling.patch queue-4.4/target-fix-lun_reset-active-i-o-handling-for-ack_kref.patch queue-4.4/target-fix-tas-handling-for-multi-session-se_node_acls.patch