sankarNarayanan / modwsgi

Automatically exported from code.google.com/p/modwsgi
0 stars 0 forks source link

Daemon process dies by SIGSEGV while initializing with ErrorLog syslog #178

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
I'm running Apache 2.2.12 and mod-wsgi 3.1 and have encountered a bug
introduced by a new mod-wsgi 3.0 feature (*12*) as described here:

http://code.google.com/p/modwsgi/wiki/ChangesInVersion0300

***
12. If daemon process defined in a virtual host, close all error logs for
other virtual hosts which don't reference the same error log. This ensures
that code can't write messages to error logs for another host, or reopen
the log and read data from the logs.
***

This won't work if the VirtualHost is using Apache's built-in ErrorLog
syslog[:facility] feature. The cause for the segfault is most probably a
invalid null pointer dereference, looking into Apache's server/log.c:

  ... if syslog ...
  s->error_log = NULL;

while in mod_wsgi.c wsgi_start_process() you assume it's of type apr_file_t.

How to reproduce?

<VirtualHost *:80>
  ErrorLog syslog
  WSGIDaemonProcess foo ...
  WSGIProcessGroup foo
  ...
</VirtualHost>

$ apache2ctl -k start

Results in:

[Thu Jan 28 20:00:18 2010] [notice] child pid 22766 exit signal
Segmentation fault (11)
[Thu Jan 28 20:00:18 2010] [info] mod_wsgi (pid=22766): Process 'foo' has
died, restarting.
... endless loop ...

How to fix?

- The short fix is to place the WSGIDaemonProcess definition outside of the
VirtualHost block, which prevents the attempt to secure the error logs.

- The better fix would probably be something like this:

In wsgi_start_process() replace the condition triggering the error log magic:

  if (daemon->group->server->is_virtual)

by:

  if (daemon->group->server->is_virtual &&
      daemon->group->server->error_log != NULL)

which would skip the whole attempt if there is no dedicated VirtualHost
error log file.

Thanks :-)

Best regards,
John Feuerstein

Original issue reported on code.google.com by jf.feuerstein@googlemail.com on 28 Jan 2010 at 7:48

GoogleCodeExporter commented 8 years ago
Believe a more complete patch is as follows. One can't just skip the whole 
process as technically not all virtual 
hosts might be being redirected to syslog for logging.

Note that patch here is against trunk, but should still apply to mod_wsgi 3.1 
cleanly.

Index: mod_wsgi.c
===================================================================
--- mod_wsgi.c  (revision 1529)
+++ mod_wsgi.c  (working copy)
@@ -11117,30 +11117,42 @@

             /*
              * Iterate over all servers and close any error
-             * logs different to that for virtual host.
+             * logs different to that for virtual host. Note that
+             * if errors are being redirected to syslog, then
+             * the server error log reference will actually be
+             * a null pointer, so need to ensure that check for
+             * that and don't attempt to close it in that case.
              */

             server = wsgi_server;

             while (server != NULL) {
-                if (server->error_log != daemon->group->server->error_log)
+                if (server->error_log &&
+                    server->error_log != daemon->group->server->error_log) {
                     apr_file_close(server->error_log);
+                }

                 server = server->next;
             }

             /*
-             * Reassociate stderr output with error log from the
-             * virtual host the daemon is associated with. Close
-             * the virtual host error log and point it at stderr
-             * log instead. Do the latter so don't get two
-             * references to same open file. Just in case
-             * anything still accesses error log of main server,
-             * map main server error log to that of the virtual
-             * host.
+        * Reassociate stderr output with error log from the
+        * virtual host the daemon is associated with. Close
+        * the virtual host error log and point it at stderr
+        * log instead. Do the latter so don't get two
+        * references to same open file. Just in case
+        * anything still accesses error log of main server,
+        * map main server error log to that of the virtual
+        * host. Note that cant do this if errors are being
+        * redirected to syslog, as indicated by virtual
+        * host error log being a null pointer. In that case
+        * just leave everything as it was. Also can't remap
+        * the error log for main server if it was being
+        * redirected to syslog but virtual host wasn't.
              */

-            if (daemon->group->server->error_log != wsgi_server->error_log) {
+            if (daemon->group->server->error_log  &&
+                daemon->group->server->error_log != wsgi_server->error_log) {
                 apr_file_open_stderr(&errfile, wsgi_server->process->pool);
                 apr_file_dup2(errfile, daemon->group->server->error_log,
                               wsgi_server->process->pool);
@@ -11148,7 +11160,8 @@
                 apr_file_close(daemon->group->server->error_log);
                 daemon->group->server->error_log = errfile;

-                wsgi_server->error_log = daemon->group->server->error_log;
+                if (wsgi_server->error_log)
+                    wsgi_server->error_log = daemon->group->server->error_log;
             }
         }

Original comment by Graham.Dumpleton@gmail.com on 31 Jan 2010 at 2:30

GoogleCodeExporter commented 8 years ago
Version 3.2 of mod_wsgi released with this fix.

Original comment by Graham.Dumpleton@gmail.com on 9 Mar 2010 at 10:11

GoogleCodeExporter commented 8 years ago

Original comment by Graham.Dumpleton@gmail.com on 9 Mar 2010 at 10:13