All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Quinn Tran <quinn.tran@qlogic.com>,
	Himanshu Madhani <himanshu.madhani@qlogic.com>,
	Sagi Grimberg <sagig@mellanox.com>,
	Christoph Hellwig <hch@lst.de>, Hannes Reinecke <hare@suse.de>,
	Andy Grover <agrover@redhat.com>,
	Mike Christie <mchristi@redhat.com>,
	Nicholas Bellinger <nab@linux-iscsi.org>
Subject: [PATCH 3.14 35/36] target: Fix race with SCF_SEND_DELAYED_TAS handling
Date: Mon,  7 Mar 2016 15:46:18 -0800	[thread overview]
Message-ID: <20160307234605.528238438@linuxfoundation.org> (raw)
In-Reply-To: <20160307234600.344036091@linuxfoundation.org>

3.14-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Nicholas Bellinger <nab@linux-iscsi.org>

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 <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>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


---
 drivers/target/target_core_transport.c |   54 +++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 12 deletions(-)

--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1754,19 +1754,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%08x\n",
 			__func__, __LINE__,
@@ -2895,8 +2897,13 @@ after_reason:
 }
 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;
 
@@ -2904,19 +2911,37 @@ int transport_check_aborted_status(struc
 	 * 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));
+	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));
 
 	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)
@@ -2938,12 +2963,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;
-			smp_mb__after_atomic_inc();
+			spin_unlock_irqrestore(&cmd->t_state_lock, flags);
 			return;
 		}
 	}
+send_abort:
 	cmd->scsi_status = SAM_STAT_TASK_ABORTED;
 
 	transport_lun_remove_cmd(cmd);

  parent reply	other threads:[~2016-03-07 23:49 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-07 23:45 [PATCH 3.14 00/36] 3.14.64-stable review Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 01/36] bio: return EINTR if copying to user space got interrupted Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 02/36] locks: fix unlock when fcntl_setlk races with a close Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 03/36] EDAC, mc_sysfs: Fix freeing bus name Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 04/36] cifs: fix out-of-bounds access in lease parsing Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 05/36] CIFS: Fix SMB2+ interim response processing for read requests Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 06/36] iommu/amd: Fix boot warning when device 00:00.0 is not iommu covered Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 07/36] x86/entry/compat: Add missing CLAC to entry_INT80_32 Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 08/36] drm/ast: Fix incorrect register check for DRAM width Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 09/36] libata: fix HDIO_GET_32BIT ioctl Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 10/36] libata: Align ata_devices id on a cacheline Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 11/36] PM / sleep / x86: Fix crash on graph trace through x86 suspend Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 12/36] Revert "jffs2: Fix lock acquisition order bug in jffs2_write_begin" Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 15/36] ALSA: ctl: Fix ioctls for X32 ABI Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 16/36] ALSA: rawmidi: Fix ioctls " Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 17/36] ALSA: timer: Fix ioctls for " Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 18/36] ALSA: seq: oss: Dont drain at closing a client Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 19/36] ALSA: hdspm: Fix wrong boolean ctl value accesses Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 20/36] ALSA: hdsp: " Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 21/36] ALSA: hdspm: Fix zero-division Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 22/36] ALSA: timer: Fix broken compat timer user status ioctl Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 23/36] usb: chipidea: otg: change workqueue ci_otg as freezable Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 24/36] USB: cp210x: Add ID for Parrot NMEA GPS Flight Recorder Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 25/36] USB: serial: option: add support for Telit LE922 PID 0x1045 Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 26/36] USB: serial: option: add support for Quectel UC20 Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 27/36] MIPS: traps: Fix SIGFPE information leak from `do_ov and `do_trap_or_bp Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 28/36] ubi: Fix out of bounds write in volume update code Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 29/36] target: Fix Task Aborted Status (TAS) handling Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 30/36] target: Add TFO->abort_task for aborted task resources release Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 31/36] target: Fix LUN_RESET active TMR descriptor handling Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 32/36] target: Fix LUN_RESET active I/O handling for ACK_KREF Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 34/36] target: Fix remote-port TMR ABORT + se_cmd fabric stop Greg Kroah-Hartman
2016-03-07 23:46 ` Greg Kroah-Hartman [this message]
2016-03-07 23:46 ` [PATCH 3.14 36/36] target: Fix WRITE_SAME/DISCARD conversion to linux 512b sectors Greg Kroah-Hartman
2016-03-08 11:44 ` [PATCH 3.14 00/36] 3.14.64-stable review Guenter Roeck
2016-03-08 16:20 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160307234605.528238438@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=agrover@redhat.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=himanshu.madhani@qlogic.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchristi@redhat.com \
    --cc=nab@linux-iscsi.org \
    --cc=quinn.tran@qlogic.com \
    --cc=sagig@mellanox.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.