From: Stefan Eissing Date: Mon, 24 Jun 2024 12:38:11 +0000 (+0000) Subject: Merge GH PR #454 X-Git-Tag: 2.4.60-rc1-candidate~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c66fe1a1dc34f6ad9f3e1fbf7c53a4b9de4bff2c;p=thirdparty%2Fapache%2Fhttpd.git Merge GH PR #454 *) mod_proxy: Fix DNS requests and connections closed before the configured addressTTL. BZ 69126. [Yann Ylavic] git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1918543 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/changes-entries/fix_proxy_determine_address.txt b/changes-entries/fix_proxy_determine_address.txt new file mode 100644 index 00000000000..9f5f33a35c1 --- /dev/null +++ b/changes-entries/fix_proxy_determine_address.txt @@ -0,0 +1,2 @@ + *) mod_proxy: Fix DNS requests and connections closed before the + configured addressTTL. BZ 69126. [Yann Ylavic] diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index a54a4face21..3da6ebf2a15 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1512,8 +1512,9 @@ static void socket_cleanup(proxy_conn_rec *conn) apr_pool_clear(conn->scpool); } -static void address_cleanup(proxy_conn_rec *conn) +static void conn_cleanup(proxy_conn_rec *conn) { + socket_cleanup(conn); conn->address = NULL; conn->addr = NULL; conn->hostname = NULL; @@ -1522,9 +1523,6 @@ static void address_cleanup(proxy_conn_rec *conn) if (conn->uds_pool) { apr_pool_clear(conn->uds_pool); } - if (conn->sock) { - socket_cleanup(conn); - } } static apr_status_t conn_pool_cleanup(void *theworker) @@ -2784,8 +2782,8 @@ static apr_status_t worker_address_resolve(proxy_worker *worker, apr_sockaddr_t *addr = *paddr; for (; addr; addr = addr->next) { addrs = apr_psprintf(pool, "%s%s%pI", - addrs ? ", " : "", addrs ? addrs : "", + addrs ? ", " : "", addr); } if (r) { @@ -2959,15 +2957,16 @@ PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char *proxy_functio */ address->expiry = apr_atomic_read32(&worker->s->address_expiry); if (address->expiry <= now) { - apr_uint32_t new_expiry = address->expiry + ttl; - while (new_expiry <= now) { - new_expiry += ttl; - } - new_expiry = apr_atomic_cas32(&worker->s->address_expiry, - new_expiry, address->expiry); - /* race lost? well the expiry should grow anyway.. */ - AP_DEBUG_ASSERT(new_expiry > now); - address->expiry = new_expiry; + apr_uint32_t prev, next = (now + ttl) - (now % ttl); + do { + prev = apr_atomic_cas32(&worker->s->address_expiry, + next, address->expiry); + if (prev == address->expiry) { + address->expiry = next; + break; + } + address->expiry = prev; + } while (prev <= now); } } else { @@ -3008,13 +3007,40 @@ PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char *proxy_functio PROXY_THREAD_UNLOCK(worker); - /* Kill any socket using the old address */ - if (conn->sock) { - if (r ? APLOGrdebug(r) : APLOGdebug(s)) { - /* XXX: this requires the old conn->addr[ess] to still - * be alive since it's not copied by apr_socket_connect() - * in ap_proxy_connect_backend(). - */ + /* Release the old conn address */ + if (conn->address) { + /* On Windows and OS/2, apr_socket_connect() called from + * ap_proxy_connect_backend() does a simple pointer copy of + * its given conn->addr[->next] into conn->sock->remote_addr. + * Thus conn->addr cannot be freed if the conn->sock should be + * kept alive (same new and old addresses) and the old address + * is still in conn->sock->remote_addr. In this case we rather + * delay the release of the old address by moving the cleanup + * to conn->scpool such that it runs when the socket is closed. + * In any other case, including other platforms, just release + * the old address now since conn->sock->remote_addr is either + * obsolete (socket forcibly closed) or a copy on conn->scpool + * already (not a dangling pointer). + */ + int keep_addr_alive = 0, + keep_conn_alive = (conn->sock && conn->addr && + proxy_addrs_equal(conn->addr, + address->addr)); + if (keep_conn_alive) { +#if defined(WIN32) || defined(OS2) + apr_sockaddr_t *remote_addr = NULL; + apr_socket_addr_get(&remote_addr, APR_REMOTE, conn->sock); + for (addr = conn->addr; addr; addr = addr->next) { + if (addr == remote_addr) { + keep_addr_alive = 1; + break; + } + } +#else + /* Nothing to do, keep_addr_alive = 0 */ +#endif + } + else if (conn->sock && (r ? APLOGrdebug(r) : APLOGdebug(s))) { apr_sockaddr_t *local_addr = NULL; apr_sockaddr_t *remote_addr = NULL; apr_socket_addr_get(&local_addr, APR_LOCAL, conn->sock); @@ -3032,18 +3058,26 @@ PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char *proxy_functio local_addr, remote_addr); } } - socket_cleanup(conn); + if (keep_addr_alive) { + apr_pool_cleanup_kill(conn->pool, conn->address, + proxy_address_cleanup); + apr_pool_cleanup_register(conn->scpool, conn->address, + proxy_address_cleanup, + apr_pool_cleanup_null); + } + else { + apr_pool_cleanup_run(conn->pool, conn->address, + proxy_address_cleanup); + if (!keep_conn_alive) { + conn_cleanup(conn); + } + } } - /* Kill the old address (if any) and use the new one */ - if (conn->address) { - apr_pool_cleanup_run(conn->pool, conn->address, - proxy_address_cleanup); - } + /* Use the new address */ apr_pool_cleanup_register(conn->pool, address, proxy_address_cleanup, apr_pool_cleanup_null); - address_cleanup(conn); conn->address = address; conn->hostname = address->hostname; conn->port = address->hostport; @@ -3125,7 +3159,7 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, if (!conn->uds_path || strcmp(conn->uds_path, uds_path) != 0) { apr_pool_t *pool = conn->pool; if (conn->uds_path) { - address_cleanup(conn); + conn_cleanup(conn); if (!conn->uds_pool) { apr_pool_create(&conn->uds_pool, worker->cp->dns_pool); } @@ -3226,7 +3260,7 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, if (conn->hostname && (conn->port != hostport || ap_cstr_casecmp(conn->hostname, hostname) != 0)) { - address_cleanup(conn); + conn_cleanup(conn); } /* Resolve the connection address with the determined hostname/port */