All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH-3.18.y 0/2] target: stable backports
@ 2017-11-16  6:17 Nicholas A. Bellinger
  2017-11-16  6:17 ` [PATCH-3.18.y 1/2] target/iscsi: Fix iSCSI task reassignment handling Nicholas A. Bellinger
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2017-11-16  6:17 UTC (permalink / raw)
  To: target-devel; +Cc: stable, Greg-KH, Nicholas Bellinger

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

Here are two target patches for v3.18.y stable, the first of which
did not originally include a stable CC, and the latter did not apply
due to a minor context change.

The series has been cut against v3.18.81.  Please apply at your earliest
convenience.

Thank you,

--nab

Bart Van Assche (1):
  target/iscsi: Fix iSCSI task reassignment handling

Nicholas Bellinger (1):
  iscsi-target: Fix iscsi_np reset hung task during parallel delete

 drivers/target/iscsi/iscsi_target.c       | 20 ++++++++------------
 drivers/target/iscsi/iscsi_target_core.h  |  1 +
 drivers/target/iscsi/iscsi_target_login.c |  7 +++++--
 include/target/iscsi/iscsi_target_core.h  |  1 +
 include/target/target_core_base.h         |  1 +
 5 files changed, 16 insertions(+), 14 deletions(-)

-- 
1.8.5.3


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 1/2] target/iscsi: Fix iSCSI task reassignment handling
  2017-11-16  6:17 [PATCH-3.18.y 0/2] target: stable backports Nicholas A. Bellinger
@ 2017-11-16  6:17 ` Nicholas A. Bellinger
  2017-11-16  6:17   ` Nicholas A. Bellinger
  2018-01-13  0:03   ` Nicholas A. Bellinger
  2 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2017-11-16  6:17 UTC (permalink / raw)
  To: target-devel; +Cc: stable, Greg-KH, Bart Van Assche, Moshe David

From: Bart Van Assche <bart.vanassche@sandisk.com>

commit 59b6986dbfcdab96a971f9663221849de79a7556 upstream.

Allocate a task management request structure for all task management
requests, including task reassignment. This change avoids that the
se_tmr->response assignment dereferences an uninitialized se_tmr
pointer.

Reported-by: Moshe David <mdavid@infinidat.com>
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Moshe David <mdavid@infinidat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target.c | 19 +++++++------------
 include/target/target_core_base.h   |  1 +
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index ce844e9..801369f 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1749,7 +1749,7 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
 	struct iscsi_tm *hdr;
 	int out_of_order_cmdsn = 0, ret;
 	bool sess_ref = false;
-	u8 function;
+	u8 function, tcm_function = TMR_UNKNOWN;
 
 	hdr			= (struct iscsi_tm *) buf;
 	hdr->flags &= ~ISCSI_FLAG_CMD_FINAL;
