From 919597d141dd79b34a9c0ef9e52a63cc43320d6c Mon Sep 17 00:00:00 2001
From: Mohit Agrawal <moagrawa@redhat.com>
Date: Mon, 3 Dec 2018 17:05:19 +0530
Subject: [PATCH 453/453] server: Resolve memory leak path in server_init
Problem: 1) server_init does not cleanup allocate resources
while it is failed before return error
2) dict leak at the time of graph destroying
Solution: 1) free resources in case of server_init is failed
2) Take dict_ref of graph xlator before destroying
the graph to avoid leak
> Change-Id: I9e31e156b9ed6bebe622745a8be0e470774e3d15
> fixes: bz#1654917
> Cherry pick from commit 46c15ea8fa98bb3d92580b192f03863c2e2a2d9c
> Reviewed on upstream link https://review.gluster.org/#/c/glusterfs/+/21750/
Change-Id: I5ba1b37840bcaa4a7fa4c05822c84016a2d89ea2
BUG: 1650138
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/157445
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Reviewed-by: Raghavendra Gowdappa <rgowdapp@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
glusterfsd/src/glusterfsd.c | 4 +++
libglusterfs/src/xlator.c | 31 ++++++++++++++++++
libglusterfs/src/xlator.h | 3 ++
rpc/rpc-lib/src/rpc-transport.c | 26 ++++++++++++----
rpc/rpc-lib/src/rpc-transport.h | 3 ++
rpc/rpc-lib/src/rpcsvc.c | 40 ++++++++++++++++++++++++
rpc/rpc-lib/src/rpcsvc.h | 3 ++
xlators/mgmt/glusterd/src/glusterd-utils.c | 33 ++------------------
xlators/protocol/server/src/server.c | 50 +++++++++++++++++++++++++++---
9 files changed, 151 insertions(+), 42 deletions(-)
diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
index 2e43cdb..78f3719 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -2411,6 +2411,10 @@ out:
xl = graph->first;
if ((ctx && (ctx->active != graph)) &&
(xl && !strcmp(xl->type, "protocol/server"))) {
+ /* Take dict ref for every graph xlator to avoid dict leak
+ at the time of graph destroying
+ */
+ gluster_graph_take_reference(graph->first);
glusterfs_graph_fini(graph);
glusterfs_graph_destroy(graph);
}
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c
index 8aa8aa1..340d83d 100644
--- a/libglusterfs/src/xlator.c
+++ b/libglusterfs/src/xlator.c
@@ -1225,3 +1225,34 @@ glusterfs_delete_volfile_checksum (glusterfs_ctx_t *ctx,
return 0;
}
+
+/*
+ The function is required to take dict ref for every xlator at graph.
+ At the time of compare graph topology create a graph and populate
+ key values in the dictionary, after finished graph comparison we do destroy
+ the new graph.At the time of construct graph we don't take any reference
+ so to avoid dict leak at the of destroying graph due to ref counter underflow
+ we need to call dict_ref here.
+
+*/
+
+void
+gluster_graph_take_reference(xlator_t *tree)
+{
+ xlator_t *trav = tree;
+ xlator_t *prev = tree;
+
+ if (!tree) {
+ gf_msg("parser", GF_LOG_ERROR, 0, LG_MSG_TREE_NOT_FOUND,
+ "Translator tree not found");
+ return;
+ }
+
+ while (prev) {
+ trav = prev->next;
+ if (prev->options)
+ dict_ref(prev->options);
+ prev = trav;
+ }
+ return;
+}
diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h
index 1879641..f8f2630 100644
--- a/libglusterfs/src/xlator.h
+++ b/libglusterfs/src/xlator.h
@@ -1087,4 +1087,7 @@ glusterfs_delete_volfile_checksum (glusterfs_ctx_t *ctx,
const char *volfile_id);
int
xlator_memrec_free (xlator_t *xl);
+
+void
+gluster_graph_take_reference(xlator_t *tree);
#endif /* _XLATOR_H */
diff --git a/rpc/rpc-lib/src/rpc-transport.c b/rpc/rpc-lib/src/rpc-transport.c
index 94880f4..77abf96 100644
--- a/rpc/rpc-lib/src/rpc-transport.c
+++ b/rpc/rpc-lib/src/rpc-transport.c
@@ -160,6 +160,25 @@ out:
return msg;
}
+void
+rpc_transport_cleanup(rpc_transport_t *trans)
+{
+ if (!trans)
+ return;
+
+ if (trans->fini)
+ trans->fini(trans);
+
+ GF_FREE(trans->name);
+
+ if (trans->xl)
+ pthread_mutex_destroy(&trans->lock);
+
+ if (trans->dl_handle)
+ dlclose(trans->dl_handle);
+
+ GF_FREE(trans);
+}
rpc_transport_t *
@@ -361,12 +380,7 @@ rpc_transport_load (glusterfs_ctx_t *ctx, dict_t *options, char *trans_name)
fail:
if (trans) {
- GF_FREE (trans->name);
-
- if (trans->dl_handle)
- dlclose (trans->dl_handle);
-
- GF_FREE (trans);
+ rpc_transport_cleanup(trans);
}
GF_FREE (name);
diff --git a/rpc/rpc-lib/src/rpc-transport.h b/rpc/rpc-lib/src/rpc-transport.h
index 33f474e..23246c5 100644
--- a/rpc/rpc-lib/src/rpc-transport.h
+++ b/rpc/rpc-lib/src/rpc-transport.h
@@ -316,4 +316,7 @@ rpc_transport_unix_options_build (dict_t **options, char *filepath,
int
rpc_transport_inet_options_build (dict_t **options, const char *hostname, int port);
+
+void
+rpc_transport_cleanup(rpc_transport_t *);
#endif /* __RPC_TRANSPORT_H__ */
diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c
index 8d0c409..695e9fb 100644
--- a/rpc/rpc-lib/src/rpcsvc.c
+++ b/rpc/rpc-lib/src/rpcsvc.c
@@ -36,6 +36,7 @@
#include <fnmatch.h>
#include <stdarg.h>
#include <stdio.h>
+#include <dlfcn.h>
#include "xdr-rpcclnt.h"
#include "glusterfs-acl.h"
@@ -1677,6 +1678,7 @@ rpcsvc_create_listener (rpcsvc_t *svc, dict_t *options, char *name)
listener = rpcsvc_listener_alloc (svc, trans);
if (listener == NULL) {
+ ret = -1;
goto out;
}
@@ -1684,6 +1686,7 @@ rpcsvc_create_listener (rpcsvc_t *svc, dict_t *options, char *name)
out:
if (!listener && trans) {
rpc_transport_disconnect (trans, _gf_true);
+ rpc_transport_cleanup(trans);
}
return ret;
@@ -2285,6 +2288,43 @@ rpcsvc_get_throttle (rpcsvc_t *svc)
return svc->throttle;
}
+/* Function call to cleanup resources for svc
+ */
+int
+rpcsvc_destroy(rpcsvc_t *svc)
+{
+ struct rpcsvc_auth_list *auth = NULL;
+ struct rpcsvc_auth_list *tmp = NULL;
+ rpcsvc_listener_t *listener = NULL;
+ rpcsvc_listener_t *next = NULL;
+ int ret = 0;
+
+ if (!svc)
+ return ret;
+
+ list_for_each_entry_safe(listener, next, &svc->listeners, list)
+ {
+ rpcsvc_listener_destroy(listener);
+ }
+
+ list_for_each_entry_safe(auth, tmp, &svc->authschemes, authlist)
+ {
+ list_del_init(&auth->authlist);
+ GF_FREE(auth);
+ }
+
+ rpcsvc_program_unregister(svc, &gluster_dump_prog);
+ if (svc->rxpool) {
+ mem_pool_destroy(svc->rxpool);
+ svc->rxpool = NULL;
+ }
+
+ pthread_mutex_destroy(&svc->rpclock);
+ GF_FREE(svc);
+
+ return ret;
+}
+
/* The global RPC service initializer.
*/
rpcsvc_t *
diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h
index 34429b4..d3aafac 100644
--- a/rpc/rpc-lib/src/rpcsvc.h
+++ b/rpc/rpc-lib/src/rpcsvc.h
@@ -610,4 +610,7 @@ rpcsvc_auth_array (rpcsvc_t *svc, char *volname, int *autharr, int arrlen);
rpcsvc_vector_sizer
rpcsvc_get_program_vector_sizer (rpcsvc_t *svc, uint32_t prognum,
uint32_t progver, int procnum);
+
+extern int
+rpcsvc_destroy(rpcsvc_t *svc);
#endif
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index ec7e27a..b63c95a 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -9384,35 +9384,6 @@ glusterd_defrag_volume_status_update (glusterd_volinfo_t *volinfo,
return ret;
}
-/*
- The function is required to take dict ref for every xlator at graph.
- At the time of compare graph topology create a graph and populate
- key values in the dictionary, after finished graph comparison we do destroy
- the new graph.At the time of construct graph we don't take any reference
- so to avoid leak due to ref counter underflow we need to call dict_ref here.
-
-*/
-
-void
-glusterd_graph_take_reference (xlator_t *tree)
-{ xlator_t *trav = tree;
- xlator_t *prev = tree;
-
- if (!tree) {
- gf_msg ("parser", GF_LOG_ERROR, 0, LG_MSG_TREE_NOT_FOUND,
- "Translator tree not found");
- return;
- }
-
- while (prev) {
- trav = prev->next;
- if (prev->options)
- dict_ref (prev->options);
- prev = trav;
- }
- return;
-}
-
int
@@ -9461,14 +9432,14 @@ glusterd_check_topology_identical (const char *filename1,
if (grph1 == NULL)
goto out;
- glusterd_graph_take_reference (grph1->first);
+ gluster_graph_take_reference (grph1->first);
/* create the graph for filename2 */
grph2 = glusterfs_graph_construct(fp2);
if (grph2 == NULL)
goto out;
- glusterd_graph_take_reference (grph2->first);
+ gluster_graph_take_reference (grph2->first);
/* compare the graph topology */
*identical = is_graph_topology_equal(grph1, grph2);
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index 65d712f..6f510ea 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -1144,12 +1144,54 @@ client_destroy_cbk (xlator_t *this, client_t *client)
return 0;
}
+void
+server_cleanup(xlator_t *this, server_conf_t *conf)
+{
+ if (!this || !conf)
+ return;
+
+ LOCK_DESTROY(&conf->itable_lock);
+ pthread_mutex_destroy(&conf->mutex);
+
+ if (this->ctx->event_pool) {
+ /* Free the event pool */
+ (void)event_pool_destroy(this->ctx->event_pool);
+ }
+
+ if (dict_get(this->options, "config-directory")) {
+ GF_FREE(conf->conf_dir);
+ conf->conf_dir = NULL;
+ }
+
+ if (conf->child_status) {
+ GF_FREE(conf->child_status);
+ conf->child_status = NULL;
+ }
+
+ if (this->ctx->statedump_path) {
+ GF_FREE(this->ctx->statedump_path);
+ this->ctx->statedump_path = NULL;
+ }
+
+ if (conf->auth_modules) {
+ gf_auth_fini(conf->auth_modules);
+ dict_unref(conf->auth_modules);
+ }
+
+ if (conf->rpc) {
+ (void)rpcsvc_destroy(conf->rpc);
+ conf->rpc = NULL;
+ }
+
+ GF_FREE(conf);
+ this->private = NULL;
+}
+
int
init (xlator_t *this)
{
int32_t ret = -1;
server_conf_t *conf = NULL;
- rpcsvc_listener_t *listener = NULL;
char *transport_type = NULL;
char *statedump_path = NULL;
int total_transport = 0;
@@ -1226,6 +1268,7 @@ init (xlator_t *this)
ret = gf_auth_init (this, conf->auth_modules);
if (ret) {
dict_unref (conf->auth_modules);
+ conf->auth_modules = NULL;
goto out;
}
@@ -1378,10 +1421,7 @@ out:
if (this != NULL) {
this->fini (this);
}
-
- if (listener != NULL) {
- rpcsvc_listener_destroy (listener);
- }
+ server_cleanup(this, conf);
}
return ret;
--
1.8.3.1