389-commits March 2013

389-commits@lists.fedoraproject.org

5 participants
68 discussions

admserv/newinst
by Noriko Hosoi 27 Mar '13

27 Mar '13

admserv/newinst/src/AdminServer.pm.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) New commits: commit db76324dfe83b748ef28e85ad968f7bd3d972716 Author: noriko <nhosoi(a)localhost.localdomain> Date: Wed Mar 27 15:07:06 2013 -0700 Ticket #47298 - remove-ds-admin.pl does not stop the admin server Bug description: Keyword to be replaced for the build had a typo in the stopAdminServer subroutine. Fix description: Replacing "@with_systemdsystemunitdir@" with "@systemdsystemunitdir@". https://fedorahosted.org/389/ticket/47298 Reviewed by Mark (Thank you!!) diff --git a/admserv/newinst/src/AdminServer.pm.in b/admserv/newinst/src/AdminServer.pm.in index d769522..937df8a 100644 --- a/admserv/newinst/src/AdminServer.pm.in +++ b/admserv/newinst/src/AdminServer.pm.in @@ -605,10 +605,10 @@ sub reconfigAdminServer { sub stopAdminServer { my $prog = "@sbindir@/stop-ds-admin"; - if ("@with_systemdsystemunitdir@") { - $prog = "/bin/systemctl stop @package_name@.service"; - } elsif ("@enable_service@") { + if ("@enable_service@") { $prog = "service @package_name@ stop"; + } elsif ("@systemdsystemunitdir@") { + $prog = "/bin/systemctl stop @package_name@.service"; } elsif (! -x $prog) { debug(1, "stopping admin server: no such program $prog: cannot stop server\n"); return 0;

1 0

ldap/servers
by Mark Reynolds 27 Mar '13

27 Mar '13

ldap/servers/slapd/sasl_map.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) New commits: commit 2e5a30d6c9d6553f9e3aa8e3c1ae0c995d156e11 Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Wed Mar 27 17:32:21 2013 -0400 Ticket 534 - RFE: Add SASL mappings fallback Bug Description: The function for removing entries from the list of SASL mappings causes segmentation faults. Fix Description: Use correct double-linked list deletion routine in the function. https://fedorahosted.org/389/ticket/534 Reviewed by: mreynolds diff --git a/ldap/servers/slapd/sasl_map.c b/ldap/servers/slapd/sasl_map.c index 60bf4a7..9652c14 100644 --- a/ldap/servers/slapd/sasl_map.c +++ b/ldap/servers/slapd/sasl_map.c @@ -138,14 +138,16 @@ sasl_map_remove_list_entry(sasl_map_private *priv, char *removeme) prev = current->prev; if (prev) { /* Unlink it */ - if(next){ - next->prev = prev; + if (next) { + next->prev = prev; } prev->next = next; } else { /* That was the first list entry */ - priv->map_data_list = current->next; - priv->map_data_list->prev = NULL; + if (next) { + next->prev = NULL; + } + priv->map_data_list = next; } /* Payload free */ sasl_map_free_data(&current);

1 0

ldap/servers
by Mark Reynolds 27 Mar '13

27 Mar '13

ldap/servers/slapd/modify.c | 46 ++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) New commits: commit a4c4daaa54d1ba94e342fcbf9af13a547886f0e3 Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Wed Mar 27 17:23:10 2013 -0400 Ticket 620 - Better logging of error messages for 389-ds-base Bug Description: In a replication environment, if you have one master set with passwordIsGlobalpolicy to on, and another master does not, then the password policy updates are rejected. However, there is no clear logging to tell you why the operatoin was rejected. Fix Description: Write a clear message in the error log stating what was the cause of the failure. https://fedorahosted.org/389/ticket/620 Reviewed by: nhosoi & nkinder (Thanks!!) diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index d42ef2c..cc1621b 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -128,22 +128,22 @@ static struct attr_value_check { void do_modify( Slapi_PBlock *pb ) { - Slapi_Operation *operation; - BerElement *ber; - char *last, *type = NULL; - ber_tag_t tag; - ber_len_t len; - LDAPMod *mod; - LDAPMod **mods; - Slapi_Mods smods; - int err; - int pw_change = 0; /* 0= no password change */ - int ignored_some_mods = 0; - int has_password_mod = 0; /* number of password mods */ - char *old_pw = NULL; /* remember the old password */ - char *rawdn = NULL; - int minssf_exclude_rootdse = 0; - LDAPMod **normalized_mods = NULL; + Slapi_Operation *operation; + Slapi_Mods smods; + BerElement *ber; + ber_tag_t tag; + ber_len_t len; + LDAPMod **normalized_mods = NULL; + LDAPMod *mod; + LDAPMod **mods; + char *last, *type = NULL; + char *old_pw = NULL; /* remember the old password */ + char *rawdn = NULL; + int minssf_exclude_rootdse = 0; + int ignored_some_mods = 0; + int has_password_mod = 0; /* number of password mods */ + int pw_change = 0; /* 0 = no password change */ + int err; LDAPDebug( LDAP_DEBUG_TRACE, "do_modify\n", 0, 0, 0 ); @@ -291,8 +291,10 @@ do_modify( Slapi_PBlock *pb ) /* check if user is allowed to modify the specified attribute */ if (!op_shared_is_allowed_attr (mod->mod_type, pb->pb_conn->c_isreplication_session)) { - /* for now we just ignore attributes that client is not allowed - to modify so not to break existing clients */ + /* + * For now we just ignore attributes that client is not allowed + * to modify so not to break existing clients + */ ++ignored_some_mods; ber_bvecfree(mod->mod_bvalues); slapi_ch_free((void **)&(mod->mod_type)); @@ -310,6 +312,14 @@ do_modify( Slapi_PBlock *pb ) } if (ignored_some_mods && (0 == smods.num_elements)) { + if(pb->pb_conn->c_isreplication_session){ + int connid, opid; + slapi_pblock_get(pb, SLAPI_CONN_ID, &connid); + slapi_pblock_get(pb, SLAPI_OPERATION_ID, &opid); + LDAPDebug( LDAP_DEBUG_ANY,"Rejecting replicated password policy operation(conn=%d op=%d) for " + "entry %s. To allow these changes to be accepted, set passwordIsGlobalPolicy to 'on' in " + "cn=config.\n", connid, opid, rawdn); + } send_ldap_result( pb, LDAP_UNWILLING_TO_PERFORM, NULL, NULL, 0, NULL ); goto free_and_return; }

1 0

Branch '389-ds-base-1.3.0' - ldap/servers
by Noriko Hosoi 27 Mar '13

27 Mar '13