@@ -1795,10 +1795,6 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
 	 * LIO-Target $FABRIC_MOD
 	 */
 	if (function != ISCSI_TM_FUNC_TASK_REASSIGN) {
-
-		u8 tcm_function;
-		int ret;
-
 		transport_init_se_cmd(&cmd->se_cmd,
 				      &lio_target_fabric_configfs->tf_ops,
 				      conn->sess->se_sess, 0, DMA_NONE,
@@ -1835,15 +1831,14 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
 			return iscsit_add_reject_cmd(cmd,
 				ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
 		}
-
-		ret = core_tmr_alloc_req(&cmd->se_cmd, cmd->tmr_req,
-					 tcm_function, GFP_KERNEL);
-		if (ret < 0)
-			return iscsit_add_reject_cmd(cmd,
+	}
+	ret = core_tmr_alloc_req(&cmd->se_cmd, cmd->tmr_req, tcm_function,
+				 GFP_KERNEL);
+	if (ret < 0)
+		return iscsit_add_reject_cmd(cmd,
 				ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
 
-		cmd->tmr_req->se_tmr_req = cmd->se_cmd.se_tmr_req;
-	}
+	cmd->tmr_req->se_tmr_req = cmd->se_cmd.se_tmr_req;
 
 	cmd->iscsi_opcode	= ISCSI_OP_SCSI_TMFUNC;
 	cmd->i_state		= ISTATE_SEND_TASKMGTRSP;
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index d1e7cb9..ab4bff0 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -231,6 +231,7 @@ enum tcm_tmreq_table {
 	TMR_LUN_RESET		= 5,
 	TMR_TARGET_WARM_RESET	= 6,
 	TMR_TARGET_COLD_RESET	= 7,
+	TMR_UNKNOWN		= 0xff,
 };
 
 /* fabric independent task management response values */
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
  2017-11-16  6:17 [PATCH-3.18.y 0/2] target: stable backports Nicholas A. Bellinger
@ 2017-11-16  6:17   ` Nicholas A. Bellinger
  2017-11-16  6:17   ` Nicholas A. Bellinger
  2018-01-13  0:03   ` Nicholas A. Bellinger
  2 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2017-11-16  6:17 UTC (permalink / raw)
  To: target-devel
  Cc: stable, Greg-KH, Nicholas Bellinger, Mike Christie, Hannes Reinecke

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

This patch fixes a bug associated with iscsit_reset_np_thread()
that can occur during parallel configfs rmdir of a single iscsi_np
used across multiple iscsi-target instances, that would result in
hung task(s) similar to below where configfs rmdir process context
was blocked indefinately waiting for iscsi_np->np_restart_comp
to finish:

[ 6726.112076] INFO: task dcp_proxy_node_:15550 blocked for more than 120 seconds.
[ 6726.119440]       Tainted: G        W  O     4.1.26-3321 #2
[ 6726.125045] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 6726.132927] dcp_proxy_node_ D ffff8803f202bc88     0 15550      1 0x00000000
[ 6726.140058]  ffff8803f202bc88 ffff88085c64d960 ffff88083b3b1ad0 ffff88087fffeb08
[ 6726.147593]  ffff8803f202c000 7fffffffffffffff ffff88083f459c28 ffff88083b3b1ad0
[ 6726.155132]  ffff88035373c100 ffff8803f202bca8 ffffffff8168ced2 ffff8803f202bcb8
[ 6726.162667] Call Trace:
[ 6726.165150]  [<ffffffff8168ced2>] schedule+0x32/0x80
[ 6726.170156]  [<ffffffff8168f5b4>] schedule_timeout+0x214/0x290
[ 6726.176030]  [<ffffffff810caef2>] ? __send_signal+0x52/0x4a0
[ 6726.181728]  [<ffffffff8168d7d6>] wait_for_completion+0x96/0x100
[ 6726.187774]  [<ffffffff810e7c80>] ? wake_up_state+0x10/0x10
[ 6726.193395]  [<ffffffffa035d6e2>] iscsit_reset_np_thread+0x62/0xe0 [iscsi_target_mod]
[ 6726.201278]  [<ffffffffa0355d86>] iscsit_tpg_disable_portal_group+0x96/0x190 [iscsi_target_mod]
[ 6726.210033]  [<ffffffffa0363f7f>] lio_target_tpg_store_enable+0x4f/0xc0 [iscsi_target_mod]
[ 6726.218351]  [<ffffffff81260c5a>] configfs_write_file+0xaa/0x110
[ 6726.224392]  [<ffffffff811ea364>] vfs_write+0xa4/0x1b0
[ 6726.229576]  [<ffffffff811eb111>] SyS_write+0x41/0xb0
[ 6726.234659]  [<ffffffff8169042e>] system_call_fastpath+0x12/0x71

It would happen because each iscsit_reset_np_thread() sets state
to ISCSI_NP_THREAD_RESET, sends SIGINT, and then blocks waiting
for completion on iscsi_np->np_restart_comp.

However, if iscsi_np was active processing a login request and
more than a single iscsit_reset_np_thread() caller to the same
iscsi_np was blocked on iscsi_np->np_restart_comp, iscsi_np
kthread process context in __iscsi_target_login_thread() would
flush pending signals and only perform a single completion of
np->np_restart_comp before going back to sleep within transport
specific iscsit_transport->iscsi_accept_np code.

To address this bug, add a iscsi_np->np_reset_count and update
__iscsi_target_login_thread() to keep completing np->np_restart_comp
until ->np_reset_count has reached zero.

Reported-by: Gary Guo <ghg@datera.io>
Tested-by: Gary Guo <ghg@datera.io>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target.c       | 1 +
 drivers/target/iscsi/iscsi_target_core.h  | 1 +
 drivers/target/iscsi/iscsi_target_login.c | 7 +++++--
 include/target/iscsi/iscsi_target_core.h  | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 801369f..2fc6b49 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -428,6 +428,7 @@ int iscsit_reset_np_thread(
 		return 0;
 	}
 	np->np_thread_state = ISCSI_NP_THREAD_RESET;
+	atomic_inc(&np->np_reset_count);
 
 	if (np->np_thread) {
 		spin_unlock_bh(&np->np_thread_lock);
diff --git a/drivers/target/iscsi/iscsi_target_core.h b/drivers/target/iscsi/iscsi_target_core.h
index 1863de2..bf3da8e 100644
--- a/drivers/target/iscsi/iscsi_target_core.h
+++ b/drivers/target/iscsi/iscsi_target_core.h
@@ -783,6 +783,7 @@ struct iscsi_np {
 	int			np_sock_type;
 	enum np_thread_state_table np_thread_state;
 	bool                    enabled;
+	atomic_t		np_reset_count;
 	enum iscsi_timer_flags_table np_login_timer_flags;
 	u32			np_exports;
 	enum np_flags_table	np_flags;
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 4608d8d..540af1b 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -1275,9 +1275,11 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
 	flush_signals(current);
 
 	spin_lock_bh(&np->np_thread_lock);
-	if (np->np_thread_state = ISCSI_NP_THREAD_RESET) {
+	if (atomic_dec_if_positive(&np->np_reset_count) >= 0) {
 		np->np_thread_state = ISCSI_NP_THREAD_ACTIVE;
+		spin_unlock_bh(&np->np_thread_lock);
 		complete(&np->np_restart_comp);
+		return 1;
 	} else if (np->np_thread_state = ISCSI_NP_THREAD_SHUTDOWN) {
 		spin_unlock_bh(&np->np_thread_lock);
 		goto exit;
@@ -1310,7 +1312,8 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
 		goto exit;
 	} else if (rc < 0) {
 		spin_lock_bh(&np->np_thread_lock);
-		if (np->np_thread_state = ISCSI_NP_THREAD_RESET) {
+		if (atomic_dec_if_positive(&np->np_reset_count) >= 0) {
+			np->np_thread_state = ISCSI_NP_THREAD_ACTIVE;
 			spin_unlock_bh(&np->np_thread_lock);
 			complete(&np->np_restart_comp);
 			iscsit_put_transport(conn->conn_transport);
diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 7bd03f8..e365ed9 100644
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -784,6 +784,7 @@ struct iscsi_np {
 	int			np_sock_type;
 	enum np_thread_state_table np_thread_state;
 	bool                    enabled;
+	atomic_t		np_reset_count;
 	enum iscsi_timer_flags_table np_login_timer_flags;
 	u32			np_exports;
 	enum np_flags_table	np_flags;
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
@ 2017-11-16  6:17   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2017-11-16  6:17 UTC (permalink / raw)
  To: target-devel
  Cc: stable, Greg-KH, Nicholas Bellinger, Mike Christie, Hannes Reinecke

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

This patch fixes a bug associated with iscsit_reset_np_thread()
that can occur during parallel configfs rmdir of a single iscsi_np
used across multiple iscsi-target instances, that would result in
hung task(s) similar to below where configfs rmdir process context
was blocked indefinately waiting for iscsi_np->np_restart_comp
to finish:

[ 6726.112076] INFO: task dcp_proxy_node_:15550 blocked for more than 120 seconds.
[ 6726.119440]       Tainted: G        W  O     4.1.26-3321 #2
[ 6726.125045] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 6726.132927] dcp_proxy_node_ D ffff8803f202bc88     0 15550      1 0x00000000
[ 6726.140058]  ffff8803f202bc88 ffff88085c64d960 ffff88083b3b1ad0 ffff88087fffeb08
[ 6726.147593]  ffff8803f202c000 7fffffffffffffff ffff88083f459c28 ffff88083b3b1ad0
[ 6726.155132]  ffff88035373c100 ffff8803f202bca8 ffffffff8168ced2 ffff8803f202bcb8
[ 6726.162667] Call Trace:
[ 6726.165150]  [<ffffffff8168ced2>] schedule+0x32/0x80
[ 6726.170156]  [<ffffffff8168f5b4>] schedule_timeout+0x214/0x290
[ 6726.176030]  [<ffffffff810caef2>] ? __send_signal+0x52/0x4a0
[ 6726.181728]  [<ffffffff8168d7d6>] wait_for_completion+0x96/0x100
[ 6726.187774]  [<ffffffff810e7c80>] ? wake_up_state+0x10/0x10
[ 6726.193395]  [<ffffffffa035d6e2>] iscsit_reset_np_thread+0x62/0xe0 [iscsi_target_mod]
[ 6726.201278]  [<ffffffffa0355d86>] iscsit_tpg_disable_portal_group+0x96/0x190 [iscsi_target_mod]
[ 6726.210033]  [<ffffffffa0363f7f>] lio_target_tpg_store_enable+0x4f/0xc0 [iscsi_target_mod]
[ 6726.218351]  [<ffffffff81260c5a>] configfs_write_file+0xaa/0x110
[ 6726.224392]  [<ffffffff811ea364>] vfs_write+0xa4/0x1b0
[ 6726.229576]  [<ffffffff811eb111>] SyS_write+0x41/0xb0
[ 6726.234659]  [<ffffffff8169042e>] system_call_fastpath+0x12/0x71

It would happen because each iscsit_reset_np_thread() sets state
to ISCSI_NP_THREAD_RESET, sends SIGINT, and then blocks waiting
for completion on iscsi_np->np_restart_comp.

However, if iscsi_np was active processing a login request and
more than a single iscsit_reset_np_thread() caller to the same
iscsi_np was blocked on iscsi_np->np_restart_comp, iscsi_np
kthread process context in __iscsi_target_login_thread() would
flush pending signals and only perform a single completion of
np->np_restart_comp before going back to sleep within transport
specific iscsit_transport->iscsi_accept_np code.

To address this bug, add a iscsi_np->np_reset_count and update
__iscsi_target_login_thread() to keep completing np->np_restart_comp
until ->np_reset_count has reached zero.

Reported-by: Gary Guo <ghg@datera.io>
Tested-by: Gary Guo <ghg@datera.io>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target.c       | 1 +
 drivers/target/iscsi/iscsi_target_core.h  | 1 +
 drivers/target/iscsi/iscsi_target_login.c | 7 +++++--
 include/target/iscsi/iscsi_target_core.h  | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 801369f..2fc6b49 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -428,6 +428,7 @@ int iscsit_reset_np_thread(
 		return 0;
 	}
 	np->np_thread_state = ISCSI_NP_THREAD_RESET;
+	atomic_inc(&np->np_reset_count);
 
 	if (np->np_thread) {
 		spin_unlock_bh(&np->np_thread_lock);
diff --git a/drivers/target/iscsi/iscsi_target_core.h b/drivers/target/iscsi/iscsi_target_core.h
index 1863de2..bf3da8e 100644
--- a/drivers/target/iscsi/iscsi_target_core.h
+++ b/drivers/target/iscsi/iscsi_target_core.h
@@ -783,6 +783,7 @@ struct iscsi_np {
 	int			np_sock_type;
 	enum np_thread_state_table np_thread_state;
 	bool                    enabled;
+	atomic_t		np_reset_count;
 	enum iscsi_timer_flags_table np_login_timer_flags;
 	u32			np_exports;
 	enum np_flags_table	np_flags;
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 4608d8d..540af1b 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -1275,9 +1275,11 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
 	flush_signals(current);
 
 	spin_lock_bh(&np->np_thread_lock);
-	if (np->np_thread_state == ISCSI_NP_THREAD_RESET) {
+	if (atomic_dec_if_positive(&np->np_reset_count) >= 0) {
 		np->np_thread_state = ISCSI_NP_THREAD_ACTIVE;
+		spin_unlock_bh(&np->np_thread_lock);
 		complete(&np->np_restart_comp);
+		return 1;
 	} else if (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN) {
 		spin_unlock_bh(&np->np_thread_lock);
 		goto exit;
@@ -1310,7 +1312,8 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
 		goto exit;
 	} else if (rc < 0) {
 		spin_lock_bh(&np->np_thread_lock);
-		if (np->np_thread_state == ISCSI_NP_THREAD_RESET) {
+		if (atomic_dec_if_positive(&np->np_reset_count) >= 0) {
+			np->np_thread_state = ISCSI_NP_THREAD_ACTIVE;
 			spin_unlock_bh(&np->np_thread_lock);
 			complete(&np->np_restart_comp);
 			iscsit_put_transport(conn->conn_transport);
diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 7bd03f8..e365ed9 100644
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -784,6 +784,7 @@ struct iscsi_np {
 	int			np_sock_type;
 	enum np_thread_state_table np_thread_state;
 	bool                    enabled;
+	atomic_t		np_reset_count;
 	enum iscsi_timer_flags_table np_login_timer_flags;
 	u32			np_exports;
 	enum np_flags_table	np_flags;
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
  2017-11-16  6:17   ` Nicholas A. Bellinger
@ 2017-11-16 16:46     ` Greg-KH
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg-KH @ 2017-11-16 16:46 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: target-devel, stable, Mike Christie, Hannes Reinecke

On Thu, Nov 16, 2017 at 06:17:40AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch fixes a bug associated with iscsit_reset_np_thread()
> that can occur during parallel configfs rmdir of a single iscsi_np
> used across multiple iscsi-target instances, that would result in
> hung task(s) similar to below where configfs rmdir process context
> was blocked indefinately waiting for iscsi_np->np_restart_comp
> to finish:
> 
> [ 6726.112076] INFO: task dcp_proxy_node_:15550 blocked for more than 120 seconds.
> [ 6726.119440]       Tainted: G        W  O     4.1.26-3321 #2
> [ 6726.125045] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 6726.132927] dcp_proxy_node_ D ffff8803f202bc88     0 15550      1 0x00000000
> [ 6726.140058]  ffff8803f202bc88 ffff88085c64d960 ffff88083b3b1ad0 ffff88087fffeb08
> [ 6726.147593]  ffff8803f202c000 7fffffffffffffff ffff88083f459c28 ffff88083b3b1ad0
> [ 6726.155132]  ffff88035373c100 ffff8803f202bca8 ffffffff8168ced2 ffff8803f202bcb8
> [ 6726.162667] Call Trace:
> [ 6726.165150]  [<ffffffff8168ced2>] schedule+0x32/0x80
> [ 6726.170156]  [<ffffffff8168f5b4>] schedule_timeout+0x214/0x290
> [ 6726.176030]  [<ffffffff810caef2>] ? __send_signal+0x52/0x4a0
> [ 6726.181728]  [<ffffffff8168d7d6>] wait_for_completion+0x96/0x100
> [ 6726.187774]  [<ffffffff810e7c80>] ? wake_up_state+0x10/0x10
> [ 6726.193395]  [<ffffffffa035d6e2>] iscsit_reset_np_thread+0x62/0xe0 [iscsi_target_mod]
> [ 6726.201278]  [<ffffffffa0355d86>] iscsit_tpg_disable_portal_group+0x96/0x190 [iscsi_target_mod]
> [ 6726.210033]  [<ffffffffa0363f7f>] lio_target_tpg_store_enable+0x4f/0xc0 [iscsi_target_mod]
> [ 6726.218351]  [<ffffffff81260c5a>] configfs_write_file+0xaa/0x110
> [ 6726.224392]  [<ffffffff811ea364>] vfs_write+0xa4/0x1b0
> [ 6726.229576]  [<ffffffff811eb111>] SyS_write+0x41/0xb0
> [ 6726.234659]  [<ffffffff8169042e>] system_call_fastpath+0x12/0x71
> 
> It would happen because each iscsit_reset_np_thread() sets state
> to ISCSI_NP_THREAD_RESET, sends SIGINT, and then blocks waiting
> for completion on iscsi_np->np_restart_comp.
> 
> However, if iscsi_np was active processing a login request and
> more than a single iscsit_reset_np_thread() caller to the same
> iscsi_np was blocked on iscsi_np->np_restart_comp, iscsi_np
> kthread process context in __iscsi_target_login_thread() would
> flush pending signals and only perform a single completion of
> np->np_restart_comp before going back to sleep within transport
> specific iscsit_transport->iscsi_accept_np code.
> 
> To address this bug, add a iscsi_np->np_reset_count and update
> __iscsi_target_login_thread() to keep completing np->np_restart_comp
> until ->np_reset_count has reached zero.
> 
> Reported-by: Gary Guo <ghg@datera.io>
> Tested-by: Gary Guo <ghg@datera.io>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: stable@vger.kernel.org # 3.10+
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/target/iscsi/iscsi_target.c       | 1 +
>  drivers/target/iscsi/iscsi_target_core.h  | 1 +
>  drivers/target/iscsi/iscsi_target_login.c | 7 +++++--
>  include/target/iscsi/iscsi_target_core.h  | 1 +
>  4 files changed, 8 insertions(+), 2 deletions(-)

What is the git commit id of this patch in Linus's tree?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
@ 2017-11-16 16:46     ` Greg-KH
  0 siblings, 0 replies; 18+ messages in thread
From: Greg-KH @ 2017-11-16 16:46 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: target-devel, stable, Mike Christie, Hannes Reinecke

On Thu, Nov 16, 2017 at 06:17:40AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch fixes a bug associated with iscsit_reset_np_thread()
> that can occur during parallel configfs rmdir of a single iscsi_np
> used across multiple iscsi-target instances, that would result in
> hung task(s) similar to below where configfs rmdir process context
> was blocked indefinately waiting for iscsi_np->np_restart_comp
> to finish:
> 
> [ 6726.112076] INFO: task dcp_proxy_node_:15550 blocked for more than 120 seconds.
> [ 6726.119440]       Tainted: G        W  O     4.1.26-3321 #2
> [ 6726.125045] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 6726.132927] dcp_proxy_node_ D ffff8803f202bc88     0 15550      1 0x00000000
> [ 6726.140058]  ffff8803f202bc88 ffff88085c64d960 ffff88083b3b1ad0 ffff88087fffeb08
> [ 6726.147593]  ffff8803f202c000 7fffffffffffffff ffff88083f459c28 ffff88083b3b1ad0
> [ 6726.155132]  ffff88035373c100 ffff8803f202bca8 ffffffff8168ced2 ffff8803f202bcb8
> [ 6726.162667] Call Trace:
> [ 6726.165150]  [<ffffffff8168ced2>] schedule+0x32/0x80
> [ 6726.170156]  [<ffffffff8168f5b4>] schedule_timeout+0x214/0x290
> [ 6726.176030]  [<ffffffff810caef2>] ? __send_signal+0x52/0x4a0
> [ 6726.181728]  [<ffffffff8168d7d6>] wait_for_completion+0x96/0x100
> [ 6726.187774]  [<ffffffff810e7c80>] ? wake_up_state+0x10/0x10
> [ 6726.193395]  [<ffffffffa035d6e2>] iscsit_reset_np_thread+0x62/0xe0 [iscsi_target_mod]
> [ 6726.201278]  [<ffffffffa0355d86>] iscsit_tpg_disable_portal_group+0x96/0x190 [iscsi_target_mod]
> [ 6726.210033]  [<ffffffffa0363f7f>] lio_target_tpg_store_enable+0x4f/0xc0 [iscsi_target_mod]
> [ 6726.218351]  [<ffffffff81260c5a>] configfs_write_file+0xaa/0x110
> [ 6726.224392]  [<ffffffff811ea364>] vfs_write+0xa4/0x1b0
> [ 6726.229576]  [<ffffffff811eb111>] SyS_write+0x41/0xb0
> [ 6726.234659]  [<ffffffff8169042e>] system_call_fastpath+0x12/0x71
> 
> It would happen because each iscsit_reset_np_thread() sets state
> to ISCSI_NP_THREAD_RESET, sends SIGINT, and then blocks waiting
> for completion on iscsi_np->np_restart_comp.
> 
> However, if iscsi_np was active processing a login request and
> more than a single iscsit_reset_np_thread() caller to the same
> iscsi_np was blocked on iscsi_np->np_restart_comp, iscsi_np
> kthread process context in __iscsi_target_login_thread() would
> flush pending signals and only perform a single completion of
> np->np_restart_comp before going back to sleep within transport
> specific iscsit_transport->iscsi_accept_np code.
> 
> To address this bug, add a iscsi_np->np_reset_count and update
> __iscsi_target_login_thread() to keep completing np->np_restart_comp
> until ->np_reset_count has reached zero.
> 
> Reported-by: Gary Guo <ghg@datera.io>
> Tested-by: Gary Guo <ghg@datera.io>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: stable@vger.kernel.org # 3.10+
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/target/iscsi/iscsi_target.c       | 1 +
>  drivers/target/iscsi/iscsi_target_core.h  | 1 +
>  drivers/target/iscsi/iscsi_target_login.c | 7 +++++--
>  include/target/iscsi/iscsi_target_core.h  | 1 +
>  4 files changed, 8 insertions(+), 2 deletions(-)

What is the git commit id of this patch in Linus's tree?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
  2017-11-16 16:46     ` Greg-KH
@ 2017-11-16 18:35       ` Mike Christie
  -1 siblings, 0 replies; 18+ messages in thread
From: Mike Christie @ 2017-11-16 18:35 UTC (permalink / raw)
  To: Greg-KH, Nicholas A. Bellinger; +Cc: target-devel, stable, Hannes Reinecke

On 11/16/2017 10:46 AM, Greg-KH wrote:
> 
> What is the git commit id of this patch in Linus's tree?
> 

It's 978d13d60c34818a41fc35962602bdfa5c03f214


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
@ 2017-11-16 18:35       ` Mike Christie
  0 siblings, 0 replies; 18+ messages in thread
From: Mike Christie @ 2017-11-16 18:35 UTC (permalink / raw)
  To: Greg-KH, Nicholas A. Bellinger; +Cc: target-devel, stable, Hannes Reinecke

On 11/16/2017 10:46 AM, Greg-KH wrote:
> 
> What is the git commit id of this patch in Linus's tree?
> 

It's 978d13d60c34818a41fc35962602bdfa5c03f214

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
  2017-11-16 18:35       ` Mike Christie
@ 2017-11-19 10:23         ` Greg-KH
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg-KH @ 2017-11-19 10:23 UTC (permalink / raw)
  To: Mike Christie
  Cc: Nicholas A. Bellinger, target-devel, stable, Hannes Reinecke

On Thu, Nov 16, 2017 at 12:35:32PM -0600, Mike Christie wrote:
> On 11/16/2017 10:46 AM, Greg-KH wrote:
> > 
> > What is the git commit id of this patch in Linus's tree?
> > 
> 
> It's 978d13d60c34818a41fc35962602bdfa5c03f214

Thanks, now queued up.

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete
@ 2017-11-19 10:23         ` Greg-KH
  0 siblings, 0 replies; 18+ messages in thread
From: Greg-KH @ 2017-11-19 10:23 UTC (permalink / raw)
  To: Mike Christie
  Cc: Nicholas A. Bellinger, target-devel, stable, Hannes Reinecke

On Thu, Nov 16, 2017 at 12:35:32PM -0600, Mike Christie wrote:
> On 11/16/2017 10:46 AM, Greg-KH wrote:
> > 
> > What is the git commit id of this patch in Linus's tree?
> > 
> 
> It's 978d13d60c34818a41fc35962602bdfa5c03f214

Thanks, now queued up.

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 0/2] target: stable backports
@ 2018-01-13  0:03   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2018-01-12 23:49 UTC (permalink / raw)
  To: target-devel; +Cc: stable, Greg-KH, Nicholas Bellinger

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

