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, Mike Christie <mchristi@redhat.com>,
	Quinn Tran <quinn.tran@qlogic.com>,
	Himanshu Madhani <himanshu.madhani@qlogic.com>,
	Christoph Hellwig <hch@lst.de>, Hannes Reinecke <hare@suse.de>,
	Nicholas Bellinger <nab@linux-iscsi.org>
Subject: [PATCH 3.14 40/46] target: Fix race between iscsi-target connection shutdown + ABORT_TASK
Date: Thu, 18 Aug 2016 15:55:02 +0200	[thread overview]
Message-ID: <20160818135448.401387482@linuxfoundation.org> (raw)
In-Reply-To: <20160818135442.457400364@linuxfoundation.org>

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

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

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

commit 064cdd2d91c2805d788876082f31cc63506f22c3 upstream.

This patch fixes a race in iscsit_release_commands_from_conn() ->
iscsit_free_cmd() -> transport_generic_free_cmd() + wait_for_tasks=1,
where CMD_T_FABRIC_STOP could end up being set after the final
kref_put() is called from core_tmr_abort_task() context.

This results in transport_generic_free_cmd() blocking indefinately
on se_cmd->cmd_wait_comp, because the target_release_cmd_kref()
check for CMD_T_FABRIC_STOP returns false.

To address this bug, make iscsit_release_commands_from_conn()
do list_splice and set CMD_T_FABRIC_STOP early while holding
iscsi_conn->cmd_lock.  Also make iscsit_aborted_task() only
remove iscsi_cmd_t if CMD_T_FABRIC_STOP has not already been
set.

Finally in target_release_cmd_kref(), only honor fabric_stop
if CMD_T_ABORTED has been set.

Cc: Mike Christie <mchristi@redhat.com>
Cc: Quinn Tran <quinn.tran@qlogic.com>
Cc: Himanshu Madhani <himanshu.madhani@qlogic.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Tested-by: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/target/iscsi/iscsi_target.c    |   22 ++++++++++++++++------
 drivers/target/target_core_transport.c |    3 ++-
 2 files changed, 18 insertions(+), 7 deletions(-)

