Bug 43873 - mod_jk memory leak when apache graceful restart
mod_jk memory leak when apache graceful restart
Product: Tomcat Connectors
Classification: Unclassified
Component: Common
Other Linux
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
Depends on:
  Show dependency tree
Reported: 2007-11-15 07:18 UTC by Eiji Takahashi
Modified: 2008-10-05 03:10 UTC (History)
0 users


Note You need to log in before you can comment on or make changes to this bug.
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.