Hi Greg-KH,

Here are two target patches for v3.18.y stable, which did not apply
due to minor context changes.

The series has been cut against v3.18.91.  Please apply at your earliest
convenience.

Thank you,

--nab

Nicholas Bellinger (2):
  iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref
  target: Avoid early CMD_T_PRE_EXECUTE failures during ABORT_TASK

 drivers/target/iscsi/iscsi_target.c    | 21 +++++++--------------
 drivers/target/target_core_tmr.c       |  9 +++++++++
 drivers/target/target_core_transport.c |  2 ++
 include/target/target_core_base.h      |  1 +
 4 files changed, 19 insertions(+), 14 deletions(-)

-- 
1.8.5.3


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 1/2] iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref
  2018-01-13  0:03   ` Nicholas A. Bellinger
@ 2018-01-13  0:03     ` Nicholas A. Bellinger
  -1 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2018-01-12 23:49 UTC (permalink / raw)
  To: target-devel
  Cc: stable, Greg-KH, Nicholas Bellinger, Donald White, Mike Christie,
	Hannes Reinecke

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

commit ae072726f6109bb1c94841d6fb3a82dde298ea85 upstream.

Since commit 59b6986dbf fixed a potential NULL pointer dereference
by allocating a se_tmr_req for ISCSI_TM_FUNC_TASK_REASSIGN, the
se_tmr_req is currently leaked by iscsit_free_cmd() because no
iscsi_cmd->se_cmd.se_tfo was associated.

