From a391176a6dece09454672c6522778d6513e90fb4 Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Tue, 25 Feb 2020 22:35:54 +0800 Subject: [PATCH 1/7] init: fix memory leak in iscsi_create_context iscsi instance is allocated in iscsi_create_context, after we return NULL, nobody could handle it anymore. Currently we can't hit this logic, anyway we still fix this. Signed-off-by: zhenwei pi --- lib/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/init.c b/lib/init.c index 13098f0..f3e2e13 100644 --- a/lib/init.c +++ b/lib/init.c @@ -218,7 +218,7 @@ iscsi_create_context(const char *initiator_name) /* initalize transport of context */ if (iscsi_init_transport(iscsi, TCP_TRANSPORT)) { - iscsi_set_error(iscsi, "Failed allocating transport"); + free(iscsi); return NULL; } From e2a7fdfb367aea959b67841202c3566e96726ecf Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Wed, 26 Feb 2020 10:29:44 +0800 Subject: [PATCH 2/7] socket: fix disconnect corner case for iser iscsi->fd is never initialized in iser driver, so iscsi_disconnect always does not work for iser context. iscsi->fd is used as a member variable of TCP context, so let iscsi TCP driver handle iscsi->fd, we just call iscsi_disconnect in iscsi_destroy_context. Luckly, TCP driver has already handle invalid iscsi->fd case in iscsi_tcp_disconnect. And fix NULL pointer case for iscsi_disconnect. Signed-off-by: zhenwei pi --- lib/init.c | 4 +--- lib/socket.c | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/init.c b/lib/init.c index f3e2e13..a120e88 100644 --- a/lib/init.c +++ b/lib/init.c @@ -393,9 +393,7 @@ iscsi_destroy_context(struct iscsi_context *iscsi) return 0; } - if (iscsi->fd != -1) { - iscsi_disconnect(iscsi); - } + iscsi_disconnect(iscsi); iscsi_cancel_pdus(iscsi); diff --git a/lib/socket.c b/lib/socket.c index 79471a3..cd6c97a 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -425,6 +425,9 @@ iscsi_tcp_disconnect(struct iscsi_context *iscsi) int iscsi_disconnect(struct iscsi_context *iscsi) { + if (!iscsi || !iscsi->drv || !iscsi->drv->disconnect) + return -1; + return iscsi->drv->disconnect(iscsi); } From d020bb003dd9a8274c964c95f49d65f951324901 Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Wed, 26 Feb 2020 11:49:18 +0800 Subject: [PATCH 3/7] iser: set iser cm thread proc name as "iscsi_cm_thread" libiscsi is usually linked by QEMU, and QEMU sets thread proc name by function. But iser cm thread is created by libiscsi privately, QEMU can't set this thread. After attaching a iser disk, we can find a new thread 'qemu-system-x86' in QEMU process. With this patch, iser cm thread works with thread name 'iscsi_cm_thread'. Signed-off-by: zhenwei pi --- lib/iser.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/iser.c b/lib/iser.c index 66f51a7..3e50489 100644 --- a/lib/iser.c +++ b/lib/iser.c @@ -17,6 +17,7 @@ #include #include +#include #include #include #include "slist.h" @@ -48,6 +49,9 @@ #ifdef __linux +/* the name can be up to 16 bytes long, including the terminating null byte*/ +#define ISER_CM_THREAD_NAME "iscsi_cm_thread" + /* MUST keep in sync with socket.c */ union socket_address { struct sockaddr_in sin; @@ -1370,6 +1374,9 @@ static void *cm_thread(void *arg) int ret; struct iscsi_context *iscsi = iser_conn->cma_id->context; + /* supported since Linux 2.6.9, not fatal error, ignore return value */ + prctl(PR_SET_NAME, ISER_CM_THREAD_NAME); + while (1) { ret = rdma_get_cm_event(iser_conn->cma_channel, &iser_conn->cma_event); if (ret) { From b4ba92094e44eda1171f1f05d123d44f30e65866 Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Wed, 26 Feb 2020 19:53:16 +0800 Subject: [PATCH 4/7] iser: fix segfault at iser_reg_mr Hit segfault at iser_reg_mr during attaching disk with backtrace: #0 0x000055ace9635b0f in iser_reg_mr (iser_conn=0x55aceca33820) at iser.c:1060 #1 iser_connected_handler (cma_id=) at iser.c:1300 #2 iser_cma_handler (event=0x7f29ef1f7950, cma_id=, iser_conn=0x55aceca33820) at iser.c:1326 #3 cm_thread (arg=0x55aceca33820) at iser.c:1380 #4 0x00007f2e2c31c4a4 in start_thread (arg=0x7f29ef1f8700) at pthread_create.c:456 #5 0x00007f2e2c05ed0f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:97 (gdb) p *iser_conn->tx_desc Cannot access memory at address 0x20 This issue can be reproduced easily by attaching several disks of iser protocol: # virsh attach-device stretch iser0.xml # virsh attach-device stretch iser1.xml ... Initialize instances with zero to avoid random value pointer. Signed-off-by: zhenwei pi --- lib/iser.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/iser.c b/lib/iser.c index 3e50489..95f95ec 100644 --- a/lib/iser.c +++ b/lib/iser.c @@ -1036,7 +1036,7 @@ iser_reg_mr(struct iser_conn *iser_conn) for (i = 0 ; i < NUM_MRS ; i++) { - tx_desc = iscsi_malloc(iscsi, sizeof(*tx_desc)); + tx_desc = iscsi_zmalloc(iscsi, sizeof(*tx_desc)); if (tx_desc == NULL) { iscsi_set_error(iscsi, "Out-Of-Memory, failed to allocate data buffer"); return -1; @@ -1478,7 +1478,7 @@ static iscsi_transport iscsi_transport_iser = { void iscsi_init_iser_transport(struct iscsi_context *iscsi) { iscsi->drv = &iscsi_transport_iser; - iscsi->opaque = iscsi_malloc(iscsi, sizeof(struct iser_conn)); + iscsi->opaque = iscsi_zmalloc(iscsi, sizeof(struct iser_conn)); iscsi->transport = ISER_TRANSPORT; /* Update iSCSI params as per iSER transport */ iscsi->initiator_max_recv_data_segment_length = ISCSI_DEF_MAX_RECV_SEG_LEN; From e114be41560b848a1ec77c89bea908316fe9846c Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Wed, 26 Feb 2020 21:51:04 +0800 Subject: [PATCH 5/7] iser: fix memory leak for cm thread If a thread is created without any attr, it works in attached mode. It means that we need run pthread_join to relaim stack of thread. Signed-off-by: zhenwei pi --- lib/iser.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/iser.c b/lib/iser.c index 95f95ec..c727be1 100644 --- a/lib/iser.c +++ b/lib/iser.c @@ -198,6 +198,7 @@ iser_free_iser_conn_res(struct iser_conn *iser_conn, bool destroy) if (iser_conn->cmthread) { pthread_cancel(iser_conn->cmthread); + pthread_join(iser_conn->cmthread, NULL); iser_conn->cmthread = 0; } From 3ccbceb6ff118b41ae41da9b0e974cd838ada653 Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Thu, 27 Feb 2020 22:13:08 +0800 Subject: [PATCH 6/7] lib/connect.c: fix wrong transport type for iser reconnect A new iscsi context is created as TCP transport type, but currently missing iscsi_init_transport to change transport to iser in reconnecting logic, then iser could never reconnect successfully. Use orignal transport to initialize new iscsi context. Signed-off-by: zhenwei pi --- lib/connect.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/connect.c b/lib/connect.c index a17b4e5..ff28e23 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -418,6 +418,14 @@ int iscsi_reconnect(struct iscsi_context *iscsi) return -1; } + /* default transport is initialized as TCP in iscsi_create_context, + we have to overwrite transport in new iscsi as old iscsi. + */ + if (iscsi_init_transport(tmp_iscsi, iscsi->transport)) { + ISCSI_LOG(iscsi, 2, "failed to initializing transport for reconnection"); + return -1; + } + ISCSI_LOG(iscsi, 2, "reconnect initiated"); iscsi_set_targetname(tmp_iscsi, iscsi->target_name); From b98454ae976c1ab5d3abf395c19a875a3f68fb02 Mon Sep 17 00:00:00 2001 From: zhenwei pi Date: Fri, 28 Feb 2020 18:29:41 +0800 Subject: [PATCH 7/7] iser: fix resource leak during reconnect After iser reconnects successfully, iser drive should close old connection and release resources. Fix resource leak in this patch, and test a lot, this patch works fine. Test env: 192.168.122.204: run as a software gateway 192.168.122.205: run iser target, default gateway 192.168.122.204 192.168.122.206: run QEMU as intiator, default gateway 192.168.122.204 run script on 192.168.122.204: for i in `seq 1 100` do iptables -s 192.168.122.205/32 -A FORWARD -m statistic --mode random --probability 1 -j DROP iptables -s 192.168.122.206/32 -A FORWARD -m statistic --mode random --probability 1 -j DROP sleep 30 iptables -F sleep 30 done Signed-off-by: zhenwei pi --- lib/iser.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/iser.c b/lib/iser.c index c727be1..81f3687 100644 --- a/lib/iser.c +++ b/lib/iser.c @@ -294,8 +294,10 @@ iscsi_iser_disconnect(struct iscsi_context *iscsi) { struct iser_conn *iser_conn = iscsi->opaque; - iser_conn_terminate(iser_conn); - iser_conn_release(iser_conn); + if (iser_conn) { + iser_conn_terminate(iser_conn); + iser_conn_release(iser_conn); + } iscsi->fd = -1; iscsi->is_connected = 0; @@ -1462,6 +1464,25 @@ iscsi_iser_connect(struct iscsi_context *iscsi, union socket_address *sa,__attri iscsi->socket_status_cb(iscsi, SCSI_STATUS_GOOD, NULL, iscsi->connect_data); iscsi->socket_status_cb = NULL; + if (iscsi->old_iscsi && iscsi->opaque != iscsi->old_iscsi->opaque) { + struct iser_conn *old_iser_conn = iscsi->old_iscsi->opaque; + int oldfd = old_iser_conn->comp_channel->fd; + int newfd = iser_conn->comp_channel->fd; + + iser_conn_terminate(old_iser_conn); + iser_conn_release(old_iser_conn); + + if (dup2(newfd, oldfd) == -1) { + return -1; + } + + close(newfd); + iser_conn->comp_channel->fd = oldfd; + + iscsi_free(iscsi->old_iscsi, iscsi->old_iscsi->opaque); + iscsi->old_iscsi->opaque = NULL; + } + return 0; }