ldap/servers/slapd/entry.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) New commits: commit 08c2ea59165fb13cc5153e0ee44f5073180c8cf9 Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Mon Mar 25 10:35:31 2013 -0400 Ticket 628 - crash in aci evaluation Bug Description: When trying to get effective rights on a entry that does not exist, can lead to a crash from dereferencing a NULL pointer. Fix Description: Check for NULL entry pointers and return the appropriate error. https://fedorahosted.org/389/ticket/628 Reviewed by: nhosoi(Thanks Noriko!) (cherry picked from commit ac8ceb91e42c31672147018d9d654879970dd38d) diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c index 19d77a9..c5780e4 100644 --- a/ldap/servers/slapd/entry.c +++ b/ldap/servers/slapd/entry.c @@ -2343,7 +2343,11 @@ slapi_entry_next_attr( const Slapi_Entry *e, Slapi_Attr *prevattr, Slapi_Attr ** int slapi_entry_attr_find( const Slapi_Entry *e, const char *type, Slapi_Attr **a ) { - int r= -1; + int r = -1; + + if(e == NULL){ + return r; + } *a = attrlist_find( e->e_attrs, type ); if (*a != NULL) { @@ -2354,11 +2358,11 @@ slapi_entry_attr_find( const Slapi_Entry *e, const char *type, Slapi_Attr **a ) * Our state information storage scheme can cause this, since * we have to hang onto the deleted value state information. */ - *a= NULL; + *a = NULL; } else { - r= 0; + r = 0; } } return r; @@ -2985,18 +2989,20 @@ slapi_entry_attr_has_syntax_value(const Slapi_Entry *e, const char *type, const Slapi_Value *value) { - int r= 0; - Slapi_Attr *attr; + int r = 0; + Slapi_Attr *attr; + if(e == NULL){ + return r; + } if(slapi_entry_attr_find(e, type, &attr)==0) { - const struct berval *bv = slapi_value_get_berval(value); + const struct berval *bv = slapi_value_get_berval(value); - if ( bv != NULL) { - r = (slapi_attr_value_find(attr, bv) == 0); - } - - } + if ( bv != NULL) { + r = (slapi_attr_value_find(attr, bv) == 0); + } + } return r; }

1 0

Branch '389-ds-base-1.2.11' - ldap/servers
by Noriko Hosoi 27 Mar '13

27 Mar '13

ldap/servers/slapd/entry.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) New commits: commit 0e2f2c028feeb9b865b7a1064e40f9d5aadfa412 Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Mon Mar 25 10:35:31 2013 -0400 Ticket 628 - crash in aci evaluation Bug Description: When trying to get effective rights on a entry that does not exist, can lead to a crash from dereferencing a NULL pointer. Fix Description: Check for NULL entry pointers and return the appropriate error. https://fedorahosted.org/389/ticket/628 Reviewed by: nhosoi(Thanks Noriko!) (cherry picked from commit ac8ceb91e42c31672147018d9d654879970dd38d) diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c index 58a9d51..1926417 100644 --- a/ldap/servers/slapd/entry.c +++ b/ldap/servers/slapd/entry.c @@ -2312,7 +2312,11 @@ slapi_entry_next_attr( const Slapi_Entry *e, Slapi_Attr *prevattr, Slapi_Attr ** int slapi_entry_attr_find( const Slapi_Entry *e, const char *type, Slapi_Attr **a ) { - int r= -1; + int r = -1; + + if(e == NULL){ + return r; + } *a = attrlist_find( e->e_attrs, type ); if (*a != NULL) { @@ -2323,11 +2327,11 @@ slapi_entry_attr_find( const Slapi_Entry *e, const char *type, Slapi_Attr **a ) * Our state information storage scheme can cause this, since * we have to hang onto the deleted value state information. */ - *a= NULL; + *a = NULL; } else { - r= 0; + r = 0; } } return r; @@ -2951,18 +2955,20 @@ slapi_entry_attr_has_syntax_value(const Slapi_Entry *e, const char *type, const Slapi_Value *value) { - int r= 0; - Slapi_Attr *attr; + int r = 0; + Slapi_Attr *attr; + if(e == NULL){ + return r; + } if(slapi_entry_attr_find(e, type, &attr)==0) { - const struct berval *bv = slapi_value_get_berval(value); + const struct berval *bv = slapi_value_get_berval(value); - if ( bv != NULL) { - r = (slapi_attr_value_find(attr, bv) == 0); - } - - } + if ( bv != NULL) { + r = (slapi_attr_value_find(attr, bv) == 0); + } + } return r; }

1 0

ldap/servers
by Mark Reynolds 27 Mar '13

27 Mar '13

ldap/servers/slapd/entry.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) New commits: commit ac8ceb91e42c31672147018d9d654879970dd38d Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Mon Mar 25 10:35:31 2013 -0400 Ticket 628 - crash in aci evaluation Bug Description: When trying to get effective rights on a entry that does not exist, can lead to a crash from dereferencing a NULL pointer. Fix Description: Check for NULL entry pointers and return the appropriate error. https://fedorahosted.org/389/ticket/628 Reviewed by: nhosoi(Thanks Noriko!) diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c index 17af1bf..d7dbf61 100644 --- a/ldap/servers/slapd/entry.c +++ b/ldap/servers/slapd/entry.c @@ -2363,7 +2363,11 @@ slapi_entry_next_attr( const Slapi_Entry *e, Slapi_Attr *prevattr, Slapi_Attr ** int slapi_entry_attr_find( const Slapi_Entry *e, const char *type, Slapi_Attr **a ) { - int r= -1; + int r = -1; + + if(e == NULL){ + return r; + } *a = attrlist_find( e->e_attrs, type ); if (*a != NULL) { @@ -2374,11 +2378,11 @@ slapi_entry_attr_find( const Slapi_Entry *e, const char *type, Slapi_Attr **a ) * Our state information storage scheme can cause this, since * we have to hang onto the deleted value state information. */ - *a= NULL; + *a = NULL; } else { - r= 0; + r = 0; } } return r; @@ -3026,18 +3030,20 @@ slapi_entry_attr_has_syntax_value(const Slapi_Entry *e, const char *type, const Slapi_Value *value) { - int r= 0; - Slapi_Attr *attr; + int r = 0; + Slapi_Attr *attr; + if(e == NULL){ + return r; + } if(slapi_entry_attr_find(e, type, &attr)==0) { - const struct berval *bv = slapi_value_get_berval(value); + const struct berval *bv = slapi_value_get_berval(value); - if ( bv != NULL) { - r = (slapi_attr_value_find(attr, bv) == 0); - } - - } + if ( bv != NULL) { + r = (slapi_attr_value_find(attr, bv) == 0); + } + } return r; }

1 0

ldap/servers
by Mark Reynolds 27 Mar '13

27 Mar '13

ldap/servers/slapd/pw.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) New commits: commit ab46f87224706a7023596b4261cbcc66b5bc037b Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Wed Mar 27 11:05:22 2013 -0400 Ticket 458 - Need to properly check if the password admin dn is set Bug Description: We were previously checking if password admin sdn was NULL, which it never is. So it would always search the database looking for the password admins. Fix Description: Call slapi_sdn_get_dn() to see if the DN was actually set. https://fedorahosted.org/389/ticket/458 Reviewed by: richm(Thanks Rich!) diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index b31c2a8..3bb87d7 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -1535,6 +1535,7 @@ pw_get_admin_users(passwdPolicy *pwp) const Slapi_DN *sdn = pwp->pw_admin; char **uniquemember_vals = NULL; char **member_vals = NULL; + char *binddn = slapi_sdn_get_dn(sdn); int uniquemember_count = 0; int member_count = 0; int nentries = 0; @@ -1542,13 +1543,13 @@ pw_get_admin_users(passwdPolicy *pwp) int res; int i; - if(sdn == NULL){ + if(binddn == NULL){ return; } /* * Check if the DN exists and has "group" objectclasses */ - slapi_search_internal_set_pb(pb, slapi_sdn_get_dn(sdn), LDAP_SCOPE_BASE,"(|(objectclass=groupofuniquenames)(objectclass=groupofnames))", + slapi_search_internal_set_pb(pb, binddn, LDAP_SCOPE_BASE,"(|(objectclass=groupofuniquenames)(objectclass=groupofnames))", NULL, 0, NULL, NULL, (void *) plugin_get_default_component_id(), 0); slapi_search_internal_pb(pb); slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &res);