To address this, treat ISCSI_TM_FUNC_TASK_REASSIGN like any other
TMR and call transport_init_se_cmd() + target_get_sess_cmd() to
setup iscsi_cmd->se_cmd.se_tfo with se_cmd->cmd_kref of 2.

This will ensure normal release operation once se_cmd->cmd_kref
reaches zero and target_release_cmd_kref() is invoked, se_tmr_req
will be released via existing target_free_cmd_mem() and
core_tmr_release_req() code.

Reported-by: Donald White <dew@datera.io>
Cc: Donald White <dew@datera.io>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 9582f08..1850597 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1750,7 +1750,6 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
 	struct iscsi_tmr_req *tmr_req;
 	struct iscsi_tm *hdr;
 	int out_of_order_cmdsn = 0, ret;
-	bool sess_ref = false;
 	u8 function, tcm_function = TMR_UNKNOWN;
 
 	hdr			= (struct iscsi_tm *) buf;
@@ -1792,19 +1791,17 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
 					     buf);
 	}
 
+	transport_init_se_cmd(&cmd->se_cmd, &lio_target_fabric_configfs->tf_ops,
+			      conn->sess->se_sess, 0, DMA_NONE,
+			      MSG_SIMPLE_TAG, cmd->sense_buffer + 2);
+
+	target_get_sess_cmd(&cmd->se_cmd, true);
+
 	/*
 	 * TASK_REASSIGN for ERL=2 / connection stays inside of
 	 * LIO-Target $FABRIC_MOD
 	 */
 	if (function != ISCSI_TM_FUNC_TASK_REASSIGN) {
-		transport_init_se_cmd(&cmd->se_cmd,
-				      &lio_target_fabric_configfs->tf_ops,
-				      conn->sess->se_sess, 0, DMA_NONE,
-				      MSG_SIMPLE_TAG, cmd->sense_buffer + 2);
-
-		target_get_sess_cmd(&cmd->se_cmd, true);
-		sess_ref = true;
-
 		switch (function) {
 		case ISCSI_TM_FUNC_ABORT_TASK:
 			tcm_function = TMR_ABORT_TASK;
@@ -1943,12 +1940,8 @@ attach:
 	 * For connection recovery, this is also the default action for
 	 * TMR TASK_REASSIGN.
 	 */
-	if (sess_ref) {
-		pr_debug("Handle TMR, using sess_ref=true check\n");
-		target_put_sess_cmd(&cmd->se_cmd);
-	}
-
 	iscsit_add_cmd_to_response_queue(cmd, conn, cmd->i_state);
+	target_put_sess_cmd(&cmd->se_cmd);
 	return 0;
 }
 EXPORT_SYMBOL(iscsit_handle_task_mgt_cmd);
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 2/2] target: Avoid early CMD_T_PRE_EXECUTE failures during ABORT_TASK
  2018-01-13  0:03   ` Nicholas A. Bellinger
@ 2018-01-13  0:03     ` Nicholas A. Bellinger
  -1 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2018-01-12 23:49 UTC (permalink / raw)
  To: target-devel
  Cc: stable, Greg-KH, Nicholas Bellinger, Donald White, Mike Christie,
	Hannes Reinecke

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

