Hello,

attached is backported patch from 2.1-BETA7 which fixes invalid handling
of NULL values in mod_ldap caching code.

There is an open bug in bugzilla:
http://issues.apache.org/bugzilla/show_bug.cgi?id=36563

And this is just updated patch against today SVN checkouted 2.0.x (patch
from bugzilla applies ok with offset of 20 lines).

Hopefully this gets accepted before 2.0.55 is out, since this bug can
cause memory leaks in util_ldap_search_node_free (not all items are
freed only those before first item with NULL value).

Ondrej.
-- 
Ondrej Sury <[EMAIL PROTECTED]>
Index: modules/experimental/util_ldap_cache.c
===================================================================
--- modules/experimental/util_ldap_cache.c	(revision 290104)
+++ modules/experimental/util_ldap_cache.c	(working copy)
@@ -158,18 +158,22 @@
 
         /* copy vals */
         if (node->vals) {
-            int k = 0;
+            int k = node->numvals;
             int i = 0;
-            while (node->vals[k++]);
             if (!(newnode->vals = util_ald_alloc(cache, sizeof(char *) * (k+1)))) {
                 util_ldap_search_node_free(cache, newnode);
                 return NULL;
             }
-            while (node->vals[i]) {
-                if (!(newnode->vals[i] = util_ald_strdup(cache, node->vals[i]))) {
-                    util_ldap_search_node_free(cache, newnode);
-                    return NULL;
+            newnode->numvals = node->numvals;
+            for (;k;k--) {
+                if (node->vals[i]) {
+                    if (!(newnode->vals[i] = util_ald_strdup(cache, node->vals[i]))) {
+                        util_ldap_search_node_free(cache, newnode);
+                        return NULL;
+                    }
                 }
+                else
+                    newnode->vals[i] = NULL;
                 i++;
             }
         }
@@ -199,9 +203,13 @@
 {
     int i = 0;
     util_search_node_t *node = (util_search_node_t *)n;
+    int k = node->numvals;
+
     if (node->vals) {
-        while (node->vals[i]) {
-            util_ald_free(cache, node->vals[i++]);
+        for (;k;k--,i++) {
+            if (node->vals[i]) {
+                util_ald_free(cache, node->vals[i]);
+            }
         }
         util_ald_free(cache, node->vals);
     }
Index: modules/experimental/util_ldap_cache.h
===================================================================
--- modules/experimental/util_ldap_cache.h	(revision 290104)
+++ modules/experimental/util_ldap_cache.h	(working copy)
@@ -110,6 +110,7 @@
 					   NULL if the bind failed */
     apr_time_t lastbind;		/* Time of last successful bind */
     const char **vals;			/* Values of queried attributes */
+    int        numvals;			/* Number of queried attributes */
 } util_search_node_t;
 
 /*
Index: modules/experimental/util_ldap.c
===================================================================
--- modules/experimental/util_ldap.c	(revision 290104)
+++ modules/experimental/util_ldap.c	(working copy)
@@ -769,6 +769,7 @@
                               const char ***retvals)
 {
     const char **vals = NULL;
+    int numvals = 0;
     int result = 0;
     LDAPMessage *res, *entry;
     char *dn;
@@ -932,6 +933,7 @@
         int i = 0;
         while (attrs[k++]);
         vals = apr_pcalloc(r->pool, sizeof(char *) * (k+1));
+        numvals = k;
         while (attrs[i]) {
             char **values;
             int j = 0;
@@ -959,6 +961,7 @@
         the_search_node.bindpw = bindpw;
         the_search_node.lastbind = apr_time_now();
         the_search_node.vals = vals;
+        the_search_node.numvals = numvals;
 
         /* Search again to make sure that another thread didn't ready insert this node
            into the cache before we got here. If it does exist then update the lastbind */
@@ -1001,6 +1004,7 @@
                               const char ***retvals)
 {
     const char **vals = NULL;
+    int numvals = 0;
     int result = 0;
     LDAPMessage *res, *entry;
     char *dn;
@@ -1115,6 +1119,7 @@
         int i = 0;
         while (attrs[k++]);
         vals = apr_pcalloc(r->pool, sizeof(char *) * (k+1));
+        numvals = k;
         while (attrs[i]) {
             char **values;
             int j = 0;
@@ -1142,6 +1147,7 @@
         the_search_node.bindpw = NULL;
         the_search_node.lastbind = apr_time_now();
         the_search_node.vals = vals;
+        the_search_node.numvals = numvals;
 
         /* Search again to make sure that another thread didn't ready insert this node
            into the cache before we got here. If it does exist then update the lastbind */

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to