1 0

ldap/servers
by Mark Reynolds 26 Mar '13

26 Mar '13

ldap/servers/plugins/dna/dna.c | 4 ++-- ldap/servers/plugins/replication/repl5_connection.c | 4 ++-- ldap/servers/plugins/replication/repl5_inc_protocol.c | 5 +++-- ldap/servers/plugins/replication/repl5_protocol_util.c | 4 ++-- ldap/servers/plugins/replication/windows_inc_protocol.c | 5 +++-- ldap/servers/slapd/entry.c | 5 +++-- ldap/servers/slapd/errormap.c | 8 ++++++-- ldap/servers/slapd/ldaputil.c | 15 ++++++++------- 8 files changed, 29 insertions(+), 21 deletions(-) New commits: commit 4c4cb3b999772534b6f32c219c362489e3868fce Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Mon Mar 25 14:06:23 2013 -0400 Ticket 587 - Replication error messages in the DS error logs Bug Description: Replicand DNA error message are seen as vague. Fix Description: Moved some DNA plugin errors messages to log as SLAPI_LOG_PLUGIN, instead of SLAPI_LOG_FATAL. Made some replication errors more human readable. Also, improved NSPR error str function to check for zero result and not report "unknown" error. https://fedorahosted.org/389/ticket/587 Reviewed by: nhosoi(Thanks Noriko!) diff --git a/ldap/servers/plugins/dna/dna.c b/ldap/servers/plugins/dna/dna.c index affdadf..e11dd99 100644 --- a/ldap/servers/plugins/dna/dna.c +++ b/ldap/servers/plugins/dna/dna.c @@ -1632,7 +1632,7 @@ static int dna_request_range(struct configEntry *config_entry, NULL, NULL, NULL, NULL); if (ret != LDAP_SUCCESS) { - slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM, + slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "dna_request_range: Error binding " " to replica server %s:%u. [error %d]\n", server->host, server->port, ret); @@ -2934,7 +2934,7 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr) * really a soft failure. */ if ((config_entry->next_range_lower == 0) && (config_entry->remaining <= config_entry->threshold)) { - slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM, + slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "dna_pre_op: Passed threshold of %" NSPRIu64 " remaining values " "for range %s. (%" NSPRIu64 " values remain)\n", diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c index 76c77c9..b8b542e 100644 --- a/ldap/servers/plugins/replication/repl5_connection.c +++ b/ldap/servers/plugins/replication/repl5_connection.c @@ -1842,7 +1842,7 @@ bind_and_check_pwp(Repl_Connection *conn, char * binddn, char *password) "%s: Replication bind with %s auth failed: LDAP error %d (%s) (%s)\n", agmt_get_long_name(conn->agmt), mech ? mech : "SIMPLE", rc, - ldap_err2string(rc), errmsg); + ldap_err2string(rc), errmsg ? errmsg : ""); } else { char *errmsg = NULL; /* errmsg is a pointer directly into the ld structure - do not free */ @@ -1851,7 +1851,7 @@ bind_and_check_pwp(Repl_Connection *conn, char * binddn, char *password) "%s: Replication bind with %s auth failed: LDAP error %d (%s) (%s)\n", agmt_get_long_name(conn->agmt), mech ? mech : "SIMPLE", rc, - ldap_err2string(rc), errmsg); + ldap_err2string(rc), errmsg ? errmsg : ""); } return (CONN_OPERATION_FAILED); diff --git a/ldap/servers/plugins/replication/repl5_inc_protocol.c b/ldap/servers/plugins/replication/repl5_inc_protocol.c index c3d027b..222bb97 100644 --- a/ldap/servers/plugins/replication/repl5_inc_protocol.c +++ b/ldap/servers/plugins/replication/repl5_inc_protocol.c @@ -1005,8 +1005,9 @@ repl5_inc_run(Private_Repl_Protocol *prp) break; case EXAMINE_RUV_GENERATION_MISMATCH: slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, - "%s: Replica has a different generation ID than the local data.\n", - agmt_get_long_name(prp->agmt)); + "%s: The remote replica has a different database generation ID than " + "the local database. You may have to reinitialize the remote replica, " + "or the local replica.\n", agmt_get_long_name(prp->agmt)); next_state = STATE_BACKOFF_START; break; case EXAMINE_RUV_REPLICA_TOO_OLD: diff --git a/ldap/servers/plugins/replication/repl5_protocol_util.c b/ldap/servers/plugins/replication/repl5_protocol_util.c index 30d211a..56f8532 100644 --- a/ldap/servers/plugins/replication/repl5_protocol_util.c +++ b/ldap/servers/plugins/replication/repl5_protocol_util.c @@ -590,8 +590,8 @@ release_replica(Private_Repl_Protocol *prp) int operation, error; conn_get_error(prp->conn, &operation, &error); slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, - "%s: Warning: unable to receive endReplication extended operation response (%s)\n", - agmt_get_long_name(prp->agmt), + "%s: Warning: Attempting to release replica, but unable to receive endReplication extended " + "operation response from the replica. Error %d (%s)\n", agmt_get_long_name(prp->agmt), error, error ? ldap_err2string(error) : "unknown error"); } else diff --git a/ldap/servers/plugins/replication/windows_inc_protocol.c b/ldap/servers/plugins/replication/windows_inc_protocol.c index 38d9011..cf979cc 100644 --- a/ldap/servers/plugins/replication/windows_inc_protocol.c +++ b/ldap/servers/plugins/replication/windows_inc_protocol.c @@ -797,8 +797,9 @@ windows_inc_run(Private_Repl_Protocol *prp) break; case EXAMINE_RUV_GENERATION_MISMATCH: slapi_log_error(SLAPI_LOG_FATAL, windows_repl_plugin_name, - "%s: Replica has a different generation ID than the local data.\n", - agmt_get_long_name(prp->agmt)); + "%s: The remote replica has a different database generation ID than " + "the local database. You may have to reinitialize the remote replica, " + "or the local replica.\n", agmt_get_long_name(prp->agmt)); next_state = STATE_BACKOFF_START; break; case EXAMINE_RUV_REPLICA_TOO_OLD: diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c index ebd49de..17af1bf 100644 --- a/ldap/servers/slapd/entry.c +++ b/ldap/servers/slapd/entry.c @@ -801,8 +801,9 @@ str2entry_dupcheck( const char *rawdn, char *s, int flags, int read_stateinfo ) bvtype = bv_null; bvvalue = bv_null; if ( slapi_ldif_parse_line( s, &bvtype, &bvvalue, &freeval ) < 0 ) { - LDAPDebug1Arg(LDAP_DEBUG_ANY, - "Warning: ignoring invalid line \"%s\"...\n", s); + LDAPDebug(LDAP_DEBUG_ANY, + "Warning: Entry (%s), ignoring invalid line \"%s\"...\n", + rawdn ? rawdn : "", s, 0); continue; } type = bvtype.bv_val; diff --git a/ldap/servers/slapd/errormap.c b/ldap/servers/slapd/errormap.c index c585d25..d685fbd 100644 --- a/ldap/servers/slapd/errormap.c +++ b/ldap/servers/slapd/errormap.c @@ -66,8 +66,12 @@ slapd_pr_strerror( const int prerrno ) { char *s; - if (( s = (char *)SECU_Strerror( (PRErrorCode)prerrno )) == NULL ) { - s = "unknown"; + if(prerrno == 0){ + s = "no error"; + } else { + if (( s = (char *)SECU_Strerror( (PRErrorCode)prerrno )) == NULL ) { + s = "unknown error"; + } } return( s ); diff --git a/ldap/servers/slapd/ldaputil.c b/ldap/servers/slapd/ldaputil.c index 1c67e51..3ca1139 100644 --- a/ldap/servers/slapd/ldaputil.c +++ b/ldap/servers/slapd/ldaputil.c @@ -1088,7 +1088,7 @@ slapi_ldap_bind( char *copy = NULL; char *ptr = NULL; int myerrno = errno; - int gaierr; + int gaierr = 0; ldap_get_option(ld, LDAP_OPT_HOST_NAME, &myhostname); if (myhostname) { @@ -1110,7 +1110,8 @@ slapi_ldap_bind( } slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind", "Error: could not send bind request for id " - "[%s] mech [%s]: error %d (%s) %d (%s) %d (%s \"%s\")\n", + "[%s] authentication mechanism [%s]: error %d (%s), system error %d (%s), " + "network error %d (%s, host \"%s\")\n", bindid ? bindid : "(anon)", mech ? mech : "SIMPLE", rc, ldap_err2string(rc), @@ -1130,7 +1131,7 @@ slapi_ldap_bind( rc = slapi_ldap_get_lderrno(ld, NULL, NULL); slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind", "Error reading bind response for id " - "[%s] mech [%s]: error %d (%s) errno %d (%s)\n", + "[%s] authentication mechanism [%s]: error %d (%s) errno %d (%s)\n", bindid ? bindid : "(anon)", mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errno, slapd_system_strerror(errno)); @@ -1139,7 +1140,7 @@ slapi_ldap_bind( rc = LDAP_TIMEOUT; slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind", "Error: timeout after [%ld.%ld] seconds reading " - "bind response for [%s] mech [%s]\n", + "bind response for [%s] authentication mechanism [%s]\n", timeout ? timeout->tv_sec : 0, timeout ? timeout->tv_usec : 0, bindid ? bindid : "(anon)", @@ -1161,7 +1162,7 @@ slapi_ldap_bind( rc = err; slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind", "Error: could not bind id " - "[%s] mech [%s]: error %d (%s) errno %d (%s)\n", + "[%s] authentication mechanism [%s]: error %d (%s) errno %d (%s)\n", bindid ? bindid : "(anon)", mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errno, slapd_system_strerror(errno)); @@ -1174,7 +1175,7 @@ slapi_ldap_bind( rc = slapi_ldap_get_lderrno(ld, NULL, NULL); slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind", "Error: could not read bind results for id " - "[%s] mech [%s]: error %d (%s) errno %d (%s)\n", + "[%s] authentication mechanism [%s]: error %d (%s) errno %d (%s)\n", bindid ? bindid : "(anon)", mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errno, slapd_system_strerror(errno)); @@ -1196,7 +1197,7 @@ slapi_ldap_bind( if (LDAP_SUCCESS != rc) { slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind", "Error: could not perform interactive bind for id " - "[%s] mech [%s]: error %d (%s)\n", + "[%s] authentication mechanism [%s]: error %d (%s)\n", bindid ? bindid : "(anon)", mech, /* mech cannot be SIMPLE here */ rc, ldap_err2string(rc));

