fix stgt crash in conn_close

fix stgt crash in conn_close

[fx chen]

[-- Attachment #1: Type: text/plain, Size: 5892 bytes --]

hello!
we found tgtd happen core dump and fix it。

1： we use the stgt code
      git clone https://github.com/fujita/tgt.git
      stgt git commit version:  44c11763d71dc019741c84a857080cd0b4a2f265

2:   our test cases
      export 10 sata disk with stgt，each sata disk capacity is 100GB,
each target have one lun， in iscsi initiator import， using FIO to to
parallel random write test for 10 devices，
      during this period the frequent disconnection of iscsi networks。

3:  tgtd crash, happen core dump
     test  about 5 hours，tgtd process crash， happen core dump， the
stack trace showed  below:
     [New LWP 121362]
     [New LWP 121349]
     [New LWP 121350]
     [New LWP 121370]
     [New LWP 121404]
     [New LWP 121365]
     [New LWP 121348]
     [Thread debugging using libthread_db enabled]
     Using host libthread_db library "/lib64/libthread_db.so.1".
     Core was generated by `tgtd -d 1'.
     Program terminated with signal 11, Segmentation fault.
     #0  0x000000000040a259 in __list_del (prev=0x7cd38c0,
next=0x5aff340) at ./list.h:83
     83 prev->next = next;
     Missing separate debuginfos, use: debuginfo-install
libgcc-4.8.3-9.el7.x86_64 libgcc-4.8.5-4.el7.x86_64
     (gdb) bt
    #0  0x000000000040a259 in __list_del (prev=0x7cd38c0,
next=0x5aff340) at ./list.h:83
    #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
    #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at
iscsi/iscsid.c:1254
    #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0,
scmd=0x623d0e0) at iscsi/iscsid.c:1269
    #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0,
result=0) at target.c:1236
    #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1,
data=0x0) at bs.c:210
    #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
    #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

4:  Our analysis of tgtd crash
      beause the IO during the frequent  connection/disconnection of
iscsi networks，this will trigger close current iscsi session，
      lead to stgt frequent call conn_close function,  conn_close will
clear all iscsi task，we found some ISCSI_OP_SCSI_CMD type's task,
      no list_del from session->cmd_list before free it.
5: Our patch for tgtd crash
From 2abf9229ea206172f2fe244539f7f87ed404eff8 Mon Sep 17 00:00:00 2001
From: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
Date: Fri, 22 Jul 2016 10:37:23 +0800
Subject: [PATCH] iscsi: fix segfault at conn_close

Remove some iscsi task from conn->session->cmd_list before free it,
otherwise it may cause tgtd process crash. Below is a backtrace info:

 Program terminated with signal 11, Segmentation fault.
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340)
at ./list.h:83
 83  prev->next = next;
 (gdb) bt
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340)
at ./list.h:83
 #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
 #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at
iscsi/iscsid.c:1254
 #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0,
scmd=0x623d0e0) at iscsi/iscsid.c:1269
 #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0,
result=0) at target.c:1236
 #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1,
data=0x0) at bs.c:210
 #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
 #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

Signed-off-by: Meng Lingkun <menglingkun@cmss.chinamobile.com>
Signed-off-by: Wang Zhengyong <wangzhengyong@cmss.chinamobile.com>
Reviewed-by: Wang Dongxu <wangdongxu@cmss.chinamobile.com>
Signed-off-by: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
---
 usr/iscsi/conn.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/usr/iscsi/conn.c b/usr/iscsi/conn.c
index e7d4e8c..3ed08a8 100644
--- a/usr/iscsi/conn.c
+++ b/usr/iscsi/conn.c
@@ -83,6 +83,22 @@ void conn_exit(struct iscsi_connection *conn)
  session_put(session);
 }

+static int find_task_in_cmd_list(struct iscsi_connection *conn,
+ struct iscsi_task *find_task)
+{
+ struct iscsi_task *task = NULL;
+ int find = 0;
+
+ list_for_each_entry(task, &conn->session->cmd_list, c_hlist) {
+ if (task == find_task) {
+ find = 1;
+ break;
+ }
+ }
+
+ return find;
+}
+
 void conn_close(struct iscsi_connection *conn)
 {
  struct iscsi_task *task, *tmp;
@@ -134,6 +150,7 @@ void conn_close(struct iscsi_connection *conn)
  list_for_each_entry_safe(task, tmp, &conn->tx_clist, c_list) {
  uint8_t op;

+ list_del(&task->c_list);
  op = task->req.opcode & ISCSI_OPCODE_MASK;

  eprintf("Forcing release of tx task %p %" PRIx64 " %x\n",
@@ -146,10 +163,14 @@ void conn_close(struct iscsi_connection *conn)
   * would be a better way to see
   * task->scmd.c_target though.
   */
- if (task->scmd.c_target)
+ if (task->scmd.c_target) {
  iscsi_free_cmd_task(task);
- else
+ } else {
+ if (find_task_in_cmd_list(conn, task))
+ list_del(&task->c_hlist);
+
  iscsi_free_task(task);
+ }
  break;
  case ISCSI_OP_NOOP_IN:
  /* NOOP_IN req is allocated within iscsi_tcp
@@ -181,6 +202,9 @@ void conn_close(struct iscsi_connection *conn)
  if (conn->rx_task) {
  eprintf("Forcing release of rx task %p %" PRIx64 "\n",
  conn->rx_task, conn->rx_task->tag);
+ if (find_task_in_cmd_list(conn, conn->rx_task))
+ list_del(&conn->rx_task->c_hlist);
+
  iscsi_free_task(conn->rx_task);
  }
  conn->rx_task = NULL;
@@ -193,6 +217,10 @@ void conn_close(struct iscsi_connection *conn)
   */
  if (task_in_scsi(task))
  continue;
+
+ if (find_task_in_cmd_list(conn, task))
+ list_del(&task->c_hlist);
+
  iscsi_free_task(task);
  }
 done:

--
1.8.3.1

[-- Attachment #2: 0001-iscsi-fix-segfault-at-conn_close.patch --]
[-- Type: application/octet-stream, Size: 3493 bytes --]

From 2abf9229ea206172f2fe244539f7f87ed404eff8 Mon Sep 17 00:00:00 2001
From: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
Date: Fri, 22 Jul 2016 10:37:23 +0800
Subject: [PATCH] iscsi: fix segfault at conn_close

Remove some iscsi task from conn->session->cmd_list before free it,
otherwise it may cause tgtd process crash. Below is a backtrace info:

 Program terminated with signal 11, Segmentation fault.
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 83  prev->next = next;
 (gdb) bt
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
 #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at iscsi/iscsid.c:1254
 #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0, scmd=0x623d0e0) at iscsi/iscsid.c:1269
 #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0, result=0) at target.c:1236
 #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1, data=0x0) at bs.c:210
 #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
 #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

Signed-off-by: Meng Lingkun <menglingkun@cmss.chinamobile.com>
Signed-off-by: Wang Zhengyong <wangzhengyong@cmss.chinamobile.com>
Reviewed-by: Wang Dongxu <wangdongxu@cmss.chinamobile.com>
Signed-off-by: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
---
 usr/iscsi/conn.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/usr/iscsi/conn.c b/usr/iscsi/conn.c
index e7d4e8c..3ed08a8 100644
--- a/usr/iscsi/conn.c
+++ b/usr/iscsi/conn.c
@@ -83,6 +83,22 @@ void conn_exit(struct iscsi_connection *conn)
 		session_put(session);
 }
 
+static int find_task_in_cmd_list(struct iscsi_connection *conn,
+				struct iscsi_task *find_task)
+{
+	struct iscsi_task *task = NULL;
+	int find = 0;
+
+	list_for_each_entry(task, &conn->session->cmd_list, c_hlist) {
+		if (task == find_task) {
+			find = 1;
+			break;
+		}
+	}
+
+	return find;
+}
+
 void conn_close(struct iscsi_connection *conn)
 {
 	struct iscsi_task *task, *tmp;
@@ -134,6 +150,7 @@ void conn_close(struct iscsi_connection *conn)
 	list_for_each_entry_safe(task, tmp, &conn->tx_clist, c_list) {
 		uint8_t op;
 
+		list_del(&task->c_list);
 		op = task->req.opcode & ISCSI_OPCODE_MASK;
 
 		eprintf("Forcing release of tx task %p %" PRIx64 " %x\n",
@@ -146,10 +163,14 @@ void conn_close(struct iscsi_connection *conn)
 			 * would be a better way to see
 			 * task->scmd.c_target though.
 			 */
-			if (task->scmd.c_target)
+			if (task->scmd.c_target) {
 				iscsi_free_cmd_task(task);
-			else
+			} else {
+				if (find_task_in_cmd_list(conn, task))
+					list_del(&task->c_hlist);
+
 				iscsi_free_task(task);
+			}
 			break;
 		case ISCSI_OP_NOOP_IN:
 			/* NOOP_IN req is allocated within iscsi_tcp
@@ -181,6 +202,9 @@ void conn_close(struct iscsi_connection *conn)
 	if (conn->rx_task) {
 		eprintf("Forcing release of rx task %p %" PRIx64 "\n",
 			conn->rx_task, conn->rx_task->tag);
+		if (find_task_in_cmd_list(conn, conn->rx_task))
+			list_del(&conn->rx_task->c_hlist);
+
 		iscsi_free_task(conn->rx_task);
 	}
 	conn->rx_task = NULL;
@@ -193,6 +217,10 @@ void conn_close(struct iscsi_connection *conn)
 		 */
 		if (task_in_scsi(task))
 			continue;
+
+		if (find_task_in_cmd_list(conn, task))
+			list_del(&task->c_hlist);
+
 		iscsi_free_task(task);
 	}
 done:
-- 
1.8.3.1

Re: fix stgt crash in conn_close

[FUJITA Tomonori]

On Tue, 26 Jul 2016 17:46:19 +0800
fx chen <chenfx2014@gmail.com> wrote:

> From c0d3f1498122430a166aa304e4e58139efec788a Mon Sep 17 00:00:00 2001
> From: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
> Date: Tue, 26 Jul 2016 11:33:39 +0800
> Subject: [PATCH v2] iscsi: fix segfault at conn_close
> 
> Remove some iscsi task from conn->session->cmd_list before free it,
> otherwise it may cause tgtd process crash. Below is a backtrace info:
> 
>  Program terminated with signal 11, Segmentation fault.
>  #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
>  83  prev->next = next;
>  (gdb) bt
>  #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
>  #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
>  #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at iscsi/iscsid.c:1254
>  #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0, scmd=0x623d0e0) at iscsi/iscsid.c:1269
>  #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0, result=0) at target.c:1236
>  #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1, data=0x0) at bs.c:210
>  #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
>  #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624
> 
> Signed-off-by: Meng Lingkun <menglingkun@cmss.chinamobile.com>
> Signed-off-by: Wang Zhengyong <wangzhengyong@cmss.chinamobile.com>
> Reviewed-by: Wang Dongxu <wangdongxu@cmss.chinamobile.com>
> Signed-off-by: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
> ---
>  usr/iscsi/iscsid.c | 4 +++-
>  usr/iscsi/iscsid.h | 2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)

Looks good to me. Thanks you so much, guys!

Re: fix stgt crash in conn_close

[Anton Kovalenko]

Hi,

fx chen <chenfx2014@gmail.com> writes:

> But carefully consider your modify，Logically it is not easy to
> understand，only fix current bug(Currently, I'm not sure whether still
> potential problems such modifications ),

Yep, I like your last version of the fix more than mine. Moving c_hlist
unlinking completely into iscsi_free_task is more logical than doing it
there "just in case" AND leaving it in iscsi_free_cmd_task.

As of c_siblings and c_list chains, they are supposed to be managed
correctly, so I'd better avoid adding an over-protective check.  As of
c_siblings, its life cycle is simple and it's obviously correct (for any
task, link when allocated, unlink when freed), as of c_list, I'm not so
sure, but that's no reason to patch iscsi_free_task ahead of time.

It would be great if you'd get time to test with a following fragment in
iscsi_free_task:

if (task->c_list.prev &&
    task->c_list.next && !list_empty(&task->c_list)) {
    abort("Freeing a task with non-empty c_list");
}



> 
> Iwe carefully consider the logical，why do below task->c_hlist.prev
> task->c_hlist.prev check in iscsi_free_task + if (task->c_hlist.prev
> && task->c_hlist.next + && !list_empty(&task->c_hlist)) { +
> list_del(&task->c_hlist); + } + Similarly，besides task->c_hlist，
> task have task->c_siblings, Why not do below task->c_siblings relate
> check?  + if (task->c_siblings.prev && task->c_siblings.next + &&
> !list_empty(&task->c_siblings)) { + list_del(&task->c_siblings); + } +
> Similarly，besides task->c_hlist， task have task->c_list, Why not do
> below task->c_list relate check?  + if (task->c_list.prev &&
> task->c_list.next + && !list_empty(&task->c_list)) { +
> list_del(&task->c_list); + } +

-- 
Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia

Re: fix stgt crash in conn_close

[fx chen]

[-- Attachment #1: Type: text/plain, Size: 3726 bytes --]

>Yep, now I use list_empty to check for list emptiness (it compares
>&c_hlist with c_hlist->head internally). Attaching the patch below.

>Are there any reasons for checking opcode then?

hi  Anton Kovalenko

we check task opcode in iscsi_free_task, if task opcode is
ISCSI_OP_SCSI_CMD, we do list list_del(&task->c_hlist);  because we
suppose iscsi_free_task is a generic interface to release task，thease
task include ISCSI_OP_SCSI_CMD，ISCSI_OP_NOOP_OUT, ISCSI_OP_SCSI_TMFUNC
and so on， but only ISCSI_OP_SCSI_CMD task is added to
adsession->cmd_list,  so， in iscsi_free_task， we check
ISCSI_OP_SCSI_CMD opcode and list_del, This logic is reasonable，

But carefully consider your modify，Logically it is not easy to
understand，only fix current bug(Currently, I'm not sure whether still
potential problems such modifications ),
Iwe carefully consider the logical，why  do below  task->c_hlist.prev
task->c_hlist.prev check in iscsi_free_task
+ if (task->c_hlist.prev && task->c_hlist.next
+    && !list_empty(&task->c_hlist)) {
+ list_del(&task->c_hlist);
+ }
+
Similarly，besides task->c_hlist， task have task->c_siblings, Why not
do below task->c_siblings relate check?
+ if (task->c_siblings.prev && task->c_siblings.next
+    && !list_empty(&task->c_siblings)) {
+ list_del(&task->c_siblings);
+ }
+
Similarly，besides task->c_hlist， task have task->c_list, Why not do
below task->c_list relate check?
+ if (task->c_list.prev && task->c_list.next
+    && !list_empty(&task->c_list)) {
+ list_del(&task->c_list);
+ }
+

Please think about our proposal！thank you

2016-07-26 16:02 GMT+08:00 Anton Kovalenko <anton.kovalenko@acronis.com>:
>
> Hi,
>
> fx chen <chenfx2014@gmail.com> writes:
>
>> we also consiger you idea，in iscsi_free_task, do task unlinking from
>> session->cmd_list(list_del(&task->c_hlist) )，but we must know，such as
>> ISCSI_OP_NOOP_OUT, ISCSI_OP_SCSI_TMFUNC,  ISCSI_OP_LOGOUT type task,
>> they don‘t add to session->cmd_list,   To solve the problem， we offer
>> patch<v2-0001-iscsi-fix-segfault-at-conn_close>。
>
> When a task is freshly allocated by iscsi_alloc_task, it has an empty
> c_hlist (see INIT_LIST_HEAD in iscsi_alloc_task), that will remain the
> same until iscsi_free_task if the task is never linked to a cmd_list.
>
> Therefore doing nothing with an empty c_hlist is enough (even list_del
> on an empty c_hlist would do no harm). That would skip ISCSI tasks which
> are not SCSI commands, exactly because they never get added to cmd_list.
>
> There are also two cases where c_hlist has prev == next == NULL, one of
> them when a task is allocated directly with iscsi_tcp_alloc_task (it
> happens for NOOP-IN "pings", and such a task should never be
> iscsi_free_task'ed), another one when a task used to be on a cmd_list,
> and then list_del was called (list_del sets prev == next == NULL).
>
> So I believe it's enough to check for NULL pointers and for an empty
> list, doing list_del if neither of these conditions is true.
>
>> in addition：after we carefully consideration： you below patch  still
>> may happen some task unlinking from session->cmd_list， when  there is
>> only one task in session->cmd_list，now task->c_hlist.next and
>> task->c_hlist.prev  is equal， according to you patch logic， this task
>> will not do list_del。
>
> Thank you for the heads up!
>
> Yep, now I use list_empty to check for list emptiness (it compares
> &c_hlist with c_hlist->head internally). Attaching the patch below.
>
> Are there any reasons for checking opcode then?
>
>
>
> --
> Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia
>

[-- Attachment #2: v2-0001-iscsi-fix-segfault-at-conn_close.patch --]
[-- Type: application/octet-stream, Size: 2506 bytes --]

From c0d3f1498122430a166aa304e4e58139efec788a Mon Sep 17 00:00:00 2001
From: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
Date: Tue, 26 Jul 2016 11:33:39 +0800
Subject: [PATCH v2] iscsi: fix segfault at conn_close

Remove some iscsi task from conn->session->cmd_list before free it,
otherwise it may cause tgtd process crash. Below is a backtrace info:

 Program terminated with signal 11, Segmentation fault.
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 83  prev->next = next;
 (gdb) bt
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
 #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at iscsi/iscsid.c:1254
 #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0, scmd=0x623d0e0) at iscsi/iscsid.c:1269
 #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0, result=0) at target.c:1236
 #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1, data=0x0) at bs.c:210
 #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
 #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

Signed-off-by: Meng Lingkun <menglingkun@cmss.chinamobile.com>
Signed-off-by: Wang Zhengyong <wangzhengyong@cmss.chinamobile.com>
Reviewed-by: Wang Dongxu <wangdongxu@cmss.chinamobile.com>
Signed-off-by: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
---
 usr/iscsi/iscsid.c | 4 +++-
 usr/iscsi/iscsid.h | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 75faae5..106ec65 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -1227,6 +1227,9 @@ void iscsi_free_task(struct iscsi_task *task)
 
 	list_del(&task->c_siblings);
 
+	if (task_opcode(task) == ISCSI_OP_SCSI_CMD)
+		list_del(&task->c_hlist);
+
 	conn->tp->free_data_buf(conn, scsi_get_in_buffer(&task->scmd));
 	conn->tp->free_data_buf(conn, scsi_get_out_buffer(&task->scmd));
 
@@ -1251,7 +1254,6 @@ void iscsi_free_cmd_task(struct iscsi_task *task)
 {
 	target_cmd_done(&task->scmd);
 
-	list_del(&task->c_hlist);
 	iscsi_free_task(task);
 }
 
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index c7f6801..b4f0439 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -60,6 +60,8 @@
 
 #define sid_to_tsih(sid) ((sid) >> 48)
 
+#define task_opcode(task) ((task)->req.opcode & ISCSI_OPCODE_MASK)
+
 struct iscsi_pdu {
 	struct iscsi_hdr bhs;
 	void *ahs;
-- 
1.8.3.1

Re: fix stgt crash in conn_close

[Anton Kovalenko]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]


Hi,

fx chen <chenfx2014@gmail.com> writes:

> we also consiger you idea，in iscsi_free_task, do task unlinking from
> session->cmd_list(list_del(&task->c_hlist) )，but we must know，such as
> ISCSI_OP_NOOP_OUT, ISCSI_OP_SCSI_TMFUNC,  ISCSI_OP_LOGOUT type task,
> they don‘t add to session->cmd_list,   To solve the problem， we offer
> patch<v2-0001-iscsi-fix-segfault-at-conn_close>。

When a task is freshly allocated by iscsi_alloc_task, it has an empty
c_hlist (see INIT_LIST_HEAD in iscsi_alloc_task), that will remain the
same until iscsi_free_task if the task is never linked to a cmd_list.

Therefore doing nothing with an empty c_hlist is enough (even list_del
on an empty c_hlist would do no harm). That would skip ISCSI tasks which
are not SCSI commands, exactly because they never get added to cmd_list.

There are also two cases where c_hlist has prev == next == NULL, one of
them when a task is allocated directly with iscsi_tcp_alloc_task (it
happens for NOOP-IN "pings", and such a task should never be
iscsi_free_task'ed), another one when a task used to be on a cmd_list,
and then list_del was called (list_del sets prev == next == NULL).

So I believe it's enough to check for NULL pointers and for an empty
list, doing list_del if neither of these conditions is true.

> in addition：after we carefully consideration： you below patch  still
> may happen some task unlinking from session->cmd_list， when  there is
> only one task in session->cmd_list，now task->c_hlist.next and
> task->c_hlist.prev  is equal， according to you patch logic， this task
> will not do list_del。

Thank you for the heads up!

Yep, now I use list_empty to check for list emptiness (it compares
&c_hlist with c_hlist->head internally). Attaching the patch below.

Are there any reasons for checking opcode then?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Unlink-task-from-c_hlist-before-free-if-needed.patch --]
[-- Type: text/x-diff, Size: 799 bytes --]

From 0c105acd248693530f31c97ab20e33ba760e42e9 Mon Sep 17 00:00:00 2001
From: Anton Kovalenko <anton.kovalenko@acronis.com>
Date: Tue, 26 Jul 2016 10:46:35 +0300
Subject: [PATCH] Unlink task from c_hlist before free if needed

---
 usr/iscsi/iscsid.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 75faae5..044819e 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -1227,6 +1227,11 @@ void iscsi_free_task(struct iscsi_task *task)
 
 	list_del(&task->c_siblings);
 
+	if (task->c_hlist.prev && task->c_hlist.next
+	    && !list_empty(&task->c_hlist)) {
+		list_del(&task->c_hlist);
+	}
+
 	conn->tp->free_data_buf(conn, scsi_get_in_buffer(&task->scmd));
 	conn->tp->free_data_buf(conn, scsi_get_out_buffer(&task->scmd));
 
-- 
2.8.1


[-- Attachment #3: Type: text/plain, Size: 75 bytes --]


-- 
Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia

Re: fix stgt crash in conn_close

[fx chen]

[-- Attachment #1: Type: text/plain, Size: 2623 bytes --]

>
>> hello!
>> we found tgtd happen core dump and fix it。

[...]

>On a second though, I think a conditional list_del(&task->c_hlist) in
>iscsi_free_task is good enough,

Hi  Anton Kovalenko:
we also consiger you idea，in iscsi_free_task, do task unlinking from
session->cmd_list(list_del(&task->c_hlist) )，but we must know，such as
ISCSI_OP_NOOP_OUT, ISCSI_OP_SCSI_TMFUNC,  ISCSI_OP_LOGOUT type task,
they don‘t add to session->cmd_list,   To solve the problem， we offer
patch<v2-0001-iscsi-fix-segfault-at-conn_close>。

in addition：after we carefully consideration： you below patch  still
may happen some task unlinking from session->cmd_list， when  there is
only one task in session->cmd_list，now task->c_hlist.next and
task->c_hlist.prev  is equal， according to you patch logic， this task
will not do list_del。
diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index b7ee0ad..dbb80a7 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -1225,6 +1225,12 @@ void iscsi_free_task(struct iscsi_task *task)

  list_del(&task->c_siblings);

+ if (task->c_hlist.next != task->c_hlist.prev) {
+ eprintf("task on c_hlist: %p %p %p\n",
+ task, task->c_hlist.prev, task->c_hlist.next);
+ list_del(&task->c_hlist);
+ }
+
  conn->tp->free_data_buf(conn, scsi_get_in_buffer(&task->scmd));
  conn->tp->free_data_buf(conn, scsi_get_out_buffer(&task->scmd));

2016-07-25 16:11 GMT+08:00 Anton Kovalenko <anton.kovalenko@acronis.com>:
> Anton Kovalenko <anton.kovalenko@acronis.com> writes:
>
>>
>>> hello!
>>> we found tgtd happen core dump and fix it。
>
> [...]
>
>> I'm attaching my own version of a preliminary fix, that avoids examining
>> the entire cmd_list on each task deallocation.
>
> On a second though, I think a conditional list_del(&task->c_hlist) in
> iscsi_free_task is good enough, but then we'd probably get rid of the
> *unconditional* list_del in iscsi_free_cmd_task, making iscsi_free_task
> responsible for task unlinking from c_hlist (it *is* responsible for
> unlinking from c_siblings anyway).
>
> What bothers me now is that a task removed from cmdlist, being a SCSI
> command, is probably not supposed to be freed without calling
> target_cmd_done (or is it?). I'm unsure if it might cause a resource
> leak of some kind.
>
>
> --
> Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia
>
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: v2-0001-iscsi-fix-segfault-at-conn_close.patch --]
[-- Type: application/octet-stream, Size: 2506 bytes --]

From c0d3f1498122430a166aa304e4e58139efec788a Mon Sep 17 00:00:00 2001
From: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
Date: Tue, 26 Jul 2016 11:33:39 +0800
Subject: [PATCH v2] iscsi: fix segfault at conn_close

Remove some iscsi task from conn->session->cmd_list before free it,
otherwise it may cause tgtd process crash. Below is a backtrace info:

 Program terminated with signal 11, Segmentation fault.
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 83  prev->next = next;
 (gdb) bt
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
 #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at iscsi/iscsid.c:1254
 #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0, scmd=0x623d0e0) at iscsi/iscsid.c:1269
 #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0, result=0) at target.c:1236
 #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1, data=0x0) at bs.c:210
 #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
 #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

Signed-off-by: Meng Lingkun <menglingkun@cmss.chinamobile.com>
Signed-off-by: Wang Zhengyong <wangzhengyong@cmss.chinamobile.com>
Reviewed-by: Wang Dongxu <wangdongxu@cmss.chinamobile.com>
Signed-off-by: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
---
 usr/iscsi/iscsid.c | 4 +++-
 usr/iscsi/iscsid.h | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 75faae5..106ec65 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -1227,6 +1227,9 @@ void iscsi_free_task(struct iscsi_task *task)
 
 	list_del(&task->c_siblings);
 
+	if (task_opcode(task) == ISCSI_OP_SCSI_CMD)
+		list_del(&task->c_hlist);
+
 	conn->tp->free_data_buf(conn, scsi_get_in_buffer(&task->scmd));
 	conn->tp->free_data_buf(conn, scsi_get_out_buffer(&task->scmd));
 
@@ -1251,7 +1254,6 @@ void iscsi_free_cmd_task(struct iscsi_task *task)
 {
 	target_cmd_done(&task->scmd);
 
-	list_del(&task->c_hlist);
 	iscsi_free_task(task);
 }
 
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index c7f6801..b4f0439 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -60,6 +60,8 @@
 
 #define sid_to_tsih(sid) ((sid) >> 48)
 
+#define task_opcode(task) ((task)->req.opcode & ISCSI_OPCODE_MASK)
+
 struct iscsi_pdu {
 	struct iscsi_hdr bhs;
 	void *ahs;
-- 
1.8.3.1

Re: fix stgt crash in conn_close

[Anton Kovalenko]

Anton Kovalenko <anton.kovalenko@acronis.com> writes:

>
>> hello!
>> we found tgtd happen core dump and fix it。

[...]

> I'm attaching my own version of a preliminary fix, that avoids examining
> the entire cmd_list on each task deallocation.

On a second though, I think a conditional list_del(&task->c_hlist) in
iscsi_free_task is good enough, but then we'd probably get rid of the
*unconditional* list_del in iscsi_free_cmd_task, making iscsi_free_task
responsible for task unlinking from c_hlist (it *is* responsible for
unlinking from c_siblings anyway).

What bothers me now is that a task removed from cmdlist, being a SCSI
command, is probably not supposed to be freed without calling
target_cmd_done (or is it?). I'm unsure if it might cause a resource
leak of some kind.


-- 
Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia

Re: fix stgt crash in conn_close

[Anton Kovalenko]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]


fx chen writes:

> hello!
> we found tgtd happen core dump and fix it。

[...]

Hi,

I believe we've hit the same problem recently (a free'd command hanging
on a session's cmd_list), and I have the same suspicion about conn_close.

I'm attaching my own version of a preliminary fix, that avoids examining
the entire cmd_list on each task deallocation.

I make use of the fact that a task's c_hlist during iscsi_free_task is
one of the following:

* an empty list head (task->c_hlist->next == task->c_hlist), if a task
  was allocated by iscsi_alloc_task and c_hlist was INIT_LIST_HEAD'ed
  and never touched
  
* a pair of null pointers for next and prev, if a task has been
  list_del'ed from a cmd_list (or allocated by raw iscsi_tcp_alloc_task,
  with all fields memset to 0 -- happens to "ping" NOOP-INs).

* a valid non-empty member of a cmd_list chain, in all other cases
  (prev!=next captures this condition perfectly).

Therefore when we have c_hlist->prev!=c_hlist->next in iscsi_free_task,
we can list_del safely, removing the task from whatever cmd_list it was
on.

Do you see any problem with my version of a fix? (see attachment)

P.S.
I believe it would be better to learn _where exactly_ does conn_close
call iscsi_free_task instead of iscsi_free_cmd_task, and why it happens,
and fix it more "precisely" in that place. 

However, having a crude, overprotective fix is better than not having
any. And as crude fixes go, I'd prefer the one which doesn't walk
cmd_list on a task deallocation.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: task_chlist_cleanup_on_free.patch --]
[-- Type: text/x-diff, Size: 550 bytes --]

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index b7ee0ad..dbb80a7 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -1225,6 +1225,12 @@ void iscsi_free_task(struct iscsi_task *task)
 
 	list_del(&task->c_siblings);
 
+	if (task->c_hlist.next != task->c_hlist.prev) {
+		eprintf("task on c_hlist: %p %p %p\n",
+			task, task->c_hlist.prev, task->c_hlist.next);
+		list_del(&task->c_hlist);
+	}
+
 	conn->tp->free_data_buf(conn, scsi_get_in_buffer(&task->scmd));
 	conn->tp->free_data_buf(conn, scsi_get_out_buffer(&task->scmd));
 

[-- Attachment #3: Type: text/plain, Size: 75 bytes --]


-- 
Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia

fix stgt crash in conn_close

[fx chen]

[-- Attachment #1: Type: text/plain, Size: 5891 bytes --]

hello!
we found tgtd happen core dump and fix it。

1： we use the stgt code
      git clone https://github.com/fujita/tgt.git
      stgt git commit version:  44c11763d71dc019741c84a857080cd0b4a2f265

2:   our test cases
      export 10 sata disk with stgt，each sata disk capacity is 100GB,
each target have one lun， in iscsi initiator import， using FIO to to
parallel random write test for 10 devices，
      during this period the frequent disconnection of iscsi networks。

3:  tgtd crash, happen core dump
     test  about 5 hours，tgtd process crash， happen core dump， the
stack trace showed  below:
     [New LWP 121362]
     [New LWP 121349]
     [New LWP 121350]
     [New LWP 121370]
     [New LWP 121404]
     [New LWP 121365]
     [New LWP 121348]
     [Thread debugging using libthread_db enabled]
     Using host libthread_db library "/lib64/libthread_db.so.1".
     Core was generated by `tgtd -d 1'.
     Program terminated with signal 11, Segmentation fault.
     #0  0x000000000040a259 in __list_del (prev=0x7cd38c0,
next=0x5aff340) at ./list.h:83
     83 prev->next = next;
     Missing separate debuginfos, use: debuginfo-install
