stable.vger.kernel.org archive mirror
 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, Sagi Grimberg <sagig@mellanox.com>,
	Nicholas Bellinger <nab@linux-iscsi.org>,
	Nicholas Bellinger <nab@daterainc.com>
Subject: [PATCH 3.14 23/44] iscsi-target: Fix iscsit_start_kthreads failure OOPs
Date: Fri, 14 Aug 2015 10:45:00 -0700	[thread overview]
Message-ID: <20150814174402.334738515@linuxfoundation.org> (raw)
In-Reply-To: <20150814174401.628233291@linuxfoundation.org>

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

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

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

commit e54198657b65625085834847ab6271087323ffea upstream.

This patch fixes a regression introduced with the following commit
in v4.0-rc1 code, where a iscsit_start_kthreads() failure triggers
a NULL pointer dereference OOPs:

    commit 88dcd2dab5c23b1c9cfc396246d8f476c872f0ca
    Author: Nicholas Bellinger <nab@linux-iscsi.org>
    Date:   Thu Feb 26 22:19:15 2015 -0800

        iscsi-target: Convert iscsi_thread_set usage to kthread.h

To address this bug, move iscsit_start_kthreads() immediately
preceeding the transmit of last login response, before signaling
a successful transition into full-feature-phase within existing
iscsi_target_do_tx_login_io() logic.

This ensures that no target-side resource allocation failures can
occur after the final login response has been successfully sent.

Also, it adds a iscsi_conn->rx_login_comp to allow the RX thread
to sleep to prevent other socket related failures until the final
iscsi_post_login_handler() call is able to complete.

Cc: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Nicholas Bellinger <nab@daterainc.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/target/iscsi/iscsi_target.c       |   18 ++++++++++--
 drivers/target/iscsi/iscsi_target_core.h  |    1 
 drivers/target/iscsi/iscsi_target_login.c |   43 +++++++++++-------------------
 drivers/target/iscsi/iscsi_target_login.h |    3 +-
 drivers/target/iscsi/iscsi_target_nego.c  |   34 +++++++++++++++++++++++
 5 files changed, 67 insertions(+), 32 deletions(-)

--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3937,7 +3937,13 @@ get_immediate:
 	}
 
 transport_err:
-	iscsit_take_action_for_connection_exit(conn);
+	/*
+	 * Avoid the normal connection failure code-path if this connection
+	 * is still within LOGIN mode, and iscsi_np process context is
+	 * responsible for cleaning up the early connection failure.
+	 */
+	if (conn->conn_state != TARG_CONN_STATE_IN_LOGIN)
+		iscsit_take_action_for_connection_exit(conn);
 out:
 	return 0;
 }