1 0

Branch '389-ds-base-1.2.11' - ldap/servers
by Noriko Hosoi 25 Mar '13

25 Mar '13

ldap/servers/slapd/attrsyntax.c | 116 +++++++++++++++++++++++++++++----------- ldap/servers/slapd/proto-slap.h | 3 + ldap/servers/slapd/schema.c | 48 +++++++++------- 3 files changed, 117 insertions(+), 50 deletions(-) New commits: commit c82987275bec0e1eb1a6c9b853fcaf389a300e71 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Mon Mar 25 14:37:18 2013 -0700 Ticket #627 - ns-slapd crashes sporadically with segmentation fault in libslapd.so Bug Description: Schema reload task (schema-reload.pl) was not thread safe. Fix Description: Attribute Syntax is stored in the hash and retrieved based upon the attribute syntax. When Schema reload task is invoked, the attribute syntax objects were completely replaced ignoring the lock protection. This patch protects the attribute syntax replacement (attr_syntax_delete_all_for_ schemareload) with the write lock. Also, attribute syntax object maintains the reference count. The schema reload respects the reference count instead of blindly deleting them. https://fedorahosted.org/389/ticket/627 Reviewed by Rich (Thank you!!) (cherry picked from commit 81b997480956b2b6fa3a5d0e8d6abf5113a06400) diff --git a/ldap/servers/slapd/attrsyntax.c b/ldap/servers/slapd/attrsyntax.c index 61503f3..cc2f226 100644 --- a/ldap/servers/slapd/attrsyntax.c +++ b/ldap/servers/slapd/attrsyntax.c @@ -96,12 +96,27 @@ attr_syntax_read_lock(void) } void +attr_syntax_write_lock(void) +{ + if (0 != attr_syntax_init()) return; + + AS_LOCK_WRITE(oid2asi_lock); + AS_LOCK_WRITE(name2asi_lock); +} + +void attr_syntax_unlock_read(void) { - if(name2asi_lock) AS_UNLOCK_READ(name2asi_lock); - if(oid2asi_lock) AS_UNLOCK_READ(oid2asi_lock); + AS_UNLOCK_READ(name2asi_lock); + AS_UNLOCK_READ(oid2asi_lock); } +void +attr_syntax_unlock_write(void) +{ + AS_UNLOCK_WRITE(name2asi_lock); + AS_UNLOCK_WRITE(oid2asi_lock); +} #if 0 @@ -255,13 +270,17 @@ attr_syntax_get_by_oid_locking_optional( const char *oid, PRBool use_lock ) struct asyntaxinfo *asi = 0; if (oid2asi) { - if ( use_lock ) AS_LOCK_READ(oid2asi_lock); + if ( use_lock ) { + AS_LOCK_READ(oid2asi_lock); + } asi = (struct asyntaxinfo *)PL_HashTableLookup_const(oid2asi, oid); if (asi) { PR_AtomicIncrement( &asi->asi_refcnt ); } - if ( use_lock ) AS_UNLOCK_READ(oid2asi_lock); + if ( use_lock ) { + AS_UNLOCK_READ(oid2asi_lock); + } } return asi; @@ -279,13 +298,15 @@ attr_syntax_add_by_oid(const char *oid, struct asyntaxinfo *a, int lock) { if (0 != attr_syntax_init()) return; - if (lock) + if (lock) { AS_LOCK_WRITE(oid2asi_lock); + } PL_HashTableAdd(oid2asi, oid, a); - if (lock) + if (lock) { AS_UNLOCK_WRITE(oid2asi_lock); + } } /* @@ -326,12 +347,16 @@ attr_syntax_get_by_name_locking_optional(const char *name, PRBool use_lock) struct asyntaxinfo *asi = 0; if (name2asi) { - if ( use_lock ) AS_LOCK_READ(name2asi_lock); + if ( use_lock ) { + AS_LOCK_READ(name2asi_lock); + } asi = (struct asyntaxinfo *)PL_HashTableLookup_const(name2asi, name); if ( NULL != asi ) { PR_AtomicIncrement( &asi->asi_refcnt ); } - if ( use_lock ) AS_UNLOCK_READ(name2asi_lock); + if ( use_lock ) { + AS_UNLOCK_READ(name2asi_lock); + } } if (!asi) /* given name may be an OID */ asi = attr_syntax_get_by_oid_locking_optional(name, use_lock); @@ -353,30 +378,38 @@ attr_syntax_return( struct asyntaxinfo *asi ) } void -attr_syntax_return_locking_optional( struct asyntaxinfo *asi, PRBool use_lock ) +attr_syntax_return_locking_optional(struct asyntaxinfo *asi, PRBool use_lock) { + int locked = 0; + if(use_lock) { + AS_LOCK_READ(name2asi_lock); + locked = 1; + } if ( NULL != asi ) { - if ( 0 == PR_AtomicDecrement( &asi->asi_refcnt )) - { - PRBool delete_it; - - if(use_lock) AS_LOCK_READ(name2asi_lock); + PRBool delete_it = PR_FALSE; + if ( 0 == PR_AtomicDecrement( &asi->asi_refcnt )) { delete_it = asi->asi_marked_for_delete; - if(use_lock) AS_UNLOCK_READ(name2asi_lock); - - if ( delete_it ) - { - AS_LOCK_WRITE(name2asi_lock); /* get a write lock */ - if ( asi->asi_marked_for_delete ) /* one final check */ - { - /* ref count is 0 and it's flagged for - * deletion, so it's safe to free now */ - attr_syntax_free(asi); + } + + if (delete_it) { + if ( asi->asi_marked_for_delete ) { /* one final check */ + if(use_lock) { + AS_UNLOCK_READ(name2asi_lock); + AS_LOCK_WRITE(name2asi_lock); + } + /* ref count is 0 and it's flagged for + * deletion, so it's safe to free now */ + attr_syntax_free(asi); + if(use_lock) { + AS_UNLOCK_WRITE(name2asi_lock); + locked = 0; } - AS_UNLOCK_WRITE(name2asi_lock); } } } + if(locked) { + AS_UNLOCK_READ(name2asi_lock); + } } /* @@ -393,8 +426,9 @@ attr_syntax_add_by_name(struct asyntaxinfo *a, int lock) { if (0 != attr_syntax_init()) return; - if (lock) + if (lock) { AS_LOCK_WRITE(name2asi_lock); + } PL_HashTableAdd(name2asi, a->asi_name, a); if ( a->asi_aliases != NULL ) { @@ -405,8 +439,9 @@ attr_syntax_add_by_name(struct asyntaxinfo *a, int lock) } } - if (lock) + if (lock) { AS_UNLOCK_WRITE(name2asi_lock); + } } @@ -952,11 +987,11 @@ attr_syntax_enumerate_attrs(AttrEnumFunc aef, void *arg, PRBool writelock ) attr_syntax_enumerate_attrs_ext(oid2asi, aef, arg); if ( writelock ) { - AS_UNLOCK_WRITE(oid2asi_lock); AS_UNLOCK_WRITE(name2asi_lock); + AS_UNLOCK_WRITE(oid2asi_lock); } else { - AS_UNLOCK_READ(oid2asi_lock); AS_UNLOCK_READ(name2asi_lock); + AS_UNLOCK_READ(oid2asi_lock); } } @@ -1054,6 +1089,21 @@ attr_syntax_delete_all() (void *)&fi, PR_TRUE ); } +/* + * Delete all attribute definitions without attr_syntax lock. + * The caller is responsible for the lock. + */ +void +attr_syntax_delete_all_for_schemareload(unsigned long flag) +{ + struct attr_syntax_enum_flaginfo fi; + + memset(&fi, 0, sizeof(fi)); + fi.asef_flag = flag; + attr_syntax_enumerate_attrs_ext(oid2asi, attr_syntax_delete_if_not_flagged, + (void *)&fi); +} + static int attr_syntax_init(void) { @@ -1174,13 +1224,19 @@ static int attr_syntax_internal_asi_add(struct asyntaxinfo *asip, void *arg) { struct asyntaxinfo *asip_copy; + int rc = 0; + if (!asip) { return 1; } /* Copy is needed since when reloading the schema, * existing syntax info is cleaned up. */ asip_copy = attr_syntax_dup(asip); - return attr_syntax_add(asip_copy); + rc = attr_syntax_add(asip_copy); + if (LDAP_SUCCESS != rc) { + attr_syntax_free(asip_copy); + } + return rc; } /* Reload internal attribute syntax stashed in the internalasi hashtable. */ diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index f1f5b25..0b76b63 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -113,7 +113,9 @@ int attrlist_replace_with_flags(Slapi_Attr **alist, const char *type, struct ber * attrsyntax.c */ void attr_syntax_read_lock(void); +void attr_syntax_write_lock(void); void attr_syntax_unlock_read(void); +void attr_syntax_unlock_write(void); int attr_syntax_exists (const char *attr_name); void attr_syntax_delete ( struct asyntaxinfo *asip ); #define SLAPI_SYNTAXLENGTH_NONE (-1) /* for syntaxlength parameter */ @@ -140,6 +142,7 @@ struct asyntaxinfo *attr_syntax_get_by_name_with_default ( const char *name ); void attr_syntax_return( struct asyntaxinfo *asi ); void attr_syntax_return_locking_optional( struct asyntaxinfo *asi, PRBool use_lock ); void attr_syntax_delete_all(void); +void attr_syntax_delete_all_for_schemareload(unsigned long flag); /* * value.c diff --git a/ldap/servers/slapd/schema.c b/ldap/servers/slapd/schema.c index 12f11c3..1768c7e 100644 --- a/ldap/servers/slapd/schema.c +++ b/ldap/servers/slapd/schema.c @@ -1406,7 +1406,7 @@ slapi_schema_list_attribute_names(unsigned long flag) aew.flag=flag; attr_syntax_enumerate_attrs(schema_list_attributes_callback, &aew, - PR_FALSE); + PR_FALSE); return aew.attrs; } @@ -2409,8 +2409,9 @@ static int schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, size_t errorbufsize ) { - int i, rc = LDAP_SUCCESS; - struct asyntaxinfo *newasip, *oldasip; + int i, rc = LDAP_SUCCESS; + struct asyntaxinfo *newasip, *oldasip; + PRUint32 schema_flags = 0; if ( NULL == mod->mod_bvalues ) { schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_at, @@ -2418,8 +2419,11 @@ schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, return LDAP_UNWILLING_TO_PERFORM; } - /* clear all of the "keep" flags */ - attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + slapi_pblock_get(pb, SLAPI_SCHEMA_FLAGS, &schema_flags); + if (!(schema_flags & (DSE_SCHEMA_NO_LOAD|DSE_SCHEMA_NO_CHECK))) { + /* clear all of the "keep" flags unless it's from schema-reload */ + attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + } for ( i = 0; mod->mod_bvalues[i] != NULL; ++i ) { if ( LDAP_SUCCESS != ( rc = read_at_ldif( mod->mod_bvalues[i]->bv_val, @@ -2477,12 +2481,14 @@ schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, * XXXmcs: we should consider reporting an error if any read only types * remain.... */ - attr_syntax_delete_all_not_flagged( SLAPI_ATTR_FLAG_KEEP - | SLAPI_ATTR_FLAG_STD_ATTR ); + attr_syntax_delete_all_not_flagged( SLAPI_ATTR_FLAG_KEEP | + SLAPI_ATTR_FLAG_STD_ATTR ); clean_up_and_return: - /* clear all of the "keep" flags */ - attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + if (!(schema_flags & (DSE_SCHEMA_NO_LOAD|DSE_SCHEMA_NO_CHECK))) { + /* clear all of the "keep" flags unless it's from schema-reload */ + attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + } return rc; } @@ -3898,14 +3904,12 @@ load_schema_dse(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *ignored, int primary_file = 0; /* this is the primary (writeable) schema file */ int schema_ds4x_compat = config_get_ds4_compatible_schema(); PRUint32 flags = *(PRUint32 *)arg; - flags |= DSE_SCHEMA_NO_GLOCK; /* don't lock global resources - during initialization */ *returncode = 0; /* * Note: there is no need to call schema_lock_write() here because this - * function is only called during server startup. + * function is only called during server startup. */ slapi_pblock_get( pb, SLAPI_DSE_IS_PRIMARY_FILE, &primary_file ); @@ -3947,6 +3951,8 @@ load_schema_dse(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *ignored, if (*returncode) return SLAPI_DSE_CALLBACK_ERROR; + flags |= DSE_SCHEMA_NO_GLOCK; /* don't lock global resources + during initialization */ if (!slapi_entry_attr_find(e, "objectclasses", &attr) && attr) { /* enumerate the values in attr */ @@ -4017,7 +4023,6 @@ load_schema_dse(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *ignored, * DSE_SCHEMA_NO_CHECK -- schema won't be checked * DSE_SCHEMA_NO_BACKEND -- don't add as backend * DSE_SCHEMA_LOCKED -- already locked; no further lock needed - */ static int init_schema_dse_ext(char *schemadir, Slapi_Backend *be, @@ -4123,7 +4128,7 @@ init_schema_dse_ext(char *schemadir, Slapi_Backend *be, "DESC 'Standard schema for LDAP' SYNTAX " "1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'RFC 2252' )", NULL, errorbuf, SLAPI_DSE_RETURNTEXT_SIZE, - DSE_SCHEMA_NO_GLOCK|schema_flags, 0, 0, 0); + schema_flags, 0, 0, 0); } if (rc) { @@ -4196,7 +4201,7 @@ init_schema_dse(const char *configdir) { schemadir = slapi_ch_smprintf("%s/%s", configdir, SCHEMA_SUBDIR_NAME); } - rc = init_schema_dse_ext(schemadir, NULL, &pschemadse, 0); + rc = init_schema_dse_ext(schemadir, NULL, &pschemadse, DSE_SCHEMA_NO_GLOCK); slapi_ch_free_string(&schemadir); return rc; } @@ -4860,14 +4865,14 @@ slapi_validate_schema_files(char *schemadir) { struct dse *my_pschemadse = NULL; int rc = init_schema_dse_ext(schemadir, NULL, &my_pschemadse, - DSE_SCHEMA_NO_LOAD | DSE_SCHEMA_NO_BACKEND); + DSE_SCHEMA_NO_LOAD | DSE_SCHEMA_NO_BACKEND); dse_destroy(my_pschemadse); /* my_pschemadse was created just to - validate the schema */ + validate the schema */ if (rc) { return LDAP_SUCCESS; } else { slapi_log_error( SLAPI_LOG_FATAL, "schema_reload", - "schema file validation failed\n" ); + "schema file validation failed\n" ); return LDAP_OBJECT_CLASS_VIOLATION; } } @@ -4893,10 +4898,13 @@ slapi_reload_schema_files(char *schemadir) } slapi_be_Wlock(be); /* be lock must be outer of schemafile lock */ reload_schemafile_lock(); - attr_syntax_delete_all(); + /* Exclude attr_syntax not to grab from the hash table while cleaning up */ + attr_syntax_write_lock(); + attr_syntax_delete_all_for_schemareload(SLAPI_ATTR_FLAG_KEEP); oc_delete_all_nolock(); + attr_syntax_unlock_write(); rc = init_schema_dse_ext(schemadir, be, &my_pschemadse, - DSE_SCHEMA_NO_CHECK | DSE_SCHEMA_LOCKED); + DSE_SCHEMA_NO_CHECK | DSE_SCHEMA_LOCKED); if (rc) { dse_destroy(pschemadse); pschemadse = my_pschemadse;