libgcc-4.8.3-9.el7.x86_64 libgcc-4.8.5-4.el7.x86_64
     (gdb) bt
    #0  0x000000000040a259 in __list_del (prev=0x7cd38c0,
next=0x5aff340) at ./list.h:83
    #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
    #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at
iscsi/iscsid.c:1254
    #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0,
scmd=0x623d0e0) at iscsi/iscsid.c:1269
    #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0,
result=0) at target.c:1236
    #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1,
data=0x0) at bs.c:210
    #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
    #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

4:  Our analysis of tgtd crash
      beause the IO during the frequent  connection/disconnection of
iscsi networks，this will trigger close current iscsi session，
      lead to stgt frequent call conn_close function,  conn_close will
clear all iscsi task，we found some ISCSI_OP_SCSI_CMD type's task,
      no list_del from session->cmd_list before free it.
5: Our patch for tgtd crash
From 2abf9229ea206172f2fe244539f7f87ed404eff8 Mon Sep 17 00:00:00 2001
From: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
Date: Fri, 22 Jul 2016 10:37:23 +0800
Subject: [PATCH] iscsi: fix segfault at conn_close

Remove some iscsi task from conn->session->cmd_list before free it,
otherwise it may cause tgtd process crash. Below is a backtrace info:

 Program terminated with signal 11, Segmentation fault.
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340)
at ./list.h:83
 83  prev->next = next;
 (gdb) bt
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340)
at ./list.h:83
 #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
 #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at
