Fix bug 5419: memory leak in ads_do_search_all_args() when enumerating 1000s of entries
authorSteven Danneman <sdanneman@sd-ubuntu.(none)>
Sat, 26 Apr 2008 01:34:46 +0000 (18:34 -0700)
committerVolker Lendecke <vl@samba.org>
Sat, 26 Apr 2008 07:08:18 +0000 (09:08 +0200)
The ads_do_search_all_args() function attempts to string together several
LDAPMessage structures, returned across several paged ldap requests, into a
single LDAPMessage structure.  It does this by pulling entries off the second
LDAPMessage structure and appending them to the first via the OpenLDAP specific
ldap_add_result_entry() call.

The problem with this approach is it skips non-entry messages such as the
result, and controls.  These messages are leaked.

The short term solution as suggested by Volker is to replace the ads_*_entry()
calls with ads_*_message() calls so we don't leak any messages.

This fixes the leak but doesn't remove the dependence on the OpenLDAP specific
implementation of ldap_add_result_entry().

source/include/ads_protos.h
source/libads/ldap.c

index 738df3ed40cabbf99688bc8327927ed3cfab8489..a372010b796e91c0231381975db1105c0b9ebabc 100644 (file)
@@ -89,6 +89,8 @@ ADS_STATUS ads_search_retry_sid(ADS_STRUCT *ads, LDAPMessage **res,
 
 LDAPMessage *ads_first_entry(ADS_STRUCT *ads, LDAPMessage *res);
 LDAPMessage *ads_next_entry(ADS_STRUCT *ads, LDAPMessage *res);
+LDAPMessage *ads_first_message(ADS_STRUCT *ads, LDAPMessage *res);
+LDAPMessage *ads_next_message(ADS_STRUCT *ads, LDAPMessage *res);
 void ads_process_results(ADS_STRUCT *ads, LDAPMessage *res,
                         bool (*fn)(ADS_STRUCT *,char *, void **, void *),
                         void *data_area);
index b0f27b598bbdf50d6aa2b1a8b8155ca48cfd038b..93213021516d13ff75e612f16b9bea642d55cf20 100644 (file)
@@ -870,8 +870,8 @@ static ADS_STATUS ads_do_paged_search(ADS_STRUCT *ads, const char *bind_path,
 
                /* this relies on the way that ldap_add_result_entry() works internally. I hope
                   that this works on all ldap libs, but I have only tested with openldap */
-               for (msg = ads_first_entry(ads, res2); msg; msg = next) {
-                       next = ads_next_entry(ads, msg);
+               for (msg = ads_first_message(ads, res2); msg; msg = next) {
+                       next = ads_next_message(ads, msg);
                        ldap_add_result_entry((LDAPMessage **)res, msg);
                }
                /* note that we do not free res2, as the memory is now
@@ -2090,6 +2090,28 @@ int ads_count_replies(ADS_STRUCT *ads, void *res)
        return ldap_next_entry(ads->ldap.ld, res);
 }
 
+/**
+ * pull the first message from a ADS result
+ * @param ads connection to ads server
+ * @param res Results of search
+ * @return first message from result
+ **/
+ LDAPMessage *ads_first_message(ADS_STRUCT *ads, LDAPMessage *res)
+{
+       return ldap_first_message(ads->ldap.ld, res);
+}
+
+/**
+ * pull the next message from a ADS result
+ * @param ads connection to ads server
+ * @param res Results of search
+ * @return next message from result
+ **/
+ LDAPMessage *ads_next_message(ADS_STRUCT *ads, LDAPMessage *res)
+{
+       return ldap_next_message(ads->ldap.ld, res);
+}
+
 /**
  * pull a single string from a ADS result
  * @param ads connection to ads server