Bug 43873

Summary: mod_jk memory leak when apache graceful restart
Product: Tomcat Connectors Reporter: Eiji Takahashi <mashmk02>
Component: CommonAssignee: Tomcat Developers Mailing List <dev>
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Hardware: Other   
OS: Linux   

Description Eiji Takahashi 2007-11-15 07:18:33 UTC
In jk_set_time_fmt(),  memory area is allocated by the following codes. 
 fmt = (char *)malloc(JK_TIME_MAX_SIZE + strlen(JK_TIME_PATTERN_MICRO));
But this area is not released. 

When i restart apache with graceful mode, apache master process causes the
memory leak a little.
I tested in the following environments.
 RHEL4.5(Apache2.0.52) + mod_jk 1.2.25/1.2.26-dev

I made following patch for apache-2.0/mod_jk.c.
--- mod_jk/native/apache-2.0/mod_jk.c   (revision 594009)
+++ mod_jk/native/apache-2.0/mod_jk.c   (working copy)
@@ -2605,6 +2605,13 @@
         jkl->log = jk_log_to_file;
         jkl->level = conf->log_level;
         jk_set_time_fmt(jkl, conf->stamp_format_string);
+        char *log_fmt_subsec = (char *)jkl->log_fmt_subsec;
+        if ( log_fmt_subsec != NULL ) {
+            if (jkl->log_fmt_size > 0)
+                jkl->log_fmt_subsec = apr_pstrdup(p, jkl->log_fmt_subsec);
+            free((void*)log_fmt_subsec);
+        }
         jkl->logger_private = flp;
         flp->jklogfp = conf->jklogfp;
         conf->log = jkl;
Comment 1 Mladen Turk 2007-11-15 08:15:41 UTC

You are correct.
The complete log_fmt_subsec is a complete mess.
It's either assigned as const char* or allocated.
Even more it's wrongly used later, presumed as JK_TIME_MAX_SIZE long,
Comment 2 Rainer Jung 2007-11-20 09:34:15 UTC
Thanks for reporting. The leak has been fixed in r596746 (in a slightly
different way, because I don't want to expose the internals of timestamp
formatting to the web server).

Mladens comments about the complicated size handling led to another revision

Even without r596747 there was no buffer overflow issue, because the longer
string was truncated with '\0' before it was used in the shorter context.

The changes will be part of version 1.2.26, which we expect to release soon.
There will be a testing tarball before, with an announcement via the dev list.