Re: Memory leak in mod_ssl ssl_callback_TmpDH

[Yann Ylavic]

On Tue, May 27, 2014 at 10:33 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>
>  #define make_get_dh(rfc,size,gen) \
>  static DH *get_dh##size(void) \
> @@ -1339,7 +1344,7 @@
>          DH_free(dh_tmp); \
>          return NULL; \
>      } \
> -    dh = dh_tmp; \
> +    apr_atomic_xchgptr((volatile void **)&dh, dh_tmp); \
>      return dh; \
>  }
>

I think you still need to handle the race (leak) with something like :

Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c    (revision 1597892)
+++ modules/ssl/ssl_engine_kernel.c    (working copy)
@@ -1324,7 +1325,7 @@ const authz_provider ssl_authz_provider_verify_cli
 #define make_get_dh(rfc,size,gen) \
 static DH *get_dh##size(void) \
 { \
-    static DH *dh = NULL; \
+    static void *volatile dh = NULL; \
     DH *dh_tmp; \
 \
     if (dh) { \
@@ -1339,7 +1340,9 @@ static DH *get_dh##size(void) \
         DH_free(dh_tmp); \
         return NULL; \
     } \
-    dh = dh_tmp; \
+    if (apr_atomic_casptr((volatile void **)&dh, dh_tmp, NULL)) { \
+        DH_free(dh_tmp); \
+    } \
     return dh; \
 }

Regards,
Yann.

PS: there is a problem with the declaration of apr_atomic_casptr() and
apr_atomic_xchgptr() in APR, "volatile void **mem" instead of "void
*volatile *mem". Hence the cast is mandatory here.