URL: https://github.com/SSSD/sssd/pull/517
Author: sumit-bose
 Title: #517: Fix two memory leaks in the AD provider
Action: opened

PR body:
"""
I found two memory leaks in the AD provider, one is triggered by every user
lookup the other during an initgroups request with tokenGroups.

To verify this just lookup a larger number of AD users, I took 500, or call
'id' for each of the users. When checking the memory consumption before and
after e.g. with 'ps' the increased memory usage should become obvious.

To analyse where the memory is used 'talloc_report_full' help. You can call it
directly inside of gdb or just use Pavel's 'sss-talloc-report' from
https://github.com/pbrezina/sssd-dev-utils.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/517/head:pr517
git checkout pr517
From 7462b7b4f3b20e46f7e58e472fbb2997b93cf46c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 16 Feb 2018 12:07:28 +0100
Subject: [PATCH 1/2] AD: sdap_get_ad_tokengroups_done() allocate temporary
 data on state

Related to https://pagure.io/SSSD/sssd/issue/3639
---
 src/providers/ldap/sdap_async_initgroups_ad.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 9da671a99..30f1d3db2 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -372,7 +372,6 @@ sdap_get_ad_tokengroups_send(TALLOC_CTX *mem_ctx,
 
 static void sdap_get_ad_tokengroups_done(struct tevent_req *subreq)
 {
-    TALLOC_CTX *tmp_ctx = NULL;
     struct sdap_get_ad_tokengroups_state *state = NULL;
     struct tevent_req *req = NULL;
     struct sysdb_attrs **users = NULL;
@@ -386,7 +385,7 @@ static void sdap_get_ad_tokengroups_done(struct tevent_req *subreq)
     req = tevent_req_callback_data(subreq, struct tevent_req);
     state = tevent_req_data(req, struct sdap_get_ad_tokengroups_state);
 
-    ret = sdap_get_generic_recv(subreq, tmp_ctx, &num_users, &users);
+    ret = sdap_get_generic_recv(subreq, state, &num_users, &users);
     talloc_zfree(subreq);
     if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE,
@@ -449,8 +448,6 @@ static void sdap_get_ad_tokengroups_done(struct tevent_req *subreq)
     ret = EOK;
 
 done:
-    talloc_free(tmp_ctx);
-
     if (ret != EOK) {
         tevent_req_error(req, ret);
         return;

From d205dd448348082721ac834ba9b0edccda5a56a6 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 16 Feb 2018 12:09:01 +0100
Subject: [PATCH 2/2] AD: do not allocate temporary data on long living context

Related to https://pagure.io/SSSD/sssd/issue/3639
---
 src/providers/ad/ad_common.c | 5 +++--
 src/providers/ad/ad_common.h | 3 ++-
 src/providers/ad/ad_id.c     | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 84845e285..2a1647173 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -1402,13 +1402,14 @@ ad_ldap_conn_list(TALLOC_CTX *mem_ctx,
 }
 
 struct sdap_id_conn_ctx **
-ad_user_conn_list(struct ad_id_ctx *ad_ctx,
+ad_user_conn_list(TALLOC_CTX *mem_ctx,
+                  struct ad_id_ctx *ad_ctx,
                   struct sss_domain_info *dom)
 {
     struct sdap_id_conn_ctx **clist;
     int cindex = 0;
 
-    clist = talloc_zero_array(ad_ctx, struct sdap_id_conn_ctx *, 3);
+    clist = talloc_zero_array(mem_ctx, struct sdap_id_conn_ctx *, 3);
     if (clist == NULL) {
         return NULL;
     }
diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index ce33b37c7..931aafc6c 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -175,7 +175,8 @@ ad_ldap_conn_list(TALLOC_CTX *mem_ctx,
                   struct sss_domain_info *dom);
 
 struct sdap_id_conn_ctx **
-ad_user_conn_list(struct ad_id_ctx *ad_ctx,
+ad_user_conn_list(TALLOC_CTX *mem_ctx,
+                  struct ad_id_ctx *ad_ctx,
                   struct sss_domain_info *dom);
 
 struct sdap_id_conn_ctx *
diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c
index 0b8f49819..782d9bc40 100644
--- a/src/providers/ad/ad_id.c
+++ b/src/providers/ad/ad_id.c
@@ -367,7 +367,7 @@ get_conn_list(TALLOC_CTX *mem_ctx, struct ad_id_ctx *ad_ctx,
 
     switch (ar->entry_type & BE_REQ_TYPE_MASK) {
     case BE_REQ_USER: /* user */
-        clist = ad_user_conn_list(ad_ctx, dom);
+        clist = ad_user_conn_list(mem_ctx, ad_ctx, dom);
         break;
     case BE_REQ_BY_SECID:   /* by SID */
     case BE_REQ_USER_AND_GROUP: /* get SID */
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to