1 0

Branch '389-ds-base-1.3.0' - ldap/servers
by Noriko Hosoi 25 Mar '13

25 Mar '13

ldap/servers/slapd/attrsyntax.c | 116 +++++++++++++++++++++++++++++----------- ldap/servers/slapd/proto-slap.h | 3 + ldap/servers/slapd/schema.c | 48 +++++++++------- 3 files changed, 117 insertions(+), 50 deletions(-) New commits: commit a7ae8de6e50ae209737f8927cf3a63e875620911 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Mon Mar 25 14:37:18 2013 -0700 Ticket #627 - ns-slapd crashes sporadically with segmentation fault in libslapd.so Bug Description: Schema reload task (schema-reload.pl) was not thread safe. Fix Description: Attribute Syntax is stored in the hash and retrieved based upon the attribute syntax. When Schema reload task is invoked, the attribute syntax objects were completely replaced ignoring the lock protection. This patch protects the attribute syntax replacement (attr_syntax_delete_all_for_ schemareload) with the write lock. Also, attribute syntax object maintains the reference count. The schema reload respects the reference count instead of blindly deleting them. https://fedorahosted.org/389/ticket/627 Reviewed by Rich (Thank you!!) (cherry picked from commit 81b997480956b2b6fa3a5d0e8d6abf5113a06400) diff --git a/ldap/servers/slapd/attrsyntax.c b/ldap/servers/slapd/attrsyntax.c index 7ac3b46..bd1a6f1 100644 --- a/ldap/servers/slapd/attrsyntax.c +++ b/ldap/servers/slapd/attrsyntax.c @@ -97,12 +97,27 @@ attr_syntax_read_lock(void) } void +attr_syntax_write_lock(void) +{ + if (0 != attr_syntax_init()) return; + + AS_LOCK_WRITE(oid2asi_lock); + AS_LOCK_WRITE(name2asi_lock); +} + +void attr_syntax_unlock_read(void) { - if(name2asi_lock) AS_UNLOCK_READ(name2asi_lock); - if(oid2asi_lock) AS_UNLOCK_READ(oid2asi_lock); + AS_UNLOCK_READ(name2asi_lock); + AS_UNLOCK_READ(oid2asi_lock); } +void +attr_syntax_unlock_write(void) +{ + AS_UNLOCK_WRITE(name2asi_lock); + AS_UNLOCK_WRITE(oid2asi_lock); +} #if 0 @@ -233,13 +248,17 @@ attr_syntax_get_by_oid_locking_optional( const char *oid, PRBool use_lock ) struct asyntaxinfo *asi = 0; if (oid2asi) { - if ( use_lock ) AS_LOCK_READ(oid2asi_lock); + if ( use_lock ) { + AS_LOCK_READ(oid2asi_lock); + } asi = (struct asyntaxinfo *)PL_HashTableLookup_const(oid2asi, oid); if (asi) { PR_AtomicIncrement( &asi->asi_refcnt ); } - if ( use_lock ) AS_UNLOCK_READ(oid2asi_lock); + if ( use_lock ) { + AS_UNLOCK_READ(oid2asi_lock); + } } return asi; @@ -257,13 +276,15 @@ attr_syntax_add_by_oid(const char *oid, struct asyntaxinfo *a, int lock) { if (0 != attr_syntax_init()) return; - if (lock) + if (lock) { AS_LOCK_WRITE(oid2asi_lock); + } PL_HashTableAdd(oid2asi, oid, a); - if (lock) + if (lock) { AS_UNLOCK_WRITE(oid2asi_lock); + } } /* @@ -304,12 +325,16 @@ attr_syntax_get_by_name_locking_optional(const char *name, PRBool use_lock) struct asyntaxinfo *asi = 0; if (name2asi) { - if ( use_lock ) AS_LOCK_READ(name2asi_lock); + if ( use_lock ) { + AS_LOCK_READ(name2asi_lock); + } asi = (struct asyntaxinfo *)PL_HashTableLookup_const(name2asi, name); if ( NULL != asi ) { PR_AtomicIncrement( &asi->asi_refcnt ); } - if ( use_lock ) AS_UNLOCK_READ(name2asi_lock); + if ( use_lock ) { + AS_UNLOCK_READ(name2asi_lock); + } } if (!asi) /* given name may be an OID */ asi = attr_syntax_get_by_oid_locking_optional(name, use_lock); @@ -331,30 +356,38 @@ attr_syntax_return( struct asyntaxinfo *asi ) } void -attr_syntax_return_locking_optional( struct asyntaxinfo *asi, PRBool use_lock ) +attr_syntax_return_locking_optional(struct asyntaxinfo *asi, PRBool use_lock) { + int locked = 0; + if(use_lock) { + AS_LOCK_READ(name2asi_lock); + locked = 1; + } if ( NULL != asi ) { - if ( 0 == PR_AtomicDecrement( &asi->asi_refcnt )) - { - PRBool delete_it; - - if(use_lock) AS_LOCK_READ(name2asi_lock); + PRBool delete_it = PR_FALSE; + if ( 0 == PR_AtomicDecrement( &asi->asi_refcnt )) { delete_it = asi->asi_marked_for_delete; - if(use_lock) AS_UNLOCK_READ(name2asi_lock); - - if ( delete_it ) - { - AS_LOCK_WRITE(name2asi_lock); /* get a write lock */ - if ( asi->asi_marked_for_delete ) /* one final check */ - { - /* ref count is 0 and it's flagged for - * deletion, so it's safe to free now */ - attr_syntax_free(asi); + } + + if (delete_it) { + if ( asi->asi_marked_for_delete ) { /* one final check */ + if(use_lock) { + AS_UNLOCK_READ(name2asi_lock); + AS_LOCK_WRITE(name2asi_lock); + } + /* ref count is 0 and it's flagged for + * deletion, so it's safe to free now */ + attr_syntax_free(asi); + if(use_lock) { + AS_UNLOCK_WRITE(name2asi_lock); + locked = 0; } - AS_UNLOCK_WRITE(name2asi_lock); } } } + if(locked) { + AS_UNLOCK_READ(name2asi_lock); + } } /* @@ -371,8 +404,9 @@ attr_syntax_add_by_name(struct asyntaxinfo *a, int lock) { if (0 != attr_syntax_init()) return; - if (lock) + if (lock) { AS_LOCK_WRITE(name2asi_lock); + } PL_HashTableAdd(name2asi, a->asi_name, a); if ( a->asi_aliases != NULL ) { @@ -383,8 +417,9 @@ attr_syntax_add_by_name(struct asyntaxinfo *a, int lock) } } - if (lock) + if (lock) { AS_UNLOCK_WRITE(name2asi_lock); + } } @@ -948,11 +983,11 @@ attr_syntax_enumerate_attrs(AttrEnumFunc aef, void *arg, PRBool writelock ) attr_syntax_enumerate_attrs_ext(oid2asi, aef, arg); if ( writelock ) { - AS_UNLOCK_WRITE(oid2asi_lock); AS_UNLOCK_WRITE(name2asi_lock); + AS_UNLOCK_WRITE(oid2asi_lock); } else { - AS_UNLOCK_READ(oid2asi_lock); AS_UNLOCK_READ(name2asi_lock); + AS_UNLOCK_READ(oid2asi_lock); } } @@ -1050,6 +1085,21 @@ attr_syntax_delete_all() (void *)&fi, PR_TRUE ); } +/* + * Delete all attribute definitions without attr_syntax lock. + * The caller is responsible for the lock. + */ +void +attr_syntax_delete_all_for_schemareload(unsigned long flag) +{ + struct attr_syntax_enum_flaginfo fi; + + memset(&fi, 0, sizeof(fi)); + fi.asef_flag = flag; + attr_syntax_enumerate_attrs_ext(oid2asi, attr_syntax_delete_if_not_flagged, + (void *)&fi); +} + static int attr_syntax_init(void) { @@ -1173,13 +1223,19 @@ static int attr_syntax_internal_asi_add(struct asyntaxinfo *asip, void *arg) { struct asyntaxinfo *asip_copy; + int rc = 0; + if (!asip) { return 1; } /* Copy is needed since when reloading the schema, * existing syntax info is cleaned up. */ asip_copy = attr_syntax_dup(asip); - return attr_syntax_add(asip_copy); + rc = attr_syntax_add(asip_copy); + if (LDAP_SUCCESS != rc) { + attr_syntax_free(asip_copy); + } + return rc; } /* Reload internal attribute syntax stashed in the internalasi hashtable. */ diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index e947248..e872cec 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -113,7 +113,9 @@ int attrlist_replace_with_flags(Slapi_Attr **alist, const char *type, struct ber * attrsyntax.c */ void attr_syntax_read_lock(void); +void attr_syntax_write_lock(void); void attr_syntax_unlock_read(void); +void attr_syntax_unlock_write(void); int attr_syntax_exists (const char *attr_name); void attr_syntax_delete ( struct asyntaxinfo *asip ); #define SLAPI_SYNTAXLENGTH_NONE (-1) /* for syntaxlength parameter */ @@ -140,6 +142,7 @@ struct asyntaxinfo *attr_syntax_get_by_name_with_default ( const char *name ); void attr_syntax_return( struct asyntaxinfo *asi ); void attr_syntax_return_locking_optional( struct asyntaxinfo *asi, PRBool use_lock ); void attr_syntax_delete_all(void); +void attr_syntax_delete_all_for_schemareload(unsigned long flag); /* * value.c diff --git a/ldap/servers/slapd/schema.c b/ldap/servers/slapd/schema.c index 5eb6520..dc56534 100644 --- a/ldap/servers/slapd/schema.c +++ b/ldap/servers/slapd/schema.c @@ -1393,7 +1393,7 @@ slapi_schema_list_attribute_names(unsigned long flag) aew.flag=flag; attr_syntax_enumerate_attrs(schema_list_attributes_callback, &aew, - PR_FALSE); + PR_FALSE); return aew.attrs; } @@ -2405,8 +2405,9 @@ static int schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, size_t errorbufsize ) { - int i, rc = LDAP_SUCCESS; - struct asyntaxinfo *newasip, *oldasip; + int i, rc = LDAP_SUCCESS; + struct asyntaxinfo *newasip, *oldasip; + PRUint32 schema_flags = 0; if ( NULL == mod->mod_bvalues ) { schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_at, @@ -2414,8 +2415,11 @@ schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, return LDAP_UNWILLING_TO_PERFORM; } - /* clear all of the "keep" flags */ - attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + slapi_pblock_get(pb, SLAPI_SCHEMA_FLAGS, &schema_flags); + if (!(schema_flags & (DSE_SCHEMA_NO_LOAD|DSE_SCHEMA_NO_CHECK))) { + /* clear all of the "keep" flags unless it's from schema-reload */ + attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + } for ( i = 0; mod->mod_bvalues[i] != NULL; ++i ) { if ( LDAP_SUCCESS != ( rc = read_at_ldif( mod->mod_bvalues[i]->bv_val, @@ -2473,12 +2477,14 @@ schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, * XXXmcs: we should consider reporting an error if any read only types * remain.... */ - attr_syntax_delete_all_not_flagged( SLAPI_ATTR_FLAG_KEEP - | SLAPI_ATTR_FLAG_STD_ATTR ); + attr_syntax_delete_all_not_flagged( SLAPI_ATTR_FLAG_KEEP | + SLAPI_ATTR_FLAG_STD_ATTR ); clean_up_and_return: - /* clear all of the "keep" flags */ - attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + if (!(schema_flags & (DSE_SCHEMA_NO_LOAD|DSE_SCHEMA_NO_CHECK))) { + /* clear all of the "keep" flags unless it's from schema-reload */ + attr_syntax_all_clear_flag( SLAPI_ATTR_FLAG_KEEP ); + } return rc; } @@ -3894,14 +3900,12 @@ load_schema_dse(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *ignored, int primary_file = 0; /* this is the primary (writeable) schema file */ int schema_ds4x_compat = config_get_ds4_compatible_schema(); PRUint32 flags = *(PRUint32 *)arg; - flags |= DSE_SCHEMA_NO_GLOCK; /* don't lock global resources - during initialization */ *returncode = 0; /* * Note: there is no need to call schema_lock_write() here because this - * function is only called during server startup. + * function is only called during server startup. */ slapi_pblock_get( pb, SLAPI_DSE_IS_PRIMARY_FILE, &primary_file ); @@ -3943,6 +3947,8 @@ load_schema_dse(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *ignored, if (*returncode) return SLAPI_DSE_CALLBACK_ERROR; + flags |= DSE_SCHEMA_NO_GLOCK; /* don't lock global resources + during initialization */ if (!slapi_entry_attr_find(e, "objectclasses", &attr) && attr) { /* enumerate the values in attr */ @@ -4013,7 +4019,6 @@ load_schema_dse(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *ignored, * DSE_SCHEMA_NO_CHECK -- schema won't be checked * DSE_SCHEMA_NO_BACKEND -- don't add as backend * DSE_SCHEMA_LOCKED -- already locked; no further lock needed - */ static int init_schema_dse_ext(char *schemadir, Slapi_Backend *be, @@ -4119,7 +4124,7 @@ init_schema_dse_ext(char *schemadir, Slapi_Backend *be, "DESC 'Standard schema for LDAP' SYNTAX " "1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'RFC 2252' )", NULL, errorbuf, SLAPI_DSE_RETURNTEXT_SIZE, - DSE_SCHEMA_NO_GLOCK|schema_flags, 0, 0, 0); + schema_flags, 0, 0, 0); } if (rc) { @@ -4192,7 +4197,7 @@ init_schema_dse(const char *configdir) { schemadir = slapi_ch_smprintf("%s/%s", configdir, SCHEMA_SUBDIR_NAME); } - rc = init_schema_dse_ext(schemadir, NULL, &pschemadse, 0); + rc = init_schema_dse_ext(schemadir, NULL, &pschemadse, DSE_SCHEMA_NO_GLOCK); slapi_ch_free_string(&schemadir); return rc; } @@ -4856,14 +4861,14 @@ slapi_validate_schema_files(char *schemadir) { struct dse *my_pschemadse = NULL; int rc = init_schema_dse_ext(schemadir, NULL, &my_pschemadse, - DSE_SCHEMA_NO_LOAD | DSE_SCHEMA_NO_BACKEND); + DSE_SCHEMA_NO_LOAD | DSE_SCHEMA_NO_BACKEND); dse_destroy(my_pschemadse); /* my_pschemadse was created just to - validate the schema */ + validate the schema */ if (rc) { return LDAP_SUCCESS; } else { slapi_log_error( SLAPI_LOG_FATAL, "schema_reload", - "schema file validation failed\n" ); + "schema file validation failed\n" ); return LDAP_OBJECT_CLASS_VIOLATION; } } @@ -4889,10 +4894,13 @@ slapi_reload_schema_files(char *schemadir) } slapi_be_Wlock(be); /* be lock must be outer of schemafile lock */ reload_schemafile_lock(); - attr_syntax_delete_all(); + /* Exclude attr_syntax not to grab from the hash table while cleaning up */ + attr_syntax_write_lock(); + attr_syntax_delete_all_for_schemareload(SLAPI_ATTR_FLAG_KEEP); oc_delete_all_nolock(); + attr_syntax_unlock_write(); rc = init_schema_dse_ext(schemadir, be, &my_pschemadse, - DSE_SCHEMA_NO_CHECK | DSE_SCHEMA_LOCKED); + DSE_SCHEMA_NO_CHECK | DSE_SCHEMA_LOCKED); if (rc) { dse_destroy(pschemadse); pschemadse = my_pschemadse;

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

389-commits March 2013