--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -505,7 +505,8 @@ static void iscsit_aborted_task(struct i
 	bool scsi_cmd = (cmd->iscsi_opcode == ISCSI_OP_SCSI_CMD);
 
 	spin_lock_bh(&conn->cmd_lock);
-	if (!list_empty(&cmd->i_conn_node))
+	if (!list_empty(&cmd->i_conn_node) &&
+	    !(cmd->se_cmd.transport_state & CMD_T_FABRIC_STOP))
 		list_del_init(&cmd->i_conn_node);
 	spin_unlock_bh(&conn->cmd_lock);
 
@@ -4160,6 +4161,7 @@ transport_err:
 
 static void iscsit_release_commands_from_conn(struct iscsi_conn *conn)
 {
+	LIST_HEAD(tmp_list);
 	struct iscsi_cmd *cmd = NULL, *cmd_tmp = NULL;
 	struct iscsi_session *sess = conn->sess;
 	/*
@@ -4168,18 +4170,26 @@ static void iscsit_release_commands_from
 	 * has been reset -> returned sleeping pre-handler state.
 	 */
 	spin_lock_bh(&conn->cmd_lock);
-	list_for_each_entry_safe(cmd, cmd_tmp, &conn->conn_cmd_list, i_conn_node) {
+	list_splice_init(&conn->conn_cmd_list, &tmp_list);
 
+	list_for_each_entry(cmd, &tmp_list, i_conn_node) {
+		struct se_cmd *se_cmd = &cmd->se_cmd;
+
+		if (se_cmd->se_tfo != NULL) {
+			spin_lock(&se_cmd->t_state_lock);
+			se_cmd->transport_state |= CMD_T_FABRIC_STOP;
+			spin_unlock(&se_cmd->t_state_lock);
+		}
+	}
+	spin_unlock_bh(&conn->cmd_lock);
+
+	list_for_each_entry_safe(cmd, cmd_tmp, &tmp_list, i_conn_node) {
 		list_del_init(&cmd->i_conn_node);
-		spin_unlock_bh(&conn->cmd_lock);
 
 		iscsit_increment_maxcmdsn(cmd, sess);
-
 		iscsit_free_cmd(cmd, true);
 
-		spin_lock_bh(&conn->cmd_lock);
 	}
-	spin_unlock_bh(&conn->cmd_lock);
 }
 
 static void iscsit_stop_timers_for_cmds(
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2407,7 +2407,8 @@ static void target_release_cmd_kref(stru
 	}
 
 	spin_lock(&se_cmd->t_state_lock);
-	fabric_stop = (se_cmd->transport_state & CMD_T_FABRIC_STOP);
+	fabric_stop = (se_cmd->transport_state & CMD_T_FABRIC_STOP) &&
+		      (se_cmd->transport_state & CMD_T_ABORTED);
 	spin_unlock(&se_cmd->t_state_lock);
 
 	if (se_cmd->cmd_wait_set || fabric_stop) {

  parent reply	other threads:[~2016-08-18 13:56 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20160818135526uscas1p209dc90415100838cf7c73a2d19ca32a3@uscas1p2.samsung.com>
2016-08-18 13:54 ` [PATCH 3.14 00/46] 3.14.77-stable review Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 01/46] tcp: make challenge acks faster Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 02/46] usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable() Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 11/46] cifs: Check for existing directory when opening file with O_CREAT Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 12/46] cifs: fix crash due to race in hmac(md5) handling Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 14/46] random: properly align get_random_int_hash Greg Kroah-Hartman
2016-08-19  3:14     ` Eric Biggers
2016-08-19  7:33       ` Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 15/46] random: print a warning for the first ten uninitialized random users Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 16/46] [PATCH BACKPORT 3.10-3.15 1/4] MIPS: KVM: Fix mapped fault broken commpage handling Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 17/46] [PATCH BACKPORT 3.10-3.15 2/4] MIPS: KVM: Add missing gfn range check Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 18/46] [PATCH BACKPORT 3.10-3.15 3/4] MIPS: KVM: Fix gfn range check in kseg0 tlb faults Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 19/46] [PATCH BACKPORT 3.10-3.15 4/4] MIPS: KVM: Propagate kseg0/mapped tlb fault errors Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 20/46] nfs: dont create zero-length requests Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 30/46] balloon: check the number of available pages in leak balloon Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 31/46] ftrace/recordmcount: Work around for addition of metag magic but not relocations Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 32/46] metag: Fix __cmpxchg_u32 asm constraint for CMP Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 33/46] IB/mlx5: Fix MODIFY_QP command input structure Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 34/46] IB/mlx5: Fix returned values of query QP Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 35/46] IB/mlx5: Fix post send fence logic Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 36/46] IB/IPoIB: Dont update neigh validity for unresolved entries Greg Kroah-Hartman
2016-08-18 13:54   ` [PATCH 3.14 37/46] IB/mlx4: Fix the SQ size of an RC QP Greg Kroah-Hartman
2016-08-18 13:55   ` [PATCH 3.14 38/46] ubi: Make volume resize power cut aware Greg Kroah-Hartman
2016-08-18 13:55   ` [PATCH 3.14 39/46] ubi: Fix race condition between ubi device creation and udev Greg Kroah-Hartman
2016-08-18 13:55   ` Greg Kroah-Hartman [this message]
2016-08-18 13:55   ` [PATCH 3.14 41/46] target: Fix max_unmap_lba_count calc overflow Greg Kroah-Hartman
2016-08-18 13:55   ` [PATCH 3.14 42/46] Input: i8042 - break load dependency between atkbd/psmouse and i8042 Greg Kroah-Hartman
2016-08-18 13:55   ` [PATCH 3.14 43/46] PCI: Mark Atheros AR9485 and QCA9882 to avoid bus reset Greg Kroah-Hartman
2016-08-18 13:55   ` [PATCH 3.14 44/46] dm flakey: error READ bios during the down_interval Greg Kroah-Hartman
2016-08-18 13:55   ` [PATCH 3.14 45/46] module: Invalidate signatures on force-loaded modules Greg Kroah-Hartman
2016-08-18 13:55   ` [PATCH 3.14 46/46] Documentation/module-signing.txt: Note need for version info if reusing a key Greg Kroah-Hartman
2016-08-18 20:05   ` [PATCH 3.14 00/46] 3.14.77-stable review Guenter Roeck
2016-08-19  7:38     ` Greg Kroah-Hartman
2016-08-18 21:34   ` 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=20160818135448.401387482@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --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=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.