@@ -4023,7 +4029,7 @@ reject:
 
 int iscsi_target_rx_thread(void *arg)
 {
-	int ret;
+	int ret, rc;
 	u8 buffer[ISCSI_HDR_LEN], opcode;
 	u32 checksum = 0, digest = 0;
 	struct iscsi_conn *conn = arg;
@@ -4033,10 +4039,16 @@ int iscsi_target_rx_thread(void *arg)
 	 * connection recovery / failure event can be triggered externally.
 	 */
 	allow_signal(SIGINT);
+	/*
+	 * Wait for iscsi_post_login_handler() to complete before allowing
+	 * incoming iscsi/tcp socket I/O, and/or failing the connection.
+	 */
+	rc = wait_for_completion_interruptible(&conn->rx_login_comp);
+	if (rc < 0)
+		return 0;
 
 	if (conn->conn_transport->transport_type == ISCSI_INFINIBAND) {
 		struct completion comp;
-		int rc;
 
 		init_completion(&comp);
 		rc = wait_for_completion_interruptible(&comp);
--- a/drivers/target/iscsi/iscsi_target_core.h
+++ b/drivers/target/iscsi/iscsi_target_core.h
@@ -604,6 +604,7 @@ struct iscsi_conn {
 	int			bitmap_id;
 	int			rx_thread_active;
 	struct task_struct	*rx_thread;
+	struct completion	rx_login_comp;
 	int			tx_thread_active;
 	struct task_struct	*tx_thread;
 	/* list_head for session connection list */
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -83,6 +83,7 @@ static struct iscsi_login *iscsi_login_i
 	init_completion(&conn->conn_logout_comp);
 	init_completion(&conn->rx_half_close_comp);
 	init_completion(&conn->tx_half_close_comp);
+	init_completion(&conn->rx_login_comp);
 	spin_lock_init(&conn->cmd_lock);
 	spin_lock_init(&conn->conn_usage_lock);
 	spin_lock_init(&conn->immed_queue_lock);
@@ -716,6 +717,7 @@ int iscsit_start_kthreads(struct iscsi_c
 
 	return 0;
 out_tx:
+	send_sig(SIGINT, conn->tx_thread, 1);
 	kthread_stop(conn->tx_thread);
 	conn->tx_thread_active = false;
 out_bitmap:
@@ -726,7 +728,7 @@ out_bitmap:
 	return ret;
 }
 
-int iscsi_post_login_handler(
+void iscsi_post_login_handler(
 	struct iscsi_np *np,
 	struct iscsi_conn *conn,
 	u8 zero_tsih)
@@ -736,7 +738,6 @@ int iscsi_post_login_handler(
 	struct se_session *se_sess = sess->se_sess;
 	struct iscsi_portal_group *tpg = sess->tpg;
 	struct se_portal_group *se_tpg = &tpg->tpg_se_tpg;
-	int rc;
 
 	iscsit_inc_conn_usage_count(conn);
 
@@ -777,10 +778,6 @@ int iscsi_post_login_handler(
 			sess->sess_ops->InitiatorName);
 		spin_unlock_bh(&sess->conn_lock);
 
-		rc = iscsit_start_kthreads(conn);
-		if (rc)
-			return rc;
-
 		iscsi_post_login_start_timers(conn);
 		/*
 		 * Determine CPU mask to ensure connection's RX and TX kthreads
@@ -789,15 +786,20 @@ int iscsi_post_login_handler(
 		iscsit_thread_get_cpumask(conn);
 		conn->conn_rx_reset_cpumask = 1;
 		conn->conn_tx_reset_cpumask = 1;
-
+		/*
+		 * Wakeup the sleeping iscsi_target_rx_thread() now that
+		 * iscsi_conn is in TARG_CONN_STATE_LOGGED_IN state.
+		 */
+		complete(&conn->rx_login_comp);
 		iscsit_dec_conn_usage_count(conn);
+
 		if (stop_timer) {
 			spin_lock_bh(&se_tpg->session_lock);
 			iscsit_stop_time2retain_timer(sess);
 			spin_unlock_bh(&se_tpg->session_lock);
 		}
 		iscsit_dec_session_usage_count(sess);
-		return 0;
+		return;
 	}
 
 	iscsi_set_session_parameters(sess->sess_ops, conn->param_list, 1);
@@ -838,10 +840,6 @@ int iscsi_post_login_handler(
 		" iSCSI Target Portal Group: %hu\n", tpg->nsessions, tpg->tpgt);
 	spin_unlock_bh(&se_tpg->session_lock);
 
-	rc = iscsit_start_kthreads(conn);
-	if (rc)
-		return rc;
-
 	iscsi_post_login_start_timers(conn);
 	/*
 	 * Determine CPU mask to ensure connection's RX and TX kthreads
@@ -850,10 +848,12 @@ int iscsi_post_login_handler(
 	iscsit_thread_get_cpumask(conn);
 	conn->conn_rx_reset_cpumask = 1;
 	conn->conn_tx_reset_cpumask = 1;
-
+	/*
+	 * Wakeup the sleeping iscsi_target_rx_thread() now that
+	 * iscsi_conn is in TARG_CONN_STATE_LOGGED_IN state.
+	 */
+	complete(&conn->rx_login_comp);
 	iscsit_dec_conn_usage_count(conn);
-
-	return 0;
 }
 
 static void iscsi_handle_login_thread_timeout(unsigned long data)
@@ -1418,23 +1418,12 @@ static int __iscsi_target_login_thread(s
 	if (ret < 0)
 		goto new_sess_out;
 
-	if (!conn->sess) {
-		pr_err("struct iscsi_conn session pointer is NULL!\n");
-		goto new_sess_out;
-	}
-
 	iscsi_stop_login_thread_timer(np);
 
-	if (signal_pending(current))
-		goto new_sess_out;
-
 	if (ret == 1) {
 		tpg_np = conn->tpg_np;
 
-		ret = iscsi_post_login_handler(np, conn, zero_tsih);
-		if (ret < 0)
-			goto new_sess_out;
-
+		iscsi_post_login_handler(np, conn, zero_tsih);
 		iscsit_deaccess_np(np, tpg, tpg_np);
 	}
 
--- a/drivers/target/iscsi/iscsi_target_login.h
+++ b/drivers/target/iscsi/iscsi_target_login.h
@@ -12,7 +12,8 @@ extern int iscsit_accept_np(struct iscsi
 extern int iscsit_get_login_rx(struct iscsi_conn *, struct iscsi_login *);
 extern int iscsit_put_login_tx(struct iscsi_conn *, struct iscsi_login *, u32);
 extern void iscsit_free_conn(struct iscsi_np *, struct iscsi_conn *);
-extern int iscsi_post_login_handler(struct iscsi_np *, struct iscsi_conn *, u8);
+extern int iscsit_start_kthreads(struct iscsi_conn *);
+extern void iscsi_post_login_handler(struct iscsi_np *, struct iscsi_conn *, u8);
 extern void iscsi_target_login_sess_out(struct iscsi_conn *, struct iscsi_np *,
 				bool, bool);
 extern int iscsi_target_login_thread(void *);
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -17,6 +17,7 @@
  ******************************************************************************/
 
 #include <linux/ctype.h>
+#include <linux/kthread.h>
 #include <scsi/iscsi_proto.h>
 #include <target/target_core_base.h>
 #include <target/target_core_fabric.h>
@@ -361,10 +362,24 @@ static int iscsi_target_do_tx_login_io(s
 		ntohl(login_rsp->statsn), login->rsp_length);
 
 	padding = ((-login->rsp_length) & 3);
+	/*
+	 * Before sending the last login response containing the transition
+	 * bit for full-feature-phase, go ahead and start up TX/RX threads
+	 * now to avoid potential resource allocation failures after the
+	 * final login response has been sent.
+	 */
+	if (login->login_complete) {
+		int rc = iscsit_start_kthreads(conn);
+		if (rc) {
+			iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
+					    ISCSI_LOGIN_STATUS_NO_RESOURCES);
+			return -1;
+		}
+	}
 
 	if (conn->conn_transport->iscsit_put_login_tx(conn, login,
 					login->rsp_length + padding) < 0)
-		return -1;
+		goto err;
 
 	login->rsp_length		= 0;
 	mutex_lock(&sess->cmdsn_mutex);
@@ -373,6 +388,23 @@ static int iscsi_target_do_tx_login_io(s
 	mutex_unlock(&sess->cmdsn_mutex);
 
 	return 0;
+
+err:
+	if (login->login_complete) {
+		if (conn->rx_thread && conn->rx_thread_active) {
+			send_sig(SIGINT, conn->rx_thread, 1);
+			kthread_stop(conn->rx_thread);
+		}
+		if (conn->tx_thread && conn->tx_thread_active) {
+			send_sig(SIGINT, conn->tx_thread, 1);
+			kthread_stop(conn->tx_thread);
+		}
+		spin_lock(&iscsit_global->ts_bitmap_lock);
+		bitmap_release_region(iscsit_global->ts_bitmap, conn->bitmap_id,
+				      get_order(1));
+		spin_unlock(&iscsit_global->ts_bitmap_lock);
+	}
+	return -1;
 }
 
 static void iscsi_target_sk_data_ready(struct sock *sk, int count)



  parent reply	other threads:[~2015-08-14 17:53 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-14 17:44 [PATCH 3.14 00/44] 3.14.51-stable review Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 01/44] ARM: realview: fix sparsemem build Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 02/44] MIPS: Malta: Dont reinitialise RTC Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 03/44] MIPS: Fix sched_getaffinity with MT FPAFF enabled Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 04/44] MIPS: Make set_pte() SMP safe Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 05/44] fsnotify: fix oops in fsnotify_clear_marks_by_group_flags() Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 06/44] drm/radeon/combios: add some validation of lvds values Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 07/44] ipr: Fix locking for unit attention handling Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 08/44] ipr: Fix incorrect trace indexing Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 09/44] ipr: Fix invalid array indexing for HRRQ Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 11/44] USB: sierra: add 1199:68AB device ID Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 12/44] ima: add support for new "euid" policy condition Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 13/44] ima: extend "mask" policy matching support Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 14/44] md: use kzalloc() when bitmap is disabled Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 15/44] ARM: sunxi: fix build for THUMB2_KERNEL Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 16/44] [PATCH] sparc64: Fix userspace FPU register corruptions Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 17/44] ASoC: pcm1681: Fix setting de-emphasis sampling rate selection Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 18/44] x86/xen: Probe target addresses in set_aliased_prot() before the hypercall Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 20/44] crypto: ixp4xx - Remove bogus BUG_ON on scattered dst buffer Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 21/44] rbd: fix copyup completion race Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.14 22/44] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc Greg Kroah-Hartman