commit 1c21a48055a67ceb693e9c2587824a8de60a217c upstream.

This patch fixes bug where early se_cmd exceptions that occur
before backend execution can result in use-after-free if/when
a subsequent ABORT_TASK occurs for the same tag.

Since an early se_cmd exception will have had se_cmd added to
se_session->sess_cmd_list via target_get_sess_cmd(), it will
not have CMD_T_COMPLETE set by the usual target_complete_cmd()
backend completion path.

This causes a subsequent ABORT_TASK + __target_check_io_state()
to signal ABORT_TASK should proceed.  As core_tmr_abort_task()
executes, it will bring the outstanding se_cmd->cmd_kref count
down to zero releasing se_cmd, after se_cmd has already been
queued with error status into fabric driver response path code.

To address this bug, introduce a CMD_T_PRE_EXECUTE bit that is
set at target_get_sess_cmd() time, and cleared immediately before
backend driver dispatch in target_execute_cmd() once CMD_T_ACTIVE
is set.

Then, check CMD_T_PRE_EXECUTE within __target_check_io_state() to
determine when an early exception has occured, and avoid aborting
this se_cmd since it will have already been queued into fabric
driver response path code.

Reported-by: Donald White <dew@datera.io>
Cc: Donald White <dew@datera.io>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_tmr.c       | 9 +++++++++
 drivers/target/target_core_transport.c | 2 ++
 include/target/target_core_base.h      | 1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 9b1a400..9372473 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -137,6 +137,15 @@ static bool __target_check_io_state(struct se_cmd *se_cmd,
 		spin_unlock(&se_cmd->t_state_lock);
 		return false;
 	}
