You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2007/11/02 22:58:57 UTC
svn commit: r591488 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c
Author: covener
Date: Fri Nov 2 14:58:57 2007
New Revision: 591488
URL: http://svn.apache.org/viewvc?rev=591488&view=rev
Log:
fix pool misuse around mod_ldap's connection cache, previously pconf
could be used during request processing
(the apr_ldap_foo only need a pool for temporary data anyway)
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/ldap/util_ldap.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=591488&r1=591487&r2=591488&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Nov 2 14:58:57 2007
@@ -2,6 +2,10 @@
Changes with Apache 2.3.0
[ When backported to 2.2.x, remove entry from this file ]
+ *) mod_ldap: Stop passing a reference to pconf around for
+ (limited) use during request processing, avoiding possible
+ memory corruption and crashes. [Eric Covener]
+
*) mod_status: Add SeeRequestTail directive, which determines if
ExtendedStatus displays the 1st 63 characters of the request
or the last 63. Useful for those requests with large string
Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=591488&r1=591487&r2=591488&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
+++ httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Nov 2 14:58:57 2007
@@ -217,7 +217,7 @@
* some hosts with ports and some without. All hosts which do not
* specify a port will use the default port.
*/
- apr_ldap_init(ldc->pool, &(ldc->ldap),
+ apr_ldap_init(r->pool, &(ldc->ldap),
ldc->host,
APR_LDAP_SSL == ldc->secure ? LDAPS_PORT : LDAP_PORT,
APR_LDAP_NONE,
@@ -245,7 +245,7 @@
/* set client certificates */
if (!apr_is_empty_array(ldc->client_certs)) {
- apr_ldap_set_option(ldc->pool, ldc->ldap, APR_LDAP_OPT_TLS_CERT,
+ apr_ldap_set_option(r->pool, ldc->ldap, APR_LDAP_OPT_TLS_CERT,
ldc->client_certs, &(result));
if (LDAP_SUCCESS != result->rc) {
uldap_connection_unbind( ldc );
@@ -256,7 +256,7 @@
/* switch on SSL/TLS */
if (APR_LDAP_NONE != ldc->secure) {
- apr_ldap_set_option(ldc->pool, ldc->ldap,
+ apr_ldap_set_option(r->pool, ldc->ldap,
APR_LDAP_OPT_TLS, &ldc->secure, &(result));
if (LDAP_SUCCESS != result->rc) {
uldap_connection_unbind( ldc );
@@ -271,7 +271,7 @@
/*XXX All of the #ifdef's need to be removed once apr-util 1.2 is released */
#ifdef APR_LDAP_OPT_VERIFY_CERT
- apr_ldap_set_option(ldc->pool, ldc->ldap,
+ apr_ldap_set_option(r->pool, ldc->ldap,
APR_LDAP_OPT_VERIFY_CERT, &(st->verify_svr_cert), &(result));
#else
#if defined(LDAPSSL_VERIFY_SERVER)
@@ -301,7 +301,7 @@
}
if (st->connectionTimeout >= 0) {
- rc = apr_ldap_set_option(ldc->pool, ldc->ldap, LDAP_OPT_NETWORK_TIMEOUT,
+ rc = apr_ldap_set_option(r->pool, ldc->ldap, LDAP_OPT_NETWORK_TIMEOUT,
(void *)&timeOut, &(result));
if (APR_SUCCESS != rc) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
@@ -533,11 +533,19 @@
*/
/* create the details to the pool in st */
l = apr_pcalloc(st->pool, sizeof(util_ldap_connection_t));
+ if (apr_pool_create(&l->pool, st->pool) != APR_SUCCESS) {
+ ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,
+ "util_ldap: Failed to create memory pool");
+#if APR_HAS_THREADS
+ apr_thread_mutex_unlock(st->mutex);
+#endif
+ return NULL;
+
+ }
#if APR_HAS_THREADS
apr_thread_mutex_create(&l->lock, APR_THREAD_MUTEX_DEFAULT, st->pool);
apr_thread_mutex_lock(l->lock);
#endif
- l->pool = st->pool;
l->bound = 0;
l->host = apr_pstrdup(st->pool, host);
l->port = port;
@@ -2278,7 +2286,7 @@
0,
&(result_err));
if (APR_SUCCESS == rc) {
- rc = apr_ldap_set_option(p, NULL, APR_LDAP_OPT_TLS_CERT,
+ rc = apr_ldap_set_option(ptemp, NULL, APR_LDAP_OPT_TLS_CERT,
(void *)st->global_certs, &(result_err));
}
Re: svn commit: r591488 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c
Posted by Eric Covener <co...@gmail.com>.
On 11/3/07, Ruediger Pluem <rp...@apache.org> wrote:
>
> What is the purpose of creating a sub pool here? I don't see directly that
> l->pool has a shorter live time than st->pool. Any pointers?
>
You are correct in that It is not required at the moment. It's a
compromise between zapping the pool reference in that
per-ldap-connection data structure completely and leaving it to
accomodate a future change in the lifetime of entries in the ldap
connection pool.
In the interim, since the entire structure is essentially 'checked
out' of the connection pool, l->pool can at safely be used during
request processing.
--
Eric Covener
covener@gmail.com
Re: svn commit: r591488 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 11/02/2007 10:58 PM, covener@apache.org wrote:
> Author: covener
> Date: Fri Nov 2 14:58:57 2007
> New Revision: 591488
>
> URL: http://svn.apache.org/viewvc?rev=591488&view=rev
> Log:
> fix pool misuse around mod_ldap's connection cache, previously pconf
> could be used during request processing
>
> (the apr_ldap_foo only need a pool for temporary data anyway)
>
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/ldap/util_ldap.c
>
> Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=591488&r1=591487&r2=591488&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
> +++ httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Nov 2 14:58:57 2007
> @@ -533,11 +533,19 @@
> */
> /* create the details to the pool in st */
> l = apr_pcalloc(st->pool, sizeof(util_ldap_connection_t));
> + if (apr_pool_create(&l->pool, st->pool) != APR_SUCCESS) {
What is the purpose of creating a sub pool here? I don't see directly that
l->pool has a shorter live time than st->pool. Any pointers?
Regards
Rüdiger