PR63305: fix graceful restart crashes in LDAP

The cache destruction was not protected by the lock used by other
cache callers.

Pull the static cleanup function into util_ldap.c so it's convenient to
use the existing locking.

Submitted By: Martin Fúsek <mfusek newps.cz>
Commited By: covener

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1856735 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 7816a26051f1851bae24248d95fd6b8b1b571ca7..e4f6be2d13b599bc446cec0bfb17359ac27e5f5c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.1
 
+  *) mod_ldap: Avoid potential crashes in util_ldap_cache_module_kill() or other
+     LDAP related functions during graceful restart of a busy server. PR63305.
+     [Martin Fúsek <mfusek newps.cz>]
+
   *) mod_cache: Fix parsing of quoted Cache-Control token arguments.
      PR 63288. [Yann Ylavic]
 

diff --git a/modules/ldap/util_ldap.c b/modules/ldap/util_ldap.c
index 7b3801b16fd161e9fc9bd84c2bf45a16a2125519..98b6befc86f75c25c91d204a1881806ebbbcc03e 100644 (file)
--- a/modules/ldap/util_ldap.c
+++ b/modules/ldap/util_ldap.c
@@ -81,7 +81,12 @@ static APR_INLINE apr_status_t ldap_cache_lock(util_ldap_state_t *st, request_re
     if (st->util_ldap_cache_lock) { 
         apr_status_t rv = apr_global_mutex_lock(st->util_ldap_cache_lock);
         if (rv != APR_SUCCESS) { 
-            ap_log_rerror(APLOG_MARK, APLOG_CRIT, rv, r, APLOGNO(10134) "LDAP cache lock failed");
+            if (r) {
+                ap_log_rerror(APLOG_MARK, APLOG_CRIT, rv, r, APLOGNO(10134) "LDAP cache lock failed");
+            }
+            else { 
+                ap_log_error(APLOG_MARK, APLOG_CRIT, rv, NULL, APLOGNO(10134) "LDAP cache lock failed");
+            }
             ap_assert(0);
         }
     }
@@ -92,7 +97,12 @@ static APR_INLINE apr_status_t ldap_cache_unlock(util_ldap_state_t *st, request_
     if (st->util_ldap_cache_lock) { 
         apr_status_t rv = apr_global_mutex_unlock(st->util_ldap_cache_lock);
         if (rv != APR_SUCCESS) { 
-            ap_log_rerror(APLOG_MARK, APLOG_CRIT, rv, r, APLOGNO(10135) "LDAP cache lock failed");
+            if (r != NULL) {
+                ap_log_rerror(APLOG_MARK, APLOG_CRIT, rv, r, APLOGNO(10135) "LDAP cache unlock failed");
+            }
+            else { 
+                ap_log_error(APLOG_MARK, APLOG_CRIT, rv, NULL, APLOGNO(10135) "LDAP cache unlock failed");
+            }
             ap_assert(0);
         }
     }
@@ -111,6 +121,25 @@ static void util_ldap_strdup (char **str, const char *newstr)
     }
 }
 
+static apr_status_t util_ldap_cache_module_kill(void *data)
+{
+    util_ldap_state_t *st = data;
+
+    util_ald_destroy_cache(st->util_ldap_cache);
+#if APR_HAS_SHARED_MEMORY
+    if (st->cache_rmm != NULL) {
+        apr_rmm_destroy (st->cache_rmm);
+        st->cache_rmm = NULL;
+    }
+    if (st->cache_shm != NULL) {
+        apr_status_t result = apr_shm_destroy(st->cache_shm);
+        st->cache_shm = NULL;
+        return result;
+    }
+#endif
+    return APR_SUCCESS;
+}
+
 /*
  * Status Handler
  * --------------
@@ -2932,6 +2961,18 @@ static int util_ldap_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
     return OK;
 }
 
+static apr_status_t util_ldap_cache_module_kill_locked(void *data)
+{
+    apr_status_t result;
+    util_ldap_state_t *st = data;
+
+    ldap_cache_lock(st, NULL);
+    result = util_ldap_cache_module_kill(data);
+    ldap_cache_unlock(st, NULL);
+
+    return result;
+}
+
 static int util_ldap_post_config(apr_pool_t *p, apr_pool_t *plog,
                                  apr_pool_t *ptemp, server_rec *s)
 {
@@ -2979,6 +3020,8 @@ static int util_ldap_post_config(apr_pool_t *p, apr_pool_t *plog,
             return DONE;
         }
 
+        apr_pool_cleanup_register(st->pool, st , util_ldap_cache_module_kill_locked, apr_pool_cleanup_null);
+
         result = ap_global_mutex_create(&st->util_ldap_cache_lock, NULL,
                                         ldap_cache_mutex_type, NULL, s, p, 0);
         if (result != APR_SUCCESS) {

diff --git a/modules/ldap/util_ldap_cache.c b/modules/ldap/util_ldap_cache.c
index 774a76e1acfd05acf990e9e1b552c63272c96796..6a944daa843ad68b41bf90140956b0258fe51bf3 100644 (file)
--- a/modules/ldap/util_ldap_cache.c
+++ b/modules/ldap/util_ldap_cache.c
@@ -396,26 +396,6 @@ void util_ldap_dn_compare_node_display(request_rec *r, util_ald_cache_t *cache,
 }
 
 
-/* ------------------------------------------------------------------ */
-static apr_status_t util_ldap_cache_module_kill(void *data)
-{
-    util_ldap_state_t *st = data;
-
-    util_ald_destroy_cache(st->util_ldap_cache);
-#if APR_HAS_SHARED_MEMORY
-    if (st->cache_rmm != NULL) {
-        apr_rmm_destroy (st->cache_rmm);
-        st->cache_rmm = NULL;
-    }
-    if (st->cache_shm != NULL) {
-        apr_status_t result = apr_shm_destroy(st->cache_shm);
-        st->cache_shm = NULL;
-        return result;
-    }
-#endif
-    return APR_SUCCESS;
-}
-
 apr_status_t util_ldap_cache_init(apr_pool_t *pool, util_ldap_state_t *st)
 {
 #if APR_HAS_SHARED_MEMORY
@@ -449,8 +429,6 @@ apr_status_t util_ldap_cache_init(apr_pool_t *pool, util_ldap_state_t *st)
 
 #endif
 
-    apr_pool_cleanup_register(st->pool, st , util_ldap_cache_module_kill, apr_pool_cleanup_null);
-
     st->util_ldap_cache =
         util_ald_create_cache(st,
                               st->search_cache_size,