+	if (se_cmd->transport_state & CMD_T_PRE_EXECUTE) {
+		if (se_cmd->scsi_status) {
+			pr_debug("Attempted to abort io tag: %u early failure"
+				 " status: 0x%02x\n", se_cmd->se_tfo->get_task_tag(se_cmd),
+				 se_cmd->scsi_status);
+			spin_unlock(&se_cmd->t_state_lock);
+			return false;
+		}
+	}
 	if (sess->sess_tearing_down || se_cmd->cmd_wait_set) {
 		pr_debug("Attempted to abort io tag: %u already shutdown,"
 			" skipping\n", se_cmd->se_tfo->get_task_tag(se_cmd));
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index e59c8b3..26afe1c 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1796,6 +1796,7 @@ void target_execute_cmd(struct se_cmd *cmd)
 	}
 
 	cmd->t_state = TRANSPORT_PROCESSING;
+	cmd->transport_state &= ~CMD_T_PRE_EXECUTE;
 	cmd->transport_state |= CMD_T_ACTIVE|CMD_T_BUSY|CMD_T_SENT;
 	spin_unlock_irq(&cmd->t_state_lock);
 	/*
@@ -2436,6 +2437,7 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref)
 		ret = -ESHUTDOWN;
 		goto out;
 	}
+	se_cmd->transport_state |= CMD_T_PRE_EXECUTE;
 	list_add_tail(&se_cmd->se_cmd_list, &se_sess->sess_cmd_list);
 out:
 	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index ab4bff0..ea5e809 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -538,6 +538,7 @@ struct se_cmd {
 #define CMD_T_BUSY		(1 << 9)
 #define CMD_T_TAS		(1 << 10)
 #define CMD_T_FABRIC_STOP	(1 << 11)
+#define CMD_T_PRE_EXECUTE	(1 << 12)
 	spinlock_t		t_state_lock;
 	struct completion	t_transport_stop_comp;
 
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 0/2] target: stable backports
@ 2018-01-13  0:03   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2018-01-13  0:03 UTC (permalink / raw)
  To: target-devel; +Cc: stable, Greg-KH, Nicholas Bellinger

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

Hi Greg-KH,

Here are two target patches for v3.18.y stable, which did not apply
due to minor context changes.

The series has been cut against v3.18.91.  Please apply at your earliest
convenience.

Thank you,

--nab

Nicholas Bellinger (2):
  iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref
  target: Avoid early CMD_T_PRE_EXECUTE failures during ABORT_TASK

 drivers/target/iscsi/iscsi_target.c    | 21 +++++++--------------
 drivers/target/target_core_tmr.c       |  9 +++++++++
 drivers/target/target_core_transport.c |  2 ++
 include/target/target_core_base.h      |  1 +
 4 files changed, 19 insertions(+), 14 deletions(-)

-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 1/2] iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref
@ 2018-01-13  0:03     ` Nicholas A. Bellinger
  0 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2018-01-13  0:03 UTC (permalink / raw)
  To: target-devel
  Cc: stable, Greg-KH, Nicholas Bellinger, Donald White, Mike Christie,
	Hannes Reinecke

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

commit ae072726f6109bb1c94841d6fb3a82dde298ea85 upstream.

Since commit 59b6986dbf fixed a potential NULL pointer dereference
by allocating a se_tmr_req for ISCSI_TM_FUNC_TASK_REASSIGN, the
se_tmr_req is currently leaked by iscsit_free_cmd() because no
iscsi_cmd->se_cmd.se_tfo was associated.

To address this, treat ISCSI_TM_FUNC_TASK_REASSIGN like any other
TMR and call transport_init_se_cmd() + target_get_sess_cmd() to
setup iscsi_cmd->se_cmd.se_tfo with se_cmd->cmd_kref of 2.

This will ensure normal release operation once se_cmd->cmd_kref
reaches zero and target_release_cmd_kref() is invoked, se_tmr_req
will be released via existing target_free_cmd_mem() and
core_tmr_release_req() code.

Reported-by: Donald White <dew@datera.io>
Cc: Donald White <dew@datera.io>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 9582f08..1850597 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1750,7 +1750,6 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
 	struct iscsi_tmr_req *tmr_req;
 	struct iscsi_tm *hdr;
 	int out_of_order_cmdsn = 0, ret;
-	bool sess_ref = false;
 	u8 function, tcm_function = TMR_UNKNOWN;
 
 	hdr			= (struct iscsi_tm *) buf;
@@ -1792,19 +1791,17 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
 					     buf);
 	}
 
+	transport_init_se_cmd(&cmd->se_cmd, &lio_target_fabric_configfs->tf_ops,
+			      conn->sess->se_sess, 0, DMA_NONE,
+			      MSG_SIMPLE_TAG, cmd->sense_buffer + 2);
+
+	target_get_sess_cmd(&cmd->se_cmd, true);
+
 	/*
 	 * TASK_REASSIGN for ERL=2 / connection stays inside of
 	 * LIO-Target $FABRIC_MOD
 	 */
 	if (function != ISCSI_TM_FUNC_TASK_REASSIGN) {
-		transport_init_se_cmd(&cmd->se_cmd,
-				      &lio_target_fabric_configfs->tf_ops,
-				      conn->sess->se_sess, 0, DMA_NONE,
-				      MSG_SIMPLE_TAG, cmd->sense_buffer + 2);
-
-		target_get_sess_cmd(&cmd->se_cmd, true);
-		sess_ref = true;
-
 		switch (function) {
 		case ISCSI_TM_FUNC_ABORT_TASK:
 			tcm_function = TMR_ABORT_TASK;
@@ -1943,12 +1940,8 @@ attach:
 	 * For connection recovery, this is also the default action for
 	 * TMR TASK_REASSIGN.
 	 */
-	if (sess_ref) {
-		pr_debug("Handle TMR, using sess_ref=true check\n");
-		target_put_sess_cmd(&cmd->se_cmd);
-	}
-
 	iscsit_add_cmd_to_response_queue(cmd, conn, cmd->i_state);
+	target_put_sess_cmd(&cmd->se_cmd);
 	return 0;
 }
 EXPORT_SYMBOL(iscsit_handle_task_mgt_cmd);
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH-3.18.y 2/2] target: Avoid early CMD_T_PRE_EXECUTE failures during ABORT_TASK
@ 2018-01-13  0:03     ` Nicholas A. Bellinger
  0 siblings, 0 replies; 18+ messages in thread
From: Nicholas A. Bellinger @ 2018-01-13  0:03 UTC (permalink / raw)
  To: target-devel
  Cc: stable, Greg-KH, Nicholas Bellinger, Donald White, Mike Christie,
	Hannes Reinecke

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

commit 1c21a48055a67ceb693e9c2587824a8de60a217c upstream.

This patch fixes bug where early se_cmd exceptions that occur
before backend execution can result in use-after-free if/when
a subsequent ABORT_TASK occurs for the same tag.

Since an early se_cmd exception will have had se_cmd added to
se_session->sess_cmd_list via target_get_sess_cmd(), it will
not have CMD_T_COMPLETE set by the usual target_complete_cmd()
backend completion path.

This causes a subsequent ABORT_TASK + __target_check_io_state()
to signal ABORT_TASK should proceed.  As core_tmr_abort_task()
executes, it will bring the outstanding se_cmd->cmd_kref count
down to zero releasing se_cmd, after se_cmd has already been
queued with error status into fabric driver response path code.

To address this bug, introduce a CMD_T_PRE_EXECUTE bit that is
set at target_get_sess_cmd() time, and cleared immediately before
backend driver dispatch in target_execute_cmd() once CMD_T_ACTIVE
is set.

Then, check CMD_T_PRE_EXECUTE within __target_check_io_state() to
determine when an early exception has occured, and avoid aborting
this se_cmd since it will have already been queued into fabric
driver response path code.

Reported-by: Donald White <dew@datera.io>
Cc: Donald White <dew@datera.io>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_tmr.c       | 9 +++++++++
 drivers/target/target_core_transport.c | 2 ++
 include/target/target_core_base.h      | 1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 9b1a400..9372473 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -137,6 +137,15 @@ static bool __target_check_io_state(struct se_cmd *se_cmd,
 		spin_unlock(&se_cmd->t_state_lock);
 		return false;
 	}
+	if (se_cmd->transport_state & CMD_T_PRE_EXECUTE) {
+		if (se_cmd->scsi_status) {
+			pr_debug("Attempted to abort io tag: %u early failure"
+				 " status: 0x%02x\n", se_cmd->se_tfo->get_task_tag(se_cmd),
+				 se_cmd->scsi_status);
+			spin_unlock(&se_cmd->t_state_lock);
+			return false;
+		}
+	}
 	if (sess->sess_tearing_down || se_cmd->cmd_wait_set) {
 		pr_debug("Attempted to abort io tag: %u already shutdown,"
 			" skipping\n", se_cmd->se_tfo->get_task_tag(se_cmd));
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index e59c8b3..26afe1c 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1796,6 +1796,7 @@ void target_execute_cmd(struct se_cmd *cmd)
 	}
 
 	cmd->t_state = TRANSPORT_PROCESSING;
+	cmd->transport_state &= ~CMD_T_PRE_EXECUTE;
 	cmd->transport_state |= CMD_T_ACTIVE|CMD_T_BUSY|CMD_T_SENT;
 	spin_unlock_irq(&cmd->t_state_lock);
 	/*
@@ -2436,6 +2437,7 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref)
 		ret = -ESHUTDOWN;
 		goto out;
 	}
+	se_cmd->transport_state |= CMD_T_PRE_EXECUTE;
 	list_add_tail(&se_cmd->se_cmd_list, &se_sess->sess_cmd_list);
 out:
 	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index ab4bff0..ea5e809 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -538,6 +538,7 @@ struct se_cmd {
 #define CMD_T_BUSY		(1 << 9)
 #define CMD_T_TAS		(1 << 10)
 #define CMD_T_FABRIC_STOP	(1 << 11)
+#define CMD_T_PRE_EXECUTE	(1 << 12)
 	spinlock_t		t_state_lock;
 	struct completion	t_transport_stop_comp;
 
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 0/2] target: stable backports
  2018-01-13  0:03   ` Nicholas A. Bellinger