iscsi/iscsid.c:1254
 #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0,
scmd=0x623d0e0) at iscsi/iscsid.c:1269
 #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0,
result=0) at target.c:1236
 #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1,
data=0x0) at bs.c:210
 #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
 #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

Signed-off-by: Meng Lingkun <menglingkun@cmss.chinamobile.com>
Signed-off-by: Wang Zhengyong <wangzhengyong@cmss.chinamobile.com>
Reviewed-by: Wang Dongxu <wangdongxu@cmss.chinamobile.com>
Signed-off-by: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
---
 usr/iscsi/conn.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/usr/iscsi/conn.c b/usr/iscsi/conn.c
index e7d4e8c..3ed08a8 100644
--- a/usr/iscsi/conn.c
+++ b/usr/iscsi/conn.c
@@ -83,6 +83,22 @@ void conn_exit(struct iscsi_connection *conn)
  session_put(session);
 }

+static int find_task_in_cmd_list(struct iscsi_connection *conn,
+ struct iscsi_task *find_task)
+{
+ struct iscsi_task *task = NULL;
+ int find = 0;
+
+ list_for_each_entry(task, &conn->session->cmd_list, c_hlist) {
+ if (task == find_task) {
+ find = 1;
+ break;
+ }
+ }
+
+ return find;
+}
+
 void conn_close(struct iscsi_connection *conn)
 {
  struct iscsi_task *task, *tmp;
@@ -134,6 +150,7 @@ void conn_close(struct iscsi_connection *conn)
  list_for_each_entry_safe(task, tmp, &conn->tx_clist, c_list) {
  uint8_t op;

+ list_del(&task->c_list);
  op = task->req.opcode & ISCSI_OPCODE_MASK;

  eprintf("Forcing release of tx task %p %" PRIx64 " %x\n",
@@ -146,10 +163,14 @@ void conn_close(struct iscsi_connection *conn)
   * would be a better way to see
   * task->scmd.c_target though.
   */
- if (task->scmd.c_target)
+ if (task->scmd.c_target) {
  iscsi_free_cmd_task(task);
- else
+ } else {
+ if (find_task_in_cmd_list(conn, task))
+ list_del(&task->c_hlist);
+
  iscsi_free_task(task);
+ }
  break;
  case ISCSI_OP_NOOP_IN:
  /* NOOP_IN req is allocated within iscsi_tcp
@@ -181,6 +202,9 @@ void conn_close(struct iscsi_connection *conn)
  if (conn->rx_task) {
  eprintf("Forcing release of rx task %p %" PRIx64 "\n",
  conn->rx_task, conn->rx_task->tag);
+ if (find_task_in_cmd_list(conn, conn->rx_task))
+ list_del(&conn->rx_task->c_hlist);
+
  iscsi_free_task(conn->rx_task);
  }
  conn->rx_task = NULL;
@@ -193,6 +217,10 @@ void conn_close(struct iscsi_connection *conn)
   */
  if (task_in_scsi(task))
  continue;
+
+ if (find_task_in_cmd_list(conn, task))
+ list_del(&task->c_hlist);
+
  iscsi_free_task(task);
  }
 done:
-- 
1.8.3.1

[-- Attachment #2: 0001-iscsi-fix-segfault-at-conn_close.patch --]
[-- Type: application/octet-stream, Size: 3493 bytes --]

From 2abf9229ea206172f2fe244539f7f87ed404eff8 Mon Sep 17 00:00:00 2001
From: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
Date: Fri, 22 Jul 2016 10:37:23 +0800
Subject: [PATCH] iscsi: fix segfault at conn_close

Remove some iscsi task from conn->session->cmd_list before free it,
otherwise it may cause tgtd process crash. Below is a backtrace info:

 Program terminated with signal 11, Segmentation fault.
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 83  prev->next = next;
 (gdb) bt
 #0  0x000000000040a259 in __list_del (prev=0x7cd38c0, next=0x5aff340) at ./list.h:83
 #1  0x000000000040a296 in list_del (entry=0x623d080) at ./list.h:88
 #2  0x000000000040edd6 in iscsi_free_cmd_task (task=0x623d010) at iscsi/iscsid.c:1254
 #3  0x000000000040ee6a in iscsi_scsi_cmd_done (nid=3356, result=0, scmd=0x623d0e0) at iscsi/iscsid.c:1269
 #4  0x0000000000432dc0 in target_cmd_io_done (cmd=0x623d0e0, result=0) at target.c:1236
 #5  0x000000000045be39 in bs_sig_request_done (fd=10, events=1, data=0x0) at bs.c:210
 #6  0x0000000000428ff2 in event_loop () at tgtd.c:432
 #7  0x0000000000429fca in main (argc=3, argv=0x7ffffcec25f8) at tgtd.c:624

Signed-off-by: Meng Lingkun <menglingkun@cmss.chinamobile.com>
Signed-off-by: Wang Zhengyong <wangzhengyong@cmss.chinamobile.com>
Reviewed-by: Wang Dongxu <wangdongxu@cmss.chinamobile.com>
Signed-off-by: Chen Fangxian <chenfangxian@cmss.chinamobile.com>
---
 usr/iscsi/conn.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/usr/iscsi/conn.c b/usr/iscsi/conn.c
index e7d4e8c..3ed08a8 100644
--- a/usr/iscsi/conn.c
+++ b/usr/iscsi/conn.c
@@ -83,6 +83,22 @@ void conn_exit(struct iscsi_connection *conn)
 		session_put(session);
 }
 
+static int find_task_in_cmd_list(struct iscsi_connection *conn,
+				struct iscsi_task *find_task)
+{
+	struct iscsi_task *task = NULL;
+	int find = 0;
+
+	list_for_each_entry(task, &conn->session->cmd_list, c_hlist) {
+		if (task == find_task) {
+			find = 1;
+			break;
+		}
+	}
+
+	return find;
+}
+
 void conn_close(struct iscsi_connection *conn)
 {
 	struct iscsi_task *task, *tmp;
@@ -134,6 +150,7 @@ void conn_close(struct iscsi_connection *conn)
 	list_for_each_entry_safe(task, tmp, &conn->tx_clist, c_list) {
 		uint8_t op;
 
+		list_del(&task->c_list);
 		op = task->req.opcode & ISCSI_OPCODE_MASK;
 
 		eprintf("Forcing release of tx task %p %" PRIx64 " %x\n",
@@ -146,10 +163,14 @@ void conn_close(struct iscsi_connection *conn)
 			 * would be a better way to see
 			 * task->scmd.c_target though.
 			 */
-			if (task->scmd.c_target)
+			if (task->scmd.c_target) {
 				iscsi_free_cmd_task(task);
-			else
+			} else {
+				if (find_task_in_cmd_list(conn, task))
+					list_del(&task->c_hlist);
+
 				iscsi_free_task(task);
+			}
 			break;
 		case ISCSI_OP_NOOP_IN:
 			/* NOOP_IN req is allocated within iscsi_tcp
@@ -181,6 +202,9 @@ void conn_close(struct iscsi_connection *conn)
 	if (conn->rx_task) {
 		eprintf("Forcing release of rx task %p %" PRIx64 "\n",
 			conn->rx_task, conn->rx_task->tag);
+		if (find_task_in_cmd_list(conn, conn->rx_task))
+			list_del(&conn->rx_task->c_hlist);
+
 		iscsi_free_task(conn->rx_task);
 	}
 	conn->rx_task = NULL;
@@ -193,6 +217,10 @@ void conn_close(struct iscsi_connection *conn)
 		 */
 		if (task_in_scsi(task))
 			continue;
+
+		if (find_task_in_cmd_list(conn, task))
+			list_del(&task->c_hlist);
+
 		iscsi_free_task(task);
 	}
 done:
-- 
1.8.3.1