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, 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 18:04 UTC|newest]

Thread overview: 50+ 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   ` 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-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 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.