@ 2018-01-13 17:33     ` Greg-KH
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg-KH @ 2018-01-13 17:33 UTC (permalink / raw)
  To: Nicholas A. Bellinger; +Cc: target-devel, stable

On Sat, Jan 13, 2018 at 12:03:16AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi Greg-KH,
> 
> Here are two target patches for v3.18.y stable, which did not apply
> due to minor context changes.
> 
> The series has been cut against v3.18.91.  Please apply at your earliest
> convenience.

Both applied, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH-3.18.y 0/2] target: stable backports
@ 2018-01-13 17:33     ` Greg-KH
  0 siblings, 0 replies; 18+ messages in thread
From: Greg-KH @ 2018-01-13 17:33 UTC (permalink / raw)
  To: Nicholas A. Bellinger; +Cc: target-devel, stable

On Sat, Jan 13, 2018 at 12:03:16AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi Greg-KH,
> 
> Here are two target patches for v3.18.y stable, which did not apply
> due to minor context changes.
> 
> The series has been cut against v3.18.91.  Please apply at your earliest
> convenience.

Both applied, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-01-13 17:33 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-16  6:17 [PATCH-3.18.y 0/2] target: stable backports Nicholas A. Bellinger
2017-11-16  6:17 ` [PATCH-3.18.y 1/2] target/iscsi: Fix iSCSI task reassignment handling Nicholas A. Bellinger
2017-11-16  6:17 ` [PATCH-3.18.y 2/2] iscsi-target: Fix iscsi_np reset hung task during parallel delete Nicholas A. Bellinger
2017-11-16  6:17   ` Nicholas A. Bellinger
2017-11-16 16:46   ` Greg-KH
2017-11-16 16:46     ` Greg-KH
2017-11-16 18:35     ` Mike Christie
2017-11-16 18:35       ` Mike Christie
2017-11-19 10:23       ` Greg-KH
2017-11-19 10:23         ` Greg-KH
2018-01-12 23:49 ` [PATCH-3.18.y 0/2] target: stable backports Nicholas A. Bellinger
2018-01-13  0:03   ` Nicholas A. Bellinger
2018-01-12 23:49   ` [PATCH-3.18.y 1/2] iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref Nicholas A. Bellinger
2018-01-13  0:03     ` Nicholas A. Bellinger
2018-01-12 23:49   ` [PATCH-3.18.y 2/2] target: Avoid early CMD_T_PRE_EXECUTE failures during ABORT_TASK Nicholas A. Bellinger
2018-01-13  0:03     ` Nicholas A. Bellinger
2018-01-13 17:33   ` [PATCH-3.18.y 0/2] target: stable backports Greg-KH
2018-01-13 17:33     ` Greg-KH

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.