From: Jim Jagielski Date: Tue, 11 Nov 2008 20:04:34 +0000 (+0000) Subject: *) mod_proxy: Prevent segmentation faults by correctly adjusting the lifetime X-Git-Tag: 2.2.11~77 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a10438c040fe5098e6e732e72f334be33ed64a0c;p=thirdparty%2Fapache%2Fhttpd.git *) mod_proxy: Prevent segmentation faults by correctly adjusting the lifetime of the buckets read from the proxy backend. PR 45792 [Ruediger Pluem] git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@713146 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 2dd6a9f9910..ef850e5dac3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ -*- coding: utf-8 -*- Changes with Apache 2.2.11 + *) mod_proxy: Prevent segmentation faults by correctly adjusting the lifetime + of the buckets read from the proxy backend. PR 45792 [Ruediger Pluem] + *) mod_proxy_ajp: Fix wrongly formatted requests where client sets Content-Length header, but doesn't provide a body. Servlet container always expects that next packet is diff --git a/include/ap_mmn.h b/include/ap_mmn.h index bbce0ed0ece..7eaff838248 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -133,6 +133,7 @@ * 20051115.17 (2.2.10) Add scolonsep to proxy_balancer * 20051115.18 (2.2.10) Add chroot support to unixd_config * 20051115.19 (2.2.11) Added ap_timeout_parameter_parse to util.c / httpd.h + * 20051115.20 (2.2.11) Add ap_proxy_buckets_lifetime_transform to mod_proxy.h */ #define MODULE_MAGIC_COOKIE 0x41503232UL /* "AP22" */ @@ -140,7 +141,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20051115 #endif -#define MODULE_MAGIC_NUMBER_MINOR 19 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 20 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 55f64598904..04b710660aa 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -750,6 +750,29 @@ PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r, #define PROXY_HAS_SCOREBOARD 0 #endif +/** + * Transform buckets from one bucket allocator to another one by creating a + * transient bucket for each data bucket and let it use the data read from + * the old bucket. Metabuckets are transformed by just recreating them. + * Attention: Currently only the following bucket types are handled: + * + * All data buckets + * FLUSH + * EOS + * + * If an other bucket type is found its type is logged as a debug message + * and APR_EGENERAL is returned. + * @param r current request record of client request. Only used for logging + * purposes + * @param from the brigade that contains the buckets to transform + * @param to the brigade that will receive the transformed buckets + * @return APR_SUCCESS if all buckets could be transformed APR_EGENERAL + * otherwise + */ +PROXY_DECLARE(apr_status_t) +ap_proxy_buckets_lifetime_transform(request_rec *r, apr_bucket_brigade *from, + apr_bucket_brigade *to); + #define PROXY_LBMETHOD "proxylbmethod" /* The number of dynamic workers that can be added when reconfiguring. diff --git a/modules/proxy/mod_proxy_ftp.c b/modules/proxy/mod_proxy_ftp.c index 45108361db5..639f9f8d5b4 100644 --- a/modules/proxy/mod_proxy_ftp.c +++ b/modules/proxy/mod_proxy_ftp.c @@ -969,7 +969,6 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, } /* TODO: see if ftp could use determine_connection */ backend->addr = connect_addr; - backend->r = r; ap_set_module_config(c->conn_config, &proxy_ftp_module, backend); } diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 3d002c9207b..b60db985547 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -1338,6 +1338,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, request_rec *rp; apr_bucket *e; apr_bucket_brigade *bb, *tmp_bb; + apr_bucket_brigade *pass_bb; int len, backasswards; int interim_response = 0; /* non-zero whilst interim 1xx responses * are being read. */ @@ -1350,6 +1351,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, const char *te = NULL; bb = apr_brigade_create(p, c->bucket_alloc); + pass_bb = apr_brigade_create(p, c->bucket_alloc); /* Get response from the remote server, and pass it up the * filter chain @@ -1768,6 +1770,9 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, break; } + /* Switch the allocator lifetime of the buckets */ + ap_proxy_buckets_lifetime_transform(r, bb, pass_bb); + /* found the last brigade? */ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { /* signal that we must leave */ @@ -1775,7 +1780,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, } /* try send what we read */ - if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS + if (ap_pass_brigade(r->output_filters, pass_bb) != APR_SUCCESS || c->aborted) { /* Ack! Phbtt! Die! User aborted! */ backend->close = 1; /* this causes socket close below */ @@ -1784,6 +1789,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, /* make sure we always clean up after ourselves */ apr_brigade_cleanup(bb); + apr_brigade_cleanup(pass_bb); } while (!finish); } diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 194dbb1d7ee..24f5aa2212d 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1624,9 +1624,6 @@ static apr_status_t connection_cleanup(void *theconn) { proxy_conn_rec *conn = (proxy_conn_rec *)theconn; proxy_worker *worker = conn->worker; - apr_bucket_brigade *bb; - conn_rec *c; - request_rec *r; /* * If the connection pool is NULL the worker @@ -1647,57 +1644,6 @@ static apr_status_t connection_cleanup(void *theconn) } #endif - r = conn->r; - if (conn->need_flush && r && (r->bytes_sent || r->eos_sent)) { - /* - * We need to ensure that buckets that may have been buffered in the - * network filters get flushed to the network. This is needed since - * these buckets have been created with the bucket allocator of the - * backend connection. This allocator either gets destroyed if - * conn->close is set or the worker address is not reusable which - * causes the connection to the backend to be closed or it will be used - * again by another frontend connection that wants to recycle the - * backend connection. - * In this case we could run into nasty race conditions (e.g. if the - * next user of the backend connection destroys the allocator before we - * sent the buckets to the network). - * - * Remark 1: Only do this if buckets where sent down the chain before - * that could still be buffered in the network filter. This is the case - * if we have sent an EOS bucket or if we actually sent buckets with - * data down the chain. In all other cases we either have not sent any - * buckets at all down the chain or we only sent meta buckets that are - * not EOS buckets down the chain. The only meta bucket that remains in - * this case is the flush bucket which would have removed all possibly - * buffered buckets in the network filter. - * If we sent a flush bucket in the case where not ANY buckets were - * sent down the chain, we break error handling which happens AFTER us. - * - * Remark 2: Doing a setaside does not help here as the buckets remain - * created by the wrong allocator in this case. - * - * Remark 3: Yes, this creates a possible performance penalty in the case - * of pipelined requests as we may send only a small amount of data over - * the wire. - */ - c = r->connection; - bb = apr_brigade_create(r->pool, c->bucket_alloc); - if (r->eos_sent) { - /* - * If we have already sent an EOS bucket send directly to the - * connection based filters. We just want to flush the buckets - * if something hasn't been sent to the network yet. - */ - ap_fflush(c->output_filters, bb); - } - else { - ap_fflush(r->output_filters, bb); - } - apr_brigade_destroy(bb); - conn->r = NULL; - conn->need_flush = 0; - } - /* determine if the connection need to be closed */ if (conn->close_on_recycle || conn->close || worker->disablereuse || !worker->is_address_reusable) { @@ -2084,8 +2030,6 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, apr_status_t err = APR_SUCCESS; apr_status_t uerr = APR_SUCCESS; - conn->r = r; - /* * Break up the URL to determine the host to connect to */ @@ -2422,12 +2366,6 @@ PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function, return OK; } - /* - * We need to flush the buckets before we return the connection to the - * connection pool. See comment in connection_cleanup for why this is - * needed. - */ - conn->need_flush = 1; bucket_alloc = apr_bucket_alloc_create(conn->scpool); /* * The socket is now open, create a new backend server connection @@ -2520,3 +2458,54 @@ PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r, e = apr_bucket_eos_create(c->bucket_alloc); APR_BRIGADE_INSERT_TAIL(brigade, e); } + +/* + * Transform buckets from one bucket allocator to another one by creating a + * transient bucket for each data bucket and let it use the data read from + * the old bucket. Metabuckets are transformed by just recreating them. + * Attention: Currently only the following bucket types are handled: + * + * All data buckets + * FLUSH + * EOS + * + * If an other bucket type is found its type is logged as a debug message + * and APR_EGENERAL is returned. + */ +PROXY_DECLARE(apr_status_t) +ap_proxy_buckets_lifetime_transform(request_rec *r, apr_bucket_brigade *from, + apr_bucket_brigade *to) +{ + apr_bucket *e; + apr_bucket *new; + const char *data; + apr_size_t bytes; + apr_status_t rv = APR_SUCCESS; + + apr_brigade_cleanup(to); + for (e = APR_BRIGADE_FIRST(from); + e != APR_BRIGADE_SENTINEL(from); + e = APR_BUCKET_NEXT(e)) { + if (!APR_BUCKET_IS_METADATA(e)) { + apr_bucket_read(e, &data, &bytes, APR_BLOCK_READ); + new = apr_bucket_transient_create(data, bytes, r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(to, new); + } + else if (APR_BUCKET_IS_FLUSH(e)) { + new = apr_bucket_flush_create(r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(to, new); + } + else if (APR_BUCKET_IS_EOS(e)) { + new = apr_bucket_eos_create(r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(to, new); + } + else { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, + "proxy: Unhandled bucket type of type %s in" + " ap_proxy_buckets_lifetime_transform", e->type->name); + rv = APR_EGENERAL; + } + } + return rv; +} +