2015-08-14 17:45 ` Greg Kroah-Hartman [this message]
2015-08-14 17:45 ` [PATCH 3.14 24/44] ALSA: hda - fix cs4210_spdif_automute() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 25/44] ipc: modify message queue accounting to not take kernel data structures into account Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 26/44] ocfs2: fix BUG in ocfs2_downconvert_thread_do_work() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 27/44] PCI: Restore PCI_MSIX_FLAGS_BIRMASK definition Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 28/44] md/raid1: extend spinlock to protect raid1_end_read_request against inconsistencies Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 29/44] sg_start_req(): make sure that theres not too many elements in iovec Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 30/44] signalfd: fix information leak in signalfd_copyinfo Greg Kroah-Hartman
2015-09-03 15:43   ` Luis Henriques
2015-08-14 17:45 ` [PATCH 3.14 31/44] signal: fix information leak in copy_siginfo_to_user Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 32/44] signal: fix information leak in copy_siginfo_from_user32 Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 33/44] fold d_kill() and d_free() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 34/44] fold try_prune_one_dentry() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 35/44] new helper: dentry_free() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 36/44] expand the call of dentry_lru_del() in dentry_kill() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 37/44] dentry_kill(): dont try to remove from shrink list Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 38/44] dont remove from shrink list in select_collect() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 39/44] more graceful recovery in umount_collect() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 40/44] dcache: dont need rcu in shrink_dentry_list() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 41/44] kvm: x86: fix kvm_apic_has_events to check for NULL pointer Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 42/44] path_openat(): fix double fput() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 43/44] md/bitmap: return an error when bitmap superblock is corrupt Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.14 44/44] mm, vmscan: Do not wait for page writeback for GFP_NOFS allocations Greg Kroah-Hartman
2015-08-15  0:11 ` [PATCH 3.14 00/44] 3.14.51-stable review Shuah Khan
     [not found] ` <55cf55dd.8c4cc20a.acdcb.49af@mx.google.com>
2015-08-15 15:10   ` Kevin Hilman
2015-08-15 16:41     ` Greg Kroah-Hartman
2015-08-15 15:17 ` Guenter Roeck

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=20150814174402.334738515@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nab@daterainc.com \
    --cc=nab@linux-iscsi.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).