From 5bbc7433dc7f7d5d4311a3c1656410675c59b00b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 26 Apr 2016 04:13:42 +0000 Subject: [PATCH 1/5] scst: Move kref_get_unless_zero() definition to This patch is a slightly modified version of a patch supplied by Israel Rukshin . git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6875 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- fcst/ft_sess.c | 19 ------------------- scst/include/backport.h | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/fcst/ft_sess.c b/fcst/ft_sess.c index f98973408..72754cd87 100644 --- a/fcst/ft_sess.c +++ b/fcst/ft_sess.c @@ -172,25 +172,6 @@ static u32 ft_sess_hash(u32 port_id) return hash_32(port_id, FT_SESS_HASH_BITS); } -#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 8, 0) && \ - ! (LINUX_VERSION_CODE >> 8 == KERNEL_VERSION(3, 4, 0) >> 8 && \ - LINUX_VERSION_CODE >= KERNEL_VERSION(3, 4, 41)) && \ - ! (LINUX_VERSION_CODE >> 8 == KERNEL_VERSION(3, 2, 0) >> 8 && \ - LINUX_VERSION_CODE >= KERNEL_VERSION(3, 2, 44)) && \ - !defined(CONFIG_SUSE_KERNEL) && \ - (!defined(RHEL_MAJOR) || RHEL_MAJOR -0 < 6 || \ - (RHEL_MAJOR -0 == 6 && RHEL_MINOR -0 < 6)) -/* - * See also commit 4b20db3 (kref: Implement kref_get_unless_zero v3 -- v3.8). - * See also commit e3a5505 in branch stable/linux-3.4.y (v3.4.41). - * See also commit 3fa8ee5 in branch stable/linux-3.2.y (v3.2.44). - */ -static inline int __must_check kref_get_unless_zero(struct kref *kref) -{ - return atomic_add_unless(&kref->refcount, 1, 0); -} -#endif - /* * Find session in local port. * Sessions and hash lists are RCU-protected. diff --git a/scst/include/backport.h b/scst/include/backport.h index 5d12d023b..61c5e9864 100644 --- a/scst/include/backport.h +++ b/scst/include/backport.h @@ -327,6 +327,27 @@ enum umh_wait { }; #endif +/* */ + +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 8, 0) && \ + ! (LINUX_VERSION_CODE >> 8 == KERNEL_VERSION(3, 4, 0) >> 8 && \ + LINUX_VERSION_CODE >= KERNEL_VERSION(3, 4, 41)) && \ + ! (LINUX_VERSION_CODE >> 8 == KERNEL_VERSION(3, 2, 0) >> 8 && \ + LINUX_VERSION_CODE >= KERNEL_VERSION(3, 2, 44)) && \ + !defined(CONFIG_SUSE_KERNEL) && \ + (!defined(RHEL_MAJOR) || RHEL_MAJOR -0 < 6 || \ + (RHEL_MAJOR -0 == 6 && RHEL_MINOR -0 < 6)) +/* + * See also commit 4b20db3 (kref: Implement kref_get_unless_zero v3 -- v3.8). + * See also commit e3a5505 in branch stable/linux-3.4.y (v3.4.41). + * See also commit 3fa8ee5 in branch stable/linux-3.2.y (v3.2.44). + */ +static inline int __must_check kref_get_unless_zero(struct kref *kref) +{ + return atomic_add_unless(&kref->refcount, 1, 0); +} +#endif + /* */ #ifndef __list_for_each From f297ef6f3818ebbea0e857c109e21b6c62a87d8f Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 26 Apr 2016 14:12:17 +0000 Subject: [PATCH 2/5] isert: fix double free of a fake request A nop that is sent from the scst is treated as a fake request. When a fake request is allocated we release it immediately, so it's wrong to release it also on a completion error (isert_pdu_err). This commit fix a NULL dereference bug when receiving completion with error on this nop. Signed-off-by: Israel Rukshin Signed-off-by: Ariel Nahum git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6876 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/isert-scst/iser_rdma.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/iscsi-scst/kernel/isert-scst/iser_rdma.c b/iscsi-scst/kernel/isert-scst/iser_rdma.c index 59e44dbb3..fc038695d 100644 --- a/iscsi-scst/kernel/isert-scst/iser_rdma.c +++ b/iscsi-scst/kernel/isert-scst/iser_rdma.c @@ -637,7 +637,7 @@ static void isert_handle_wc_error(struct ib_wc *wc) #endif if (unlikely(num_sge == 0)) /* Drain WR */ isert_sched_conn_drained(isert_conn); - else + else if (!isert_pdu->is_fake_rx) isert_pdu_err(&isert_pdu->iscsi); break; case ISER_WR_RDMA_READ: @@ -646,7 +646,8 @@ static void isert_handle_wc_error(struct ib_wc *wc) isert_buf->dma_dir); isert_buf->sg_cnt = 0; } - isert_pdu_err(&isert_pdu->iscsi); + if (!isert_pdu->is_fake_rx) + isert_pdu_err(&isert_pdu->iscsi); break; case ISER_WR_RECV: /* this should be the Flush, no task has been created yet */ From 0283a854ed764ff3799a11e1f04dbd09ec40c816 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 26 Apr 2016 14:12:47 +0000 Subject: [PATCH 3/5] isert: wait for RDMA CM event to complete before destroy resources There is a race between RDMACM event handler and isert_conn_free. The event handler use the connection resources and isert_conn_free destroy them. This commit fix multiple NULL dereference bugs. Signed-off-by: Israel Rukshin git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6877 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/isert-scst/iser_rdma.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/iscsi-scst/kernel/isert-scst/iser_rdma.c b/iscsi-scst/kernel/isert-scst/iser_rdma.c index fc038695d..92f98fcb8 100644 --- a/iscsi-scst/kernel/isert-scst/iser_rdma.c +++ b/iscsi-scst/kernel/isert-scst/iser_rdma.c @@ -1157,12 +1157,6 @@ fail_create_qp: goto out; } -static void isert_conn_qp_destroy(struct isert_connection *isert_conn) -{ - rdma_destroy_qp(isert_conn->cm_id); - isert_conn->qp = NULL; -} - static struct isert_connection *isert_conn_create(struct rdma_cm_id *cm_id, struct isert_device *isert_dev) { @@ -1239,7 +1233,7 @@ fail_post_recv: mutex_lock(&dev_list_mutex); isert_dev->cq_qps[cq->idx]--; mutex_unlock(&dev_list_mutex); - isert_conn_qp_destroy(isert_conn); + rdma_destroy_qp(isert_conn->cm_id); fail_qp: isert_pdu_free(isert_conn->login_rsp_pdu); fail_login_rsp_pdu: @@ -1273,7 +1267,9 @@ static void isert_kref_free(struct kref *kref) isert_free_conn_resources(isert_conn); - isert_conn_qp_destroy(isert_conn); + rdma_destroy_id(isert_conn->cm_id); + ib_destroy_qp(isert_conn->qp); + isert_conn->qp = NULL; mutex_lock(&dev_list_mutex); isert_dev->cq_qps[cq->idx]--; @@ -1283,8 +1279,6 @@ static void isert_kref_free(struct kref *kref) isert_portal_free(isert_conn->portal); mutex_unlock(&dev_list_mutex); - rdma_destroy_id(isert_conn->cm_id); - isert_conn_kfree(isert_conn); module_put(THIS_MODULE); From 07bc4bb1af7f241a5bfa95e0e9bb7ca389b7a9c5 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 26 Apr 2016 14:13:13 +0000 Subject: [PATCH 4/5] isert: clean tmo timer when freeing the connection The tmo timer is set when allocating a new connection. We need to release it in case it didn't execute yet. Else timer execution will lead to a NULL dereference bug on conn. Signed-off-by: Israel Rukshin git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6878 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/isert-scst/iser_rdma.c | 7 +++++++ iscsi-scst/kernel/isert-scst/isert.h | 1 + iscsi-scst/kernel/isert-scst/isert_login.c | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/iscsi-scst/kernel/isert-scst/iser_rdma.c b/iscsi-scst/kernel/isert-scst/iser_rdma.c index 92f98fcb8..fe98696ec 100644 --- a/iscsi-scst/kernel/isert-scst/iser_rdma.c +++ b/iscsi-scst/kernel/isert-scst/iser_rdma.c @@ -42,6 +42,7 @@ #include "isert_dbg.h" #include "iser.h" +#include "isert.h" #include "iser_datamover.h" #define ISER_CQ_ENTRIES (128 * 1024) @@ -1255,6 +1256,7 @@ static void isert_deref_device(struct isert_device *isert_dev) static void isert_kref_free(struct kref *kref) { + struct isert_conn_dev *dev; struct isert_connection *isert_conn = container_of(kref, struct isert_connection, kref); @@ -1268,6 +1270,11 @@ static void isert_kref_free(struct kref *kref) isert_free_conn_resources(isert_conn); rdma_destroy_id(isert_conn->cm_id); + + dev = isert_get_priv(&isert_conn->iscsi); + if (dev) + isert_del_timer(dev); + ib_destroy_qp(isert_conn->qp); isert_conn->qp = NULL; diff --git a/iscsi-scst/kernel/isert-scst/isert.h b/iscsi-scst/kernel/isert-scst/isert.h index 8dde1a70a..0c5113edf 100644 --- a/iscsi-scst/kernel/isert-scst/isert.h +++ b/iscsi-scst/kernel/isert-scst/isert.h @@ -131,5 +131,6 @@ int isert_conn_alloc(struct iscsi_session *session, struct iscsit_transport *t); int isert_handle_close_connection(struct iscsi_conn *conn); void isert_close_all_portals(void); +void isert_del_timer(struct isert_conn_dev *dev); #endif /* __ISERT_H__ */ diff --git a/iscsi-scst/kernel/isert-scst/isert_login.c b/iscsi-scst/kernel/isert-scst/isert_login.c index 0a3720306..9d78bb1bc 100644 --- a/iscsi-scst/kernel/isert-scst/isert_login.c +++ b/iscsi-scst/kernel/isert-scst/isert_login.c @@ -83,7 +83,7 @@ static struct isert_conn_dev *get_available_dev(struct isert_listener_dev *dev, return res; } -static void isert_del_timer(struct isert_conn_dev *dev) +void isert_del_timer(struct isert_conn_dev *dev) { if (dev->timer_active) { dev->timer_active = 0; From 76b20c76b838c49aba790ff600d52de5eb4a7ba3 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 26 Apr 2016 14:13:33 +0000 Subject: [PATCH 5/5] isert: fix race between teardown flow and connect flow It is possible to get ESTABLISHED RDMACM event while the connection is already started teardown flow (i.e. addr change). At teardown the refcount is reduced to zero and we start releasing the connection. In established event We need to check conn is not in teardown flow by checking its ref count is not 0. Signed-off-by: Israel Rukshin git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6879 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/isert-scst/iser_rdma.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/iscsi-scst/kernel/isert-scst/iser_rdma.c b/iscsi-scst/kernel/isert-scst/iser_rdma.c index fe98696ec..9f50728d9 100644 --- a/iscsi-scst/kernel/isert-scst/iser_rdma.c +++ b/iscsi-scst/kernel/isert-scst/iser_rdma.c @@ -1453,7 +1453,10 @@ static int isert_cm_connect_handler(struct rdma_cm_id *cm_id, if (unlikely(ret)) goto out; - kref_get(&isert_conn->kref); + /* check if already started teardown */ + if (!unlikely(kref_get_unless_zero(&isert_conn->kref))) + goto out; + /* notify upper layer */ ret = isert_conn_established(&isert_conn->iscsi, (struct sockaddr *)&isert_conn->peer_addr,