httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject [patch] resolving segfaults when ap_run_pre_connection() fails.
Date Tue, 18 Feb 2003 01:15:47 GMT
why ap_process_connection() doesn't check the return value of
ap_run_pre_connection()? Currently if the pre_connection phase fails (without
setting c->aborted) ap_run_process_connection is executed
nevertheless.

if a custom pre_connection handler fails, before giving a chance for
core_pre_connection handler to run, it will cause a segfault later in
the net_time_filter filter (during the process_connection phase):

     if (!ctx) {
         f->ctx = ctx = apr_palloc(f->r->pool, sizeof(*ctx));
         ctx->first_line = 1;
         ctx->csd = ap_get_module_config(f->c->conn_config, &core_module);
     }

ctx->csd is now NULL and it segfaults next in apr_socket_timeout_set():

     if (mode != AP_MODE_INIT && mode != AP_MODE_EATCRLF) {
         if (ctx->first_line) {
             apr_socket_timeout_set(ctx->csd,
                                    keptalive
                                       ? f->c->base_server->keep_alive_timeout
                                       : f->c->base_server->timeout);

Now, since core_pre_connection returns DONE, it's impossible to insert
anything after it. So telling developers to insert their
pre_connection hooks after core_pre_connection is not an option.

Here is a proposed patch, may be logging an error message is a good idea.

Index: server/connection.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/connection.c,v
retrieving revision 1.106
diff -u -r1.106 connection.c
--- server/connection.c 15 Jul 2002 08:05:10 -0000      1.106
+++ server/connection.c 18 Feb 2003 01:11:38 -0000
@@ -199,11 +199,12 @@

  AP_CORE_DECLARE(void) ap_process_connection(conn_rec *c, void *csd)
  {
+    apr_status_t rc;
      ap_update_vhost_given_ip(c);

-    ap_run_pre_connection(c, csd);
-
-    if (!c->aborted) {
+    rc = ap_run_pre_connection(c, csd);
+
+    if ((rc == OK || rc == DONE) && !c->aborted) {
          ap_run_process_connection(c);
      }
  }

Finally, I see that there are other hooks in the public API whose return value 
is not checked (e.g. ap_run_child_init). I thought that developers are 
supposed to return a failure status if their callbacks fail and expect Apache 
to abort the processing.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


Mime
View raw message