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.10 21/35] iscsi-target: Fix iscsit_start_kthreads failure OOPs
Date: Fri, 14 Aug 2015 10:44:59 -0700	[thread overview]
Message-ID: <20150814174354.528962514@linuxfoundation.org> (raw)
In-Reply-To: <20150814174353.835241087@linuxfoundation.org>

3.10-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 |    1 
 drivers/target/iscsi/iscsi_target_nego.c  |   34 +++++++++++++++++++++++
 5 files changed, 66 insertions(+), 31 deletions(-)

--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3874,7 +3874,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;
 }
@@ -3956,7 +3962,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;
@@ -3966,10 +3972,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
@@ -589,6 +589,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
@@ -84,6 +84,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);
@@ -718,6 +719,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:
@@ -728,7 +730,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)
@@ -738,7 +740,6 @@ int iscsi_post_login_handler(
 	struct se_session *se_sess = sess->se_sess;
 	struct iscsi_portal_group *tpg = ISCSI_TPG_S(sess);
 	struct se_portal_group *se_tpg = &tpg->tpg_se_tpg;
-	int rc;
 
 	iscsit_inc_conn_usage_count(conn);
 
@@ -779,10 +780,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
@@ -791,15 +788,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);
@@ -840,10 +842,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
@@ -852,10 +850,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)
@@ -1331,20 +1331,9 @@ static int __iscsi_target_login_thread(s
 	if (iscsi_target_start_negotiation(login, conn) < 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;
-
-	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 = NULL;
--- a/drivers/target/iscsi/iscsi_target_login.h
+++ b/drivers/target/iscsi/iscsi_target_login.h
@@ -12,6 +12,7 @@ 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 iscsit_start_kthreads(struct iscsi_conn *);
 extern int iscsi_target_login_thread(void *);
 extern int iscsi_login_disable_FIM_keys(struct iscsi_param_list *, struct iscsi_conn *);
 
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -19,6 +19,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>
@@ -352,10 +353,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);
@@ -364,6 +379,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 int iscsi_target_do_login_io(struct iscsi_conn *conn, struct iscsi_login *login)



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

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-14 17:44 [PATCH 3.10 00/35] 3.10.87-stable review Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 01/35] ARM: realview: fix sparsemem build Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 02/35] MIPS: Fix sched_getaffinity with MT FPAFF enabled Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 03/35] MIPS: Make set_pte() SMP safe Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 04/35] fsnotify: fix oops in fsnotify_clear_marks_by_group_flags() Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 05/35] drm/radeon/combios: add some validation of lvds values Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 06/35] ipr: Fix locking for unit attention handling Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 07/35] ipr: Fix incorrect trace indexing Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 08/35] ipr: Fix invalid array indexing for HRRQ Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 10/35] USB: sierra: add 1199:68AB device ID Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 11/35] md: use kzalloc() when bitmap is disabled Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 12/35] ipmi: fix timeout calculation when bmc is disconnected Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 13/35] mfd: sm501: dbg_regs attribute must be read-only Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 14/35] perf/x86/amd: Rework AMD PMU init code Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 15/35] sparc64: Fix FPU register corruption with AES crypto offload Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 16/35] sparc64: Fix userspace FPU register corruptions Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 17/35] 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.10 19/35] crypto: ixp4xx - Remove bogus BUG_ON on scattered dst buffer Greg Kroah-Hartman
2015-08-14 17:44 ` [PATCH 3.10 20/35] rbd: fix copyup completion race Greg Kroah-Hartman
2015-08-14 17:44 ` Greg Kroah-Hartman [this message]
2015-08-14 17:45 ` [PATCH 3.10 22/35] ALSA: hda - fix cs4210_spdif_automute() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 23/35] ipc: modify message queue accounting to not take kernel data structures into account Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 24/35] ocfs2: fix BUG in ocfs2_downconvert_thread_do_work() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 25/35] md/raid1: extend spinlock to protect raid1_end_read_request against inconsistencies Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 26/35] sg_start_req(): make sure that theres not too many elements in iovec Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 27/35] ARM: Fix !kuser helpers case Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 28/35] ARM: Fix FIQ code on VIVT CPUs Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 29/35] ARM: 7819/1: fiq: Cast the first argument of flush_icache_range() Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 30/35] signalfd: fix information leak in signalfd_copyinfo Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 31/35] signal: fix information leak in copy_siginfo_to_user Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 32/35] signal: fix information leak in copy_siginfo_from_user32 Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 33/35] kvm: x86: fix kvm_apic_has_events to check for NULL pointer Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 34/35] md/bitmap: return an error when bitmap superblock is corrupt Greg Kroah-Hartman
2015-08-14 17:45 ` [PATCH 3.10 35/35] mm, vmscan: Do not wait for page writeback for GFP_NOFS allocations Greg Kroah-Hartman
     [not found] ` <55ce3244.6a6ab40a.1286.60de@mx.google.com>
2015-08-14 18:25   ` [PATCH 3.10 00/35] 3.10.87-stable review Kevin Hilman
2015-08-14 19:15     ` Greg Kroah-Hartman
2015-08-14 19:25       ` Tyler Baker
2015-08-14 18:36   ` Kevin Hilman
     [not found] ` <55ce7ee9.c365b40a.aac67.ffffc07a@mx.google.com>
2015-08-14 23:52   ` Kevin Hilman
2015-08-15  0:46     ` Greg Kroah-Hartman
2015-08-15  0:12 ` Shuah Khan
2015-08-15 15:15 ` 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=20150814174354.528962514@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.