Branch '389-ds-base-1.3.1' - ldap/servers
by Noriko Hosoi
ldap/servers/plugins/replication/repl5_connection.c | 89 +++++++++++---------
ldap/servers/slapd/ldaputil.c | 39 ++++----
2 files changed, 69 insertions(+), 59 deletions(-)
New commits:
commit 2a80b7152823ca16628c2da48614166b8d2104a4
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Mon Mar 31 15:17:59 2014 -0400
Ticket 47759 - Crash in replication when server is under write load
Bug Description: When the server is under alot of load, a race condition allows
a replication connection LDAP struct to be freed(unbind) while
it is being used by another thread. This leads to a crash.
Fix Description: Extend the connection lock to also cover ldap client interaction
(e.g. conn->ld struct).
https://fedorahosted.org/389/ticket/47759
Reviewed by: nhosoi & rmeggins(Thanks!!)
(cherry picked from commit 9940ca29ca258891c52640a23adc2851afe59d0e)
(cherry picked from commit 0e576c85c34826c4d63d9578db55f8179b4a1a60)
diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c
index 668abda..17d1d9c 100644
--- a/ldap/servers/plugins/replication/repl5_connection.c
+++ b/ldap/servers/plugins/replication/repl5_connection.c
@@ -138,6 +138,7 @@ static void repl5_debug_timeout_callback(time_t when, void *arg);
/* Forward declarations */
static void close_connection_internal(Repl_Connection *conn);
+static void conn_delete_internal(Repl_Connection *conn);
/*
* Create a new connection object. Returns a pointer to the object, or
@@ -182,11 +183,22 @@ conn_new(Repl_Agmt *agmt)
rpc->plain = NULL;
return rpc;
loser:
- conn_delete(rpc);
+ conn_delete_internal(rpc);
slapi_ch_free((void**)&rpc);
return NULL;
}
+static PRBool
+conn_connected_locked(Repl_Connection *conn, int locked)
+{
+ PRBool return_value;
+
+ if(!locked) PR_Lock(conn->lock);
+ return_value = STATE_CONNECTED == conn->state;
+ if(!locked) PR_Unlock(conn->lock);
+
+ return return_value;
+}
/*
* Return PR_TRUE if the connection is in the connected state
@@ -194,14 +206,9 @@ loser:
static PRBool
conn_connected(Repl_Connection *conn)
{
- PRBool return_value;
- PR_Lock(conn->lock);
- return_value = STATE_CONNECTED == conn->state;
- PR_Unlock(conn->lock);
- return return_value;
+ return conn_connected_locked(conn, 1);
}
-
/*
* Destroy a connection object.
*/
@@ -243,7 +250,6 @@ conn_delete(Repl_Connection *conn)
if (slapi_eq_cancel(conn->linger_event) == 1)
{
/* Event was found and cancelled. Destroy the connection object. */
- PR_Unlock(conn->lock);
destroy_it = PR_TRUE;
}
else
@@ -254,16 +260,15 @@ conn_delete(Repl_Connection *conn)
* off, so arrange for the event to destroy the object .
*/
conn->delete_after_linger = PR_TRUE;
- PR_Unlock(conn->lock);
}
}
if (destroy_it)
{
conn_delete_internal(conn);
}
+ PR_Unlock(conn->lock);
}
-
/*
* Return the last operation type processed by the connection
* object, and the LDAP error encountered.
@@ -327,17 +332,18 @@ conn_read_result_ex(Repl_Connection *conn, char **retoidp, struct berval **retda
while (!slapi_is_shutting_down())
{
/* we have to make sure the update sending thread does not
- attempt to call conn_disconnect while we are reading
+ attempt to close connection while we are reading
results - so lock the conn while we get the results */
PR_Lock(conn->lock);
+
if ((STATE_CONNECTED != conn->state) || !conn->ld) {
rc = -1;
return_value = CONN_NOT_CONNECTED;
PR_Unlock(conn->lock);
break;
}
-
rc = ldap_result(conn->ld, send_msgid, 1, &local_timeout, &res);
+
PR_Unlock(conn->lock);
if (0 != rc)
@@ -661,8 +667,10 @@ perform_operation(Repl_Connection *conn, int optype, const char *dn,
server_controls[1] = update_control;
server_controls[2] = NULL;
- /* lock the conn to prevent the result reader thread
- from closing the connection out from under us */
+ /*
+ * Lock the conn to prevent the result reader thread
+ * from closing the connection out from under us.
+ */
PR_Lock(conn->lock);
if (STATE_CONNECTED == conn->state)
{
@@ -804,7 +812,6 @@ conn_send_rename(Repl_Connection *conn, const char *dn,
NULL /* extop OID */, NULL /* extop payload */, message_id);
}
-
/*
* Send an LDAP extended operation.
*/
@@ -818,7 +825,6 @@ conn_send_extended_operation(Repl_Connection *conn, const char *extop_oid,
update_control, extop_oid, payload, message_id);
}
-
/*
* Synchronously read an entry and return a specific attribute's values.
* Returns CONN_OPERATION_SUCCESS if successful. Returns
@@ -838,6 +844,8 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
LDAPMessage *res = NULL;
char *attrs[2];
+ PR_Lock(conn->lock);
+
PR_ASSERT(NULL != type);
if (conn_connected(conn))
{
@@ -860,7 +868,7 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
}
else if (IS_DISCONNECT_ERROR(ldap_rc))
{
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -878,10 +886,11 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
{
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
-
/*
* Return an pointer to a string describing the connection's status.
*/
@@ -892,8 +901,6 @@ conn_get_status(Repl_Connection *conn)
return conn->status;
}
-
-
/*
* Cancel any outstanding linger timer. Should be called when
* a replication session is beginning.
@@ -925,7 +932,6 @@ conn_cancel_linger(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
/*
* Called when our linger timeout timer expires. This means
* we should check to see if perhaps the connection's become
@@ -957,7 +963,6 @@ linger_timeout(time_t event_time, void *arg)
}
}
-
/*
* Indicate that a session is ending. The linger timer starts when
* this function is called.
@@ -995,8 +1000,6 @@ conn_start_linger(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
-
/*
* If no connection is currently active, opens a connection and binds to
* the remote server. If a connection is open (e.g. lingering) then
@@ -1015,10 +1018,14 @@ conn_connect(Repl_Connection *conn)
ConnResult return_value = CONN_OPERATION_SUCCESS;
int pw_ret = 1;
- /** Connection already open just return SUCCESS **/
- if(conn->state == STATE_CONNECTED) goto done;
-
PR_Lock(conn->lock);
+
+ /* Connection already open, just return SUCCESS */
+ if(conn->state == STATE_CONNECTED){
+ PR_Unlock(conn->lock);
+ return return_value;
+ }
+
if (conn->flag_agmt_changed) {
/* So far we cannot change Hostname and Port */
/* slapi_ch_free((void **)&conn->hostname); */
@@ -1033,7 +1040,6 @@ conn_connect(Repl_Connection *conn)
conn->port = agmt_get_port(conn->agmt); /* port could be updated */
slapi_ch_free((void **)&conn->plain);
}
- PR_Unlock(conn->lock);
creds = agmt_get_credentials(conn->agmt);
@@ -1174,6 +1180,7 @@ done:
{
close_connection_internal(conn);
}
+ PR_Unlock(conn->lock);
return return_value;
}
@@ -1209,7 +1216,6 @@ conn_disconnect(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
/*
* Determine if the remote replica supports DS 5.0 replication.
* Return codes:
@@ -1226,6 +1232,7 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds50_repl == -1) {
@@ -1273,7 +1280,7 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1293,10 +1300,11 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
-
/*
* Determine if the remote replica supports DS 7.1 replication.
* Return codes:
@@ -1313,6 +1321,7 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds71_repl == -1) {
@@ -1344,7 +1353,7 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1364,6 +1373,8 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
@@ -1383,6 +1394,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds90_repl == -1) {
@@ -1414,7 +1426,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1423,7 +1435,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
}
}
if (NULL != res)
- ldap_msgfree(res);
+ ldap_msgfree(res);
}
else
{
@@ -1435,6 +1447,8 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
@@ -1452,7 +1466,6 @@ conn_replica_is_readonly(Repl_Connection *conn)
}
}
-
/*
* Return 1 if "value" is a value of attribute type "type" in entry "entry".
* Otherwise, return 0.
@@ -1501,9 +1514,6 @@ attribute_string_value_present(LDAP *ld, LDAPMessage *entry, const char *type,
return return_value;
}
-
-
-
/*
* Read the remote server's schema entry, then read the local schema entry,
* and compare the nsschemacsn attribute. If the local csn is newer, or
@@ -1533,7 +1543,7 @@ conn_push_schema(Repl_Connection *conn, CSN **remotecsn)
return_value = CONN_OPERATION_FAILED;
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "NULL remote CSN\n");
}
- else if (!conn_connected(conn))
+ else if (!conn_connected_locked(conn, 0 /* not locked */))
{
return_value = CONN_NOT_CONNECTED;
slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
@@ -1699,6 +1709,7 @@ conn_push_schema(Repl_Connection *conn, CSN **remotecsn)
{
csn_free(&localcsn);
}
+
return return_value;
}
diff --git a/ldap/servers/slapd/ldaputil.c b/ldap/servers/slapd/ldaputil.c
index edc8267..08601bd 100644
--- a/ldap/servers/slapd/ldaputil.c
+++ b/ldap/servers/slapd/ldaputil.c
@@ -1011,8 +1011,8 @@ slapi_ldap_bind(
than the currently unused clientctrls */
ldap_get_option(ld, LDAP_OPT_CLIENT_CONTROLS, &clientctrls);
if (clientctrls && clientctrls[0] &&
- slapi_control_present(clientctrls, START_TLS_OID, NULL, NULL)) {
- secure = 2;
+ slapi_control_present(clientctrls, START_TLS_OID, NULL, NULL)) {
+ secure = 2;
} else {
#if defined(USE_OPENLDAP)
/* openldap doesn't have a SSL/TLS yes/no flag - so grab the
@@ -1051,12 +1051,12 @@ slapi_ldap_bind(
slapi_log_error(SLAPI_LOG_SHELL, "slapi_ldap_bind",
"Set up conn to use client auth\n");
}
- bvcreds.bv_val = NULL; /* ignore username and passed in creds */
- bvcreds.bv_len = 0; /* for external auth */
- bindid = NULL;
+ bvcreds.bv_val = NULL; /* ignore username and passed in creds */
+ bvcreds.bv_len = 0; /* for external auth */
+ bindid = NULL;
} else { /* other type of auth */
- bvcreds.bv_val = (char *)creds;
- bvcreds.bv_len = creds ? strlen(creds) : 0;
+ bvcreds.bv_val = (char *)creds;
+ bvcreds.bv_len = creds ? strlen(creds) : 0;
}
if (secure == 2) { /* send start tls */
@@ -1084,31 +1084,29 @@ slapi_ldap_bind(
bindid, creds);
if ((rc = ldap_sasl_bind(ld, bindid, mech, &bvcreds, serverctrls,
NULL /* clientctrls */, &mymsgid))) {
- char *myhostname = NULL;
- char *copy = NULL;
+ char *hostname = NULL;
+ char *host_port = NULL;
char *ptr = NULL;
int myerrno = errno;
int gaierr = 0;
- ldap_get_option(ld, LDAP_OPT_HOST_NAME, &myhostname);
- if (myhostname) {
- ptr = strchr(myhostname, ':');
+ ldap_get_option(ld, LDAP_OPT_HOST_NAME, &host_port);
+ if (host_port) {
+ ptr = strchr(host_port, ':');
if (ptr) {
- copy = slapi_ch_strdup(myhostname);
- *(copy + (ptr - myhostname)) = '\0';
- slapi_ch_free_string(&myhostname);
- myhostname = copy;
+ hostname = slapi_ch_strdup(host_port);
+ *(hostname + (ptr - host_port)) = '\0';
}
}
-
if (0 == myerrno) {
struct addrinfo *result = NULL;
- gaierr = getaddrinfo(myhostname, NULL, NULL, &result);
+ gaierr = getaddrinfo(hostname, NULL, NULL, &result);
myerrno = errno;
if (result) {
freeaddrinfo(result);
}
}
+
slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind",
"Error: could not send bind request for id "
"[%s] authentication mechanism [%s]: error %d (%s), system error %d (%s), "
@@ -1119,8 +1117,9 @@ slapi_ldap_bind(
PR_GetError(), slapd_pr_strerror(PR_GetError()),
myerrno ? myerrno : gaierr,
myerrno ? slapd_system_strerror(myerrno) : gai_strerror(gaierr),
- myhostname ? myhostname : "unknown host");
- slapi_ch_free_string(&myhostname);
+ host_port ? host_port : "unknown host");
+ slapi_ch_free_string(&hostname);
+ slapi_ch_free_string(&host_port);
goto done;
}
9 years, 6 months
- 1
- 0
Branch '389-ds-base-1.3.2' - ldap/servers
by Noriko Hosoi
ldap/servers/plugins/replication/repl5_connection.c | 89 +++++++++++---------
ldap/servers/slapd/ldaputil.c | 39 ++++----
2 files changed, 69 insertions(+), 59 deletions(-)
New commits:
commit 0e576c85c34826c4d63d9578db55f8179b4a1a60
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Mon Mar 31 15:17:59 2014 -0400
Ticket 47759 - Crash in replication when server is under write load
Bug Description: When the server is under alot of load, a race condition allows
a replication connection LDAP struct to be freed(unbind) while
it is being used by another thread. This leads to a crash.
Fix Description: Extend the connection lock to also cover ldap client interaction
(e.g. conn->ld struct).
https://fedorahosted.org/389/ticket/47759
Reviewed by: nhosoi & rmeggins(Thanks!!)
(cherry picked from commit 9940ca29ca258891c52640a23adc2851afe59d0e)
diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c
index 4bd14a8..2069a98 100644
--- a/ldap/servers/plugins/replication/repl5_connection.c
+++ b/ldap/servers/plugins/replication/repl5_connection.c
@@ -142,6 +142,7 @@ static void repl5_debug_timeout_callback(time_t when, void *arg);
/* Forward declarations */
static void close_connection_internal(Repl_Connection *conn);
+static void conn_delete_internal(Repl_Connection *conn);
/*
* Create a new connection object. Returns a pointer to the object, or
@@ -186,11 +187,22 @@ conn_new(Repl_Agmt *agmt)
rpc->plain = NULL;
return rpc;
loser:
- conn_delete(rpc);
+ conn_delete_internal(rpc);
slapi_ch_free((void**)&rpc);
return NULL;
}
+static PRBool
+conn_connected_locked(Repl_Connection *conn, int locked)
+{
+ PRBool return_value;
+
+ if(!locked) PR_Lock(conn->lock);
+ return_value = STATE_CONNECTED == conn->state;
+ if(!locked) PR_Unlock(conn->lock);
+
+ return return_value;
+}
/*
* Return PR_TRUE if the connection is in the connected state
@@ -198,14 +210,9 @@ loser:
static PRBool
conn_connected(Repl_Connection *conn)
{
- PRBool return_value;
- PR_Lock(conn->lock);
- return_value = STATE_CONNECTED == conn->state;
- PR_Unlock(conn->lock);
- return return_value;
+ return conn_connected_locked(conn, 1);
}
-
/*
* Destroy a connection object.
*/
@@ -247,7 +254,6 @@ conn_delete(Repl_Connection *conn)
if (slapi_eq_cancel(conn->linger_event) == 1)
{
/* Event was found and cancelled. Destroy the connection object. */
- PR_Unlock(conn->lock);
destroy_it = PR_TRUE;
}
else
@@ -258,16 +264,15 @@ conn_delete(Repl_Connection *conn)
* off, so arrange for the event to destroy the object .
*/
conn->delete_after_linger = PR_TRUE;
- PR_Unlock(conn->lock);
}
}
if (destroy_it)
{
conn_delete_internal(conn);
}
+ PR_Unlock(conn->lock);
}
-
/*
* Return the last operation type processed by the connection
* object, and the LDAP error encountered.
@@ -331,17 +336,18 @@ conn_read_result_ex(Repl_Connection *conn, char **retoidp, struct berval **retda
while (!slapi_is_shutting_down())
{
/* we have to make sure the update sending thread does not
- attempt to call conn_disconnect while we are reading
+ attempt to close connection while we are reading
results - so lock the conn while we get the results */
PR_Lock(conn->lock);
+
if ((STATE_CONNECTED != conn->state) || !conn->ld) {
rc = -1;
return_value = CONN_NOT_CONNECTED;
PR_Unlock(conn->lock);
break;
}
-
rc = ldap_result(conn->ld, send_msgid, 1, &local_timeout, &res);
+
PR_Unlock(conn->lock);
if (0 != rc)
@@ -665,8 +671,10 @@ perform_operation(Repl_Connection *conn, int optype, const char *dn,
server_controls[1] = update_control;
server_controls[2] = NULL;
- /* lock the conn to prevent the result reader thread
- from closing the connection out from under us */
+ /*
+ * Lock the conn to prevent the result reader thread
+ * from closing the connection out from under us.
+ */
PR_Lock(conn->lock);
if (STATE_CONNECTED == conn->state)
{
@@ -808,7 +816,6 @@ conn_send_rename(Repl_Connection *conn, const char *dn,
NULL /* extop OID */, NULL /* extop payload */, message_id);
}
-
/*
* Send an LDAP extended operation.
*/
@@ -822,7 +829,6 @@ conn_send_extended_operation(Repl_Connection *conn, const char *extop_oid,
update_control, extop_oid, payload, message_id);
}
-
/*
* Synchronously read an entry and return a specific attribute's values.
* Returns CONN_OPERATION_SUCCESS if successful. Returns
@@ -842,6 +848,8 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
LDAPMessage *res = NULL;
char *attrs[2];
+ PR_Lock(conn->lock);
+
PR_ASSERT(NULL != type);
if (conn_connected(conn))
{
@@ -864,7 +872,7 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
}
else if (IS_DISCONNECT_ERROR(ldap_rc))
{
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -882,10 +890,11 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
{
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
-
/*
* Return an pointer to a string describing the connection's status.
*/
@@ -896,8 +905,6 @@ conn_get_status(Repl_Connection *conn)
return conn->status;
}
-
-
/*
* Cancel any outstanding linger timer. Should be called when
* a replication session is beginning.
@@ -929,7 +936,6 @@ conn_cancel_linger(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
/*
* Called when our linger timeout timer expires. This means
* we should check to see if perhaps the connection's become
@@ -961,7 +967,6 @@ linger_timeout(time_t event_time, void *arg)
}
}
-
/*
* Indicate that a session is ending. The linger timer starts when
* this function is called.
@@ -999,8 +1004,6 @@ conn_start_linger(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
-
/*
* If no connection is currently active, opens a connection and binds to
* the remote server. If a connection is open (e.g. lingering) then
@@ -1019,10 +1022,14 @@ conn_connect(Repl_Connection *conn)
ConnResult return_value = CONN_OPERATION_SUCCESS;
int pw_ret = 1;
- /** Connection already open just return SUCCESS **/
- if(conn->state == STATE_CONNECTED) goto done;
-
PR_Lock(conn->lock);
+
+ /* Connection already open, just return SUCCESS */
+ if(conn->state == STATE_CONNECTED){
+ PR_Unlock(conn->lock);
+ return return_value;
+ }
+
if (conn->flag_agmt_changed) {
/* So far we cannot change Hostname and Port */
/* slapi_ch_free((void **)&conn->hostname); */
@@ -1037,7 +1044,6 @@ conn_connect(Repl_Connection *conn)
conn->port = agmt_get_port(conn->agmt); /* port could be updated */
slapi_ch_free((void **)&conn->plain);
}
- PR_Unlock(conn->lock);
creds = agmt_get_credentials(conn->agmt);
@@ -1178,6 +1184,7 @@ done:
{
close_connection_internal(conn);
}
+ PR_Unlock(conn->lock);
return return_value;
}
@@ -1213,7 +1220,6 @@ conn_disconnect(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
/*
* Determine if the remote replica supports DS 5.0 replication.
* Return codes:
@@ -1230,6 +1236,7 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds50_repl == -1) {
@@ -1277,7 +1284,7 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1297,10 +1304,11 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
-
/*
* Determine if the remote replica supports DS 7.1 replication.
* Return codes:
@@ -1317,6 +1325,7 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds71_repl == -1) {
@@ -1348,7 +1357,7 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1368,6 +1377,8 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
@@ -1387,6 +1398,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds90_repl == -1) {
@@ -1418,7 +1430,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1427,7 +1439,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
}
}
if (NULL != res)
- ldap_msgfree(res);
+ ldap_msgfree(res);
}
else
{
@@ -1439,6 +1451,8 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
@@ -1456,7 +1470,6 @@ conn_replica_is_readonly(Repl_Connection *conn)
}
}
-
/*
* Return 1 if "value" is a value of attribute type "type" in entry "entry".
* Otherwise, return 0.
@@ -1505,9 +1518,6 @@ attribute_string_value_present(LDAP *ld, LDAPMessage *entry, const char *type,
return return_value;
}
-
-
-
/*
* Read the remote server's schema entry, then read the local schema entry,
* and compare the nsschemacsn attribute. If the local csn is newer, or
@@ -1537,7 +1547,7 @@ conn_push_schema(Repl_Connection *conn, CSN **remotecsn)
return_value = CONN_OPERATION_FAILED;
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "NULL remote CSN\n");
}
- else if (!conn_connected(conn))
+ else if (!conn_connected_locked(conn, 0 /* not locked */))
{
return_value = CONN_NOT_CONNECTED;
slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
@@ -1730,6 +1740,7 @@ conn_push_schema(Repl_Connection *conn, CSN **remotecsn)
{
csn_free(&localcsn);
}
+
return return_value;
}
diff --git a/ldap/servers/slapd/ldaputil.c b/ldap/servers/slapd/ldaputil.c
index ed3491e..df423e5 100644
--- a/ldap/servers/slapd/ldaputil.c
+++ b/ldap/servers/slapd/ldaputil.c
@@ -1014,8 +1014,8 @@ slapi_ldap_bind(
than the currently unused clientctrls */
ldap_get_option(ld, LDAP_OPT_CLIENT_CONTROLS, &clientctrls);
if (clientctrls && clientctrls[0] &&
- slapi_control_present(clientctrls, START_TLS_OID, NULL, NULL)) {
- secure = 2;
+ slapi_control_present(clientctrls, START_TLS_OID, NULL, NULL)) {
+ secure = 2;
} else {
#if defined(USE_OPENLDAP)
/* openldap doesn't have a SSL/TLS yes/no flag - so grab the
@@ -1054,12 +1054,12 @@ slapi_ldap_bind(
slapi_log_error(SLAPI_LOG_SHELL, "slapi_ldap_bind",
"Set up conn to use client auth\n");
}
- bvcreds.bv_val = NULL; /* ignore username and passed in creds */
- bvcreds.bv_len = 0; /* for external auth */
- bindid = NULL;
+ bvcreds.bv_val = NULL; /* ignore username and passed in creds */
+ bvcreds.bv_len = 0; /* for external auth */
+ bindid = NULL;
} else { /* other type of auth */
- bvcreds.bv_val = (char *)creds;
- bvcreds.bv_len = creds ? strlen(creds) : 0;
+ bvcreds.bv_val = (char *)creds;
+ bvcreds.bv_len = creds ? strlen(creds) : 0;
}
if (secure == 2) { /* send start tls */
@@ -1087,31 +1087,29 @@ slapi_ldap_bind(
bindid, creds);
if ((rc = ldap_sasl_bind(ld, bindid, mech, &bvcreds, serverctrls,
NULL /* clientctrls */, &mymsgid))) {
- char *myhostname = NULL;
- char *copy = NULL;
+ char *hostname = NULL;
+ char *host_port = NULL;
char *ptr = NULL;
int myerrno = errno;
int gaierr = 0;
- ldap_get_option(ld, LDAP_OPT_HOST_NAME, &myhostname);
- if (myhostname) {
- ptr = strchr(myhostname, ':');
+ ldap_get_option(ld, LDAP_OPT_HOST_NAME, &host_port);
+ if (host_port) {
+ ptr = strchr(host_port, ':');
if (ptr) {
- copy = slapi_ch_strdup(myhostname);
- *(copy + (ptr - myhostname)) = '\0';
- slapi_ch_free_string(&myhostname);
- myhostname = copy;
+ hostname = slapi_ch_strdup(host_port);
+ *(hostname + (ptr - host_port)) = '\0';
}
}
-
if (0 == myerrno) {
struct addrinfo *result = NULL;
- gaierr = getaddrinfo(myhostname, NULL, NULL, &result);
+ gaierr = getaddrinfo(hostname, NULL, NULL, &result);
myerrno = errno;
if (result) {
freeaddrinfo(result);
}
}
+
slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind",
"Error: could not send bind request for id "
"[%s] authentication mechanism [%s]: error %d (%s), system error %d (%s), "
@@ -1122,8 +1120,9 @@ slapi_ldap_bind(
PR_GetError(), slapd_pr_strerror(PR_GetError()),
myerrno ? myerrno : gaierr,
myerrno ? slapd_system_strerror(myerrno) : gai_strerror(gaierr),
- myhostname ? myhostname : "unknown host");
- slapi_ch_free_string(&myhostname);
+ host_port ? host_port : "unknown host");
+ slapi_ch_free_string(&hostname);
+ slapi_ch_free_string(&host_port);
goto done;
}
9 years, 6 months
- 1
- 0
Branch '389-ds-base-1.2.11' - ldap/servers
by Noriko Hosoi
ldap/servers/slapd/valueset.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
New commits:
commit d36f7ea51cb8cb37d4d937dcf47254bf12a41c6b
Author: Rich Megginson <rmeggins(a)redhat.com>
Date: Mon Jul 29 12:36:02 2013 -0600
Ticket #47448 - Segfault in 389-ds-base-1.3.1.4-1.fc19 when setting up FreeIPA replication
https://fedorahosted.org/389/ticket/47448
Reviewed by: lkrispenz (Thanks!)
Branch: master
Fix Description: valueset_add_valueset() sets the values in the vs1
destination valueset. It expects that vs1 is empty. Particularly, the
sorted array. If the source valueset vs2->sorted is NULL, it assumes
vs1->sorted is NULL already, and does not free it and set it to NULL.
The fix is to free both vs1->sorted and vs1->va. NOTE: this fixes
the crash, but does not address the larger issue that the semantics of
valueset_add_valueset are not correct - valueset_add_valueset should add
the values from vs2 to vs1, rather than replace vs1 with vs2.
Also added post-condition assertions.
Platforms tested: RHEL6 x86_64
Flag Day: no
Doc impact: no
(cherry picked from commit df53a874436503ef99594fc09e3d817317f86940)
Backported from 389-ds-base-1.3.1 to 389-ds-base-1.2.11.
The patch was reviewed by rmeggins(a)redhat.com (Thank you, Rich!!)
NOTE: this patch is needed for Ticket #346 - Slow ldapmodify operation
time for large quantities of multi-valued attribute values
diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c
index 29078d4..e83e740 100644
--- a/ldap/servers/slapd/valueset.c
+++ b/ldap/servers/slapd/valueset.c
@@ -573,6 +573,7 @@ slapi_valueset_done(Slapi_ValueSet *vs)
{
if(vs!=NULL)
{
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
if(vs->va!=NULL)
{
valuearray_free(&vs->va);
@@ -604,6 +605,7 @@ slapi_valueset_set_from_smod(Slapi_ValueSet *vs, Slapi_Mod *smod)
Slapi_Value **va= NULL;
valuearray_init_bervalarray(slapi_mod_get_ldapmod_byref(smod)->mod_bvalues, &va);
valueset_set_valuearray_passin(vs, va);
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
}
void
@@ -624,7 +626,7 @@ valueset_set_valuearray_byval(Slapi_ValueSet *vs, Slapi_Value **addvals)
}
}
vs->va[j] = NULL;
-
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
}
void
@@ -634,6 +636,7 @@ valueset_set_valuearray_passin(Slapi_ValueSet *vs, Slapi_Value **addvals)
vs->va= addvals;
vs->num = valuearray_count(addvals);
vs->max = vs->num + 1;
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
}
void
@@ -747,6 +750,7 @@ valueset_remove_value_sorted(const Slapi_Attr *a, Slapi_ValueSet *vs, const Slap
for (i=0; i < vs->num; i++) {
if (vs->sorted[i] > index) vs->sorted[i]--;
}
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
}
return r;
}
@@ -763,6 +767,7 @@ valueset_remove_value(const Slapi_Attr *a, Slapi_ValueSet *vs, const Slapi_Value
if (r)
vs->num--;
}
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
return r;
}
}
@@ -791,6 +796,7 @@ valueset_purge(Slapi_ValueSet *vs, const CSN *csn)
slapi_ch_free ((void **)&vs->sorted);
vs->sorted = NULL;
}
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
}
return 0;
}
@@ -980,6 +986,7 @@ valueset_array_to_sorted (const Slapi_Attr *a, Slapi_ValueSet *vs)
}
vs->sorted[j+1] = swap;
}
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
}
/* insert a value into a sorted array, if dupcheck is set no duplicate values will be accepted
* (is there a reason to allow duplicates ? LK
@@ -995,10 +1002,12 @@ valueset_insert_value_to_sorted(const Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_V
if (vs->num == 0) {
vs->sorted[0] = 0;
vs->num++;
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
return(0);
} else if (valueset_value_cmp (a, vi, vs->va[vs->sorted[vs->num-1]]) > 0 ) {
vs->sorted[vs->num] = vs->num;
vs->num++;
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
return (vs->num);
}
v = valueset_find_sorted (a, vs, vi, &index);
@@ -1009,6 +1018,7 @@ valueset_insert_value_to_sorted(const Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_V
memmove(&vs->sorted[index+1],&vs->sorted[index],(vs->num - index)* sizeof(int));
vs->sorted[index] = vs->num;
vs->num++;
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
return(index);
}
@@ -1111,6 +1121,7 @@ slapi_valueset_add_attr_valuearray_ext(const Slapi_Attr *a, Slapi_ValueSet *vs,
}
(vs->va)[vs->num] = NULL;
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
return (rc);
}
@@ -1148,6 +1159,8 @@ valueset_add_valueset(Slapi_ValueSet *vs1, const Slapi_ValueSet *vs2)
int i;
if (vs1 && vs2) {
+ valuearray_free(&vs1->va);
+ slapi_ch_free((void **)&vs1->sorted);
if (vs2->va) {
/* need to copy valuearray */
if (vs2->max == 0) {
@@ -1168,6 +1181,7 @@ valueset_add_valueset(Slapi_ValueSet *vs1, const Slapi_ValueSet *vs2)
vs1->sorted = (int *) slapi_ch_malloc( vs1->max* sizeof(int));
memcpy(&vs1->sorted[0],&vs2->sorted[0],vs1->num* sizeof(int));
}
+ PR_ASSERT((vs1->sorted == NULL) || (vs1->num == 0) || ((vs1->sorted[0] >= 0) && (vs1->sorted[0] < vs1->num)));
}
}
@@ -1322,6 +1336,7 @@ valueset_replace_valuearray_ext(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *
vs->va = valstoreplace;
vs->num = vals_count;
vs->max = vals_count + 1;
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
} else {
/* verify the given values are not duplicated. */
unsigned long flags = SLAPI_VALUE_FLAG_PASSIN|SLAPI_VALUE_FLAG_DUPCHECK;
@@ -1349,6 +1364,7 @@ valueset_replace_valuearray_ext(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *
vs->num = vs_new->num;
vs->max = vs_new->max;
slapi_valueset_free (vs_new);
+ PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
}
else
{
9 years, 6 months
- 1
- 0
ldap/servers
by Mark Reynolds
ldap/servers/plugins/replication/repl5.h | 1
ldap/servers/plugins/replication/repl5_tot_protocol.c | 31 ++++++++++++------
2 files changed, 22 insertions(+), 10 deletions(-)
New commits:
commit 48cd960a5660aec2ab21367f6fb602df08ec8fc1
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Tue Apr 1 13:25:40 2014 -0400
Ticket 47655 - Improve replication total update logging
Description: If an online initialization failed, the error message was vague and
did not fully imply that it failed.
Also, in repl5_tot_waitfor_async_results() we did not take into
account if progress was being made, and our timeout was incorrectly
set to 5 minutes, instead of 30 seconds.
https://fedorahosted.org/389/ticket/47655
Reviewed by: rmeggins(Thanks!)
diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h
index 9aee2b5..af0d19d 100644
--- a/ldap/servers/plugins/replication/repl5.h
+++ b/ldap/servers/plugins/replication/repl5.h
@@ -445,6 +445,7 @@ typedef enum
CONN_SUPPORTS_DS90_REPL,
CONN_DOES_NOT_SUPPORT_DS90_REPL
} ConnResult;
+
Repl_Connection *conn_new(Repl_Agmt *agmt);
ConnResult conn_connect(Repl_Connection *conn);
void conn_disconnect(Repl_Connection *conn);
diff --git a/ldap/servers/plugins/replication/repl5_tot_protocol.c b/ldap/servers/plugins/replication/repl5_tot_protocol.c
index 2db5178..d4f0fcc 100644
--- a/ldap/servers/plugins/replication/repl5_tot_protocol.c
+++ b/ldap/servers/plugins/replication/repl5_tot_protocol.c
@@ -282,6 +282,8 @@ repl5_tot_waitfor_async_results(callback_data *cb_data)
{
int done = 0;
int loops = 0;
+ int last_entry = 0;
+
/* Keep pulling results off the LDAP connection until we catch up to the last message id stored in the rd */
while (!done)
{
@@ -304,9 +306,16 @@ repl5_tot_waitfor_async_results(callback_data *cb_data)
/* If not then sleep a bit */
DS_Sleep(PR_SecondsToInterval(1));
loops++;
+
+ if(last_entry < cb_data->last_message_id_received){
+ /* we are making progress - reset the loop counter */
+ loops = 0;
+ }
+ last_entry = cb_data->last_message_id_received;
+
/* If we sleep forever then we can conclude that something bad happened, and bail... */
/* Arbitrary 30 second delay : basically we should only expect to wait as long as it takes to process a few operations, which should be on the order of a second at most */
- if (!done && (loops > 300))
+ if (!done && (loops > 30))
{
/* Log a warning */
slapi_log_error(SLAPI_LOG_FATAL, NULL,
@@ -458,7 +467,9 @@ repl5_tot_run(Private_Repl_Protocol *prp)
if (!prp->repl50consumer)
{
- repl5_tot_waitfor_async_results(&cb_data);
+ if(cb_data.rc == LDAP_SUCCESS){ /* no need to wait if we already failed */
+ repl5_tot_waitfor_async_results(&cb_data);
+ }
rc = repl5_tot_destroy_async_result_thread(&cb_data);
if (rc) {
slapi_log_error (SLAPI_LOG_FATAL, repl_plugin_name, "%s: repl5_tot_run: "
@@ -480,15 +491,15 @@ repl5_tot_run(Private_Repl_Protocol *prp)
agmt_update_done(prp->agmt, 1);
release_replica(prp);
- if (rc != LDAP_SUCCESS)
- {
- slapi_log_error (SLAPI_LOG_REPL, repl_plugin_name, "%s: repl5_tot_run: "
- "failed to obtain data to send to the consumer; LDAP error - %d\n",
- agmt_get_long_name(prp->agmt), rc);
+ if (rc != LDAP_SUCCESS)
+ {
+ slapi_log_error (SLAPI_LOG_FATAL, repl_plugin_name, "Total update failed for replica \"%s\", "
+ "error (%d)\n", agmt_get_long_name(prp->agmt), rc);
agmt_set_last_init_status(prp->agmt, rc, 0, "Total update aborted");
- } else {
- slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "Finished total update of replica "
- "\"%s\". Sent %lu entries.\n", agmt_get_long_name(prp->agmt), cb_data.num_entries);
+ } else {
+ slapi_log_error (SLAPI_LOG_FATAL, repl_plugin_name, "Finished total update of replica "
+ "\"%s\". Sent %lu entries.\n",
+ agmt_get_long_name(prp->agmt), cb_data.num_entries);
agmt_set_last_init_status(prp->agmt, 0, 0, "Total update succeeded");
}
9 years, 6 months
- 1
- 0
ldap/servers
by Mark Reynolds
ldap/servers/plugins/replication/repl5_connection.c | 89 +++++++++++---------
ldap/servers/slapd/ldaputil.c | 39 ++++----
2 files changed, 69 insertions(+), 59 deletions(-)
New commits:
commit 9940ca29ca258891c52640a23adc2851afe59d0e
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Mon Mar 31 15:17:59 2014 -0400
Ticket 47759 - Crash in replication when server is under write load
Bug Description: When the server is under alot of load, a race condition allows
a replication connection LDAP struct to be freed(unbind) while
it is being used by another thread. This leads to a crash.
Fix Description: Extend the connection lock to also cover ldap client interaction
(e.g. conn->ld struct).
https://fedorahosted.org/389/ticket/47759
Reviewed by: nhosoi & rmeggins(Thanks!!)
diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c
index 0e49b94..3d29a79 100644
--- a/ldap/servers/plugins/replication/repl5_connection.c
+++ b/ldap/servers/plugins/replication/repl5_connection.c
@@ -142,6 +142,7 @@ static void repl5_debug_timeout_callback(time_t when, void *arg);
/* Forward declarations */
static void close_connection_internal(Repl_Connection *conn);
+static void conn_delete_internal(Repl_Connection *conn);
/*
* Create a new connection object. Returns a pointer to the object, or
@@ -186,11 +187,22 @@ conn_new(Repl_Agmt *agmt)
rpc->plain = NULL;
return rpc;
loser:
- conn_delete(rpc);
+ conn_delete_internal(rpc);
slapi_ch_free((void**)&rpc);
return NULL;
}
+static PRBool
+conn_connected_locked(Repl_Connection *conn, int locked)
+{
+ PRBool return_value;
+
+ if(!locked) PR_Lock(conn->lock);
+ return_value = STATE_CONNECTED == conn->state;
+ if(!locked) PR_Unlock(conn->lock);
+
+ return return_value;
+}
/*
* Return PR_TRUE if the connection is in the connected state
@@ -198,14 +210,9 @@ loser:
static PRBool
conn_connected(Repl_Connection *conn)
{
- PRBool return_value;
- PR_Lock(conn->lock);
- return_value = STATE_CONNECTED == conn->state;
- PR_Unlock(conn->lock);
- return return_value;
+ return conn_connected_locked(conn, 1);
}
-
/*
* Destroy a connection object.
*/
@@ -247,7 +254,6 @@ conn_delete(Repl_Connection *conn)
if (slapi_eq_cancel(conn->linger_event) == 1)
{
/* Event was found and cancelled. Destroy the connection object. */
- PR_Unlock(conn->lock);
destroy_it = PR_TRUE;
}
else
@@ -258,16 +264,15 @@ conn_delete(Repl_Connection *conn)
* off, so arrange for the event to destroy the object .
*/
conn->delete_after_linger = PR_TRUE;
- PR_Unlock(conn->lock);
}
}
if (destroy_it)
{
conn_delete_internal(conn);
}
+ PR_Unlock(conn->lock);
}
-
/*
* Return the last operation type processed by the connection
* object, and the LDAP error encountered.
@@ -331,17 +336,18 @@ conn_read_result_ex(Repl_Connection *conn, char **retoidp, struct berval **retda
while (!slapi_is_shutting_down())
{
/* we have to make sure the update sending thread does not
- attempt to call conn_disconnect while we are reading
+ attempt to close connection while we are reading
results - so lock the conn while we get the results */
PR_Lock(conn->lock);
+
if ((STATE_CONNECTED != conn->state) || !conn->ld) {
rc = -1;
return_value = CONN_NOT_CONNECTED;
PR_Unlock(conn->lock);
break;
}
-
rc = ldap_result(conn->ld, send_msgid, 1, &local_timeout, &res);
+
PR_Unlock(conn->lock);
if (0 != rc)
@@ -665,8 +671,10 @@ perform_operation(Repl_Connection *conn, int optype, const char *dn,
server_controls[1] = update_control;
server_controls[2] = NULL;
- /* lock the conn to prevent the result reader thread
- from closing the connection out from under us */
+ /*
+ * Lock the conn to prevent the result reader thread
+ * from closing the connection out from under us.
+ */
PR_Lock(conn->lock);
if (STATE_CONNECTED == conn->state)
{
@@ -808,7 +816,6 @@ conn_send_rename(Repl_Connection *conn, const char *dn,
NULL /* extop OID */, NULL /* extop payload */, message_id);
}
-
/*
* Send an LDAP extended operation.
*/
@@ -822,7 +829,6 @@ conn_send_extended_operation(Repl_Connection *conn, const char *extop_oid,
update_control, extop_oid, payload, message_id);
}
-
/*
* Synchronously read an entry and return a specific attribute's values.
* Returns CONN_OPERATION_SUCCESS if successful. Returns
@@ -842,6 +848,8 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
LDAPMessage *res = NULL;
char *attrs[2];
+ PR_Lock(conn->lock);
+
PR_ASSERT(NULL != type);
if (conn_connected(conn))
{
@@ -864,7 +872,7 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
}
else if (IS_DISCONNECT_ERROR(ldap_rc))
{
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -882,10 +890,11 @@ conn_read_entry_attribute(Repl_Connection *conn, const char *dn,
{
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
-
/*
* Return an pointer to a string describing the connection's status.
*/
@@ -896,8 +905,6 @@ conn_get_status(Repl_Connection *conn)
return conn->status;
}
-
-
/*
* Cancel any outstanding linger timer. Should be called when
* a replication session is beginning.
@@ -929,7 +936,6 @@ conn_cancel_linger(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
/*
* Called when our linger timeout timer expires. This means
* we should check to see if perhaps the connection's become
@@ -961,7 +967,6 @@ linger_timeout(time_t event_time, void *arg)
}
}
-
/*
* Indicate that a session is ending. The linger timer starts when
* this function is called.
@@ -999,8 +1004,6 @@ conn_start_linger(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
-
/*
* If no connection is currently active, opens a connection and binds to
* the remote server. If a connection is open (e.g. lingering) then
@@ -1019,10 +1022,14 @@ conn_connect(Repl_Connection *conn)
ConnResult return_value = CONN_OPERATION_SUCCESS;
int pw_ret = 1;
- /** Connection already open just return SUCCESS **/
- if(conn->state == STATE_CONNECTED) goto done;
-
PR_Lock(conn->lock);
+
+ /* Connection already open, just return SUCCESS */
+ if(conn->state == STATE_CONNECTED){
+ PR_Unlock(conn->lock);
+ return return_value;
+ }
+
if (conn->flag_agmt_changed) {
/* So far we cannot change Hostname and Port */
/* slapi_ch_free((void **)&conn->hostname); */
@@ -1037,7 +1044,6 @@ conn_connect(Repl_Connection *conn)
conn->port = agmt_get_port(conn->agmt); /* port could be updated */
slapi_ch_free((void **)&conn->plain);
}
- PR_Unlock(conn->lock);
creds = agmt_get_credentials(conn->agmt);
@@ -1178,6 +1184,7 @@ done:
{
close_connection_internal(conn);
}
+ PR_Unlock(conn->lock);
return return_value;
}
@@ -1213,7 +1220,6 @@ conn_disconnect(Repl_Connection *conn)
PR_Unlock(conn->lock);
}
-
/*
* Determine if the remote replica supports DS 5.0 replication.
* Return codes:
@@ -1230,6 +1236,7 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds50_repl == -1) {
@@ -1277,7 +1284,7 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1297,10 +1304,11 @@ conn_replica_supports_ds5_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
-
/*
* Determine if the remote replica supports DS 7.1 replication.
* Return codes:
@@ -1317,6 +1325,7 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds71_repl == -1) {
@@ -1348,7 +1357,7 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1368,6 +1377,8 @@ conn_replica_supports_ds71_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
@@ -1387,6 +1398,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
ConnResult return_value;
int ldap_rc;
+ PR_Lock(conn->lock);
if (conn_connected(conn))
{
if (conn->supports_ds90_repl == -1) {
@@ -1418,7 +1430,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
if (IS_DISCONNECT_ERROR(ldap_rc))
{
conn->last_ldap_error = ldap_rc; /* specific reason */
- conn_disconnect(conn);
+ close_connection_internal(conn);
return_value = CONN_NOT_CONNECTED;
}
else
@@ -1427,7 +1439,7 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
}
}
if (NULL != res)
- ldap_msgfree(res);
+ ldap_msgfree(res);
}
else
{
@@ -1439,6 +1451,8 @@ conn_replica_supports_ds90_repl(Repl_Connection *conn)
/* Not connected */
return_value = CONN_NOT_CONNECTED;
}
+ PR_Unlock(conn->lock);
+
return return_value;
}
@@ -1456,7 +1470,6 @@ conn_replica_is_readonly(Repl_Connection *conn)
}
}
-
/*
* Return 1 if "value" is a value of attribute type "type" in entry "entry".
* Otherwise, return 0.
@@ -1505,9 +1518,6 @@ attribute_string_value_present(LDAP *ld, LDAPMessage *entry, const char *type,
return return_value;
}
-
-
-
/*
* Read the remote server's schema entry, then read the local schema entry,
* and compare the nsschemacsn attribute. If the local csn is newer, or
@@ -1537,7 +1547,7 @@ conn_push_schema(Repl_Connection *conn, CSN **remotecsn)
return_value = CONN_OPERATION_FAILED;
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "NULL remote CSN\n");
}
- else if (!conn_connected(conn))
+ else if (!conn_connected_locked(conn, 0 /* not locked */))
{
return_value = CONN_NOT_CONNECTED;
slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
@@ -1754,6 +1764,7 @@ conn_push_schema(Repl_Connection *conn, CSN **remotecsn)
{
csn_free(&localcsn);
}
+
return return_value;
}
diff --git a/ldap/servers/slapd/ldaputil.c b/ldap/servers/slapd/ldaputil.c
index 6e559ee..7daaaf3 100644
--- a/ldap/servers/slapd/ldaputil.c
+++ b/ldap/servers/slapd/ldaputil.c
@@ -1044,8 +1044,8 @@ slapi_ldap_bind(
than the currently unused clientctrls */
ldap_get_option(ld, LDAP_OPT_CLIENT_CONTROLS, &clientctrls);
if (clientctrls && clientctrls[0] &&
- slapi_control_present(clientctrls, START_TLS_OID, NULL, NULL)) {
- secure = 2;
+ slapi_control_present(clientctrls, START_TLS_OID, NULL, NULL)) {
+ secure = 2;
} else {
#if defined(USE_OPENLDAP)
/* openldap doesn't have a SSL/TLS yes/no flag - so grab the
@@ -1084,12 +1084,12 @@ slapi_ldap_bind(
slapi_log_error(SLAPI_LOG_SHELL, "slapi_ldap_bind",
"Set up conn to use client auth\n");
}
- bvcreds.bv_val = NULL; /* ignore username and passed in creds */
- bvcreds.bv_len = 0; /* for external auth */
- bindid = NULL;
+ bvcreds.bv_val = NULL; /* ignore username and passed in creds */
+ bvcreds.bv_len = 0; /* for external auth */
+ bindid = NULL;
} else { /* other type of auth */
- bvcreds.bv_val = (char *)creds;
- bvcreds.bv_len = creds ? strlen(creds) : 0;
+ bvcreds.bv_val = (char *)creds;
+ bvcreds.bv_len = creds ? strlen(creds) : 0;
}
if (secure == 2) { /* send start tls */
@@ -1117,31 +1117,29 @@ slapi_ldap_bind(
bindid, creds);
if ((rc = ldap_sasl_bind(ld, bindid, mech, &bvcreds, serverctrls,
NULL /* clientctrls */, &mymsgid))) {
- char *myhostname = NULL;
- char *copy = NULL;
+ char *hostname = NULL;
+ char *host_port = NULL;
char *ptr = NULL;
int myerrno = errno;
int gaierr = 0;
- ldap_get_option(ld, LDAP_OPT_HOST_NAME, &myhostname);
- if (myhostname) {
- ptr = strchr(myhostname, ':');
+ ldap_get_option(ld, LDAP_OPT_HOST_NAME, &host_port);
+ if (host_port) {
+ ptr = strchr(host_port, ':');
if (ptr) {
- copy = slapi_ch_strdup(myhostname);
- *(copy + (ptr - myhostname)) = '\0';
- slapi_ch_free_string(&myhostname);
- myhostname = copy;
+ hostname = slapi_ch_strdup(host_port);
+ *(hostname + (ptr - host_port)) = '\0';
}
}
-
if (0 == myerrno) {
struct addrinfo *result = NULL;
- gaierr = getaddrinfo(myhostname, NULL, NULL, &result);
+ gaierr = getaddrinfo(hostname, NULL, NULL, &result);
myerrno = errno;
if (result) {
freeaddrinfo(result);
}
}
+
slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind",
"Error: could not send bind request for id "
"[%s] authentication mechanism [%s]: error %d (%s), system error %d (%s), "
@@ -1152,8 +1150,9 @@ slapi_ldap_bind(
PR_GetError(), slapd_pr_strerror(PR_GetError()),
myerrno ? myerrno : gaierr,
myerrno ? slapd_system_strerror(myerrno) : gai_strerror(gaierr),
- myhostname ? myhostname : "unknown host");
- slapi_ch_free_string(&myhostname);
+ host_port ? host_port : "unknown host");
+ slapi_ch_free_string(&hostname);
+ slapi_ch_free_string(&host_port);
goto done;
}
9 years, 6 months
- 1
- 0