diff options
author | Noriko Hosoi <nhosoi@redhat.com> | 2007-10-12 18:03:43 +0000 |
---|---|---|
committer | Noriko Hosoi <nhosoi@redhat.com> | 2007-10-12 18:03:43 +0000 |
commit | ee385e64f9dff2f36ffa8eebd872f98aadcffbdb (patch) | |
tree | ee202c50603315d75268df89ad58398b7e9bc6be | |
parent | ad8f6417258a3eca48da8cd8778721fdecc61078 (diff) | |
download | ds-ee385e64f9dff2f36ffa8eebd872f98aadcffbdb.tar.gz ds-ee385e64f9dff2f36ffa8eebd872f98aadcffbdb.tar.xz ds-ee385e64f9dff2f36ffa8eebd872f98aadcffbdb.zip |
Resolves: #193724
Summary: "nested" filtered roles result in deadlock (Comment #12)
Description:
1. Changed cache_lock to the read-write lock.
2. Instead of using the local vattr_context in vattr_test_filter, use the one
set in pblock as much as possible. To achieve the goal, introduced
pb_vattr_context to pblock.
3. Increased VATTR_LOOP_COUNT_MAX from 50 to 256.
4. When the loop count hit VATTR_LOOP_COUNT_MAX, it sets
LDAP_UNWILLING_TO_PERFORM and returns it to the client.
-rw-r--r-- | ldap/servers/plugins/cos/cos_cache.c | 9 | ||||
-rw-r--r-- | ldap/servers/plugins/roles/roles_cache.c | 142 | ||||
-rw-r--r-- | ldap/servers/plugins/roles/roles_cache.h | 1 | ||||
-rw-r--r-- | ldap/servers/plugins/roles/roles_plugin.c | 2 | ||||
-rw-r--r-- | ldap/servers/slapd/back-ldbm/ldbm_search.c | 361 | ||||
-rw-r--r-- | ldap/servers/slapd/filterentry.c | 12 | ||||
-rw-r--r-- | ldap/servers/slapd/slap.h | 1 | ||||
-rw-r--r-- | ldap/servers/slapd/slapi-private.h | 3 | ||||
-rw-r--r-- | ldap/servers/slapd/vattr.c | 72 | ||||
-rw-r--r-- | ldap/servers/slapd/vattr_spi.h | 2 |
10 files changed, 340 insertions, 265 deletions
diff --git a/ldap/servers/plugins/cos/cos_cache.c b/ldap/servers/plugins/cos/cos_cache.c index 9eac62a1..88678868 100644 --- a/ldap/servers/plugins/cos/cos_cache.c +++ b/ldap/servers/plugins/cos/cos_cache.c @@ -2222,6 +2222,7 @@ bail: returns 0 on success, we added a computed attribute 1 on outright failure + > LDAP ERROR CODE -1 when doesn't know about attribute {PARPAR} must also check the attribute does not exist if we are not @@ -2392,10 +2393,14 @@ static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Sl int free_flags = 0; if(pSpec && pSpec->val) { - slapi_vattr_values_get_sp(context, e, pSpec->val, &pAttrSpecs, &type_name_disposition, &actual_type_name, 0, &free_flags); + ret = slapi_vattr_values_get_sp(context, e, pSpec->val, &pAttrSpecs, &type_name_disposition, &actual_type_name, 0, &free_flags); /* MAB: We need to free actual_type_name here !!! XXX BAD--should use slapi_vattr_values_free() */ slapi_ch_free((void **) &actual_type_name); + if (SLAPI_VIRTUALATTRS_LOOP_DETECTED == ret) { + ret = LDAP_UNWILLING_TO_PERFORM; + goto bail; + } } if(pAttrSpecs || pDef->cosType == COSTYPE_POINTER) @@ -2548,6 +2553,8 @@ static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Sl ret = 1; else if(hit == 1) ret = 0; + else + ret = -1; if(props) *props = 0; diff --git a/ldap/servers/plugins/roles/roles_cache.c b/ldap/servers/plugins/roles/roles_cache.c index 9aa00191..4d3c4037 100644 --- a/ldap/servers/plugins/roles/roles_cache.c +++ b/ldap/servers/plugins/roles/roles_cache.c @@ -105,7 +105,7 @@ typedef struct _roles_cache_def { PRThread *roles_tid; int keeprunning; - Slapi_Mutex *cache_lock; + PRRWLock *cache_lock; Slapi_Mutex *stop_lock; Slapi_Mutex *change_lock; @@ -143,6 +143,7 @@ typedef struct _roles_cache_build_result Slapi_Entry *requested_entry; /* entry to get nsrole from */ int has_value; /* flag to determine if a new value has been added to the result */ int need_value; /* flag to determine if we need the result */ + vattr_context *context; /* vattr context */ } roles_cache_build_result; /* Structure used to check if is_entry_member_of is part of a role defined in its suffix */ @@ -178,8 +179,9 @@ static int roles_cache_build_nsrole( caddr_t data, caddr_t arg ); static int roles_cache_find_node( caddr_t d1, caddr_t d2 ); static int roles_cache_find_roles_in_suffix(Slapi_DN *target_entry_dn, roles_cache_def **list_of_roles); static int roles_is_entry_member_of_object(caddr_t data, caddr_t arg ); +static int roles_is_entry_member_of_object_ext(vattr_context *c, caddr_t data, caddr_t arg ); static int roles_check_managed(Slapi_Entry *entry_to_check, role_object *role, int *present); -static int roles_check_filtered(Slapi_Entry *entry_to_check, role_object *role, int *present); +static int roles_check_filtered(vattr_context *c, Slapi_Entry *entry_to_check, role_object *role, int *present); static int roles_check_nested(caddr_t data, caddr_t arg); static int roles_is_inscope(Slapi_Entry *entry_to_check, Slapi_DN *role_dn); static void berval_set_string(struct berval *bv, const char* string); @@ -303,7 +305,7 @@ static roles_cache_def *roles_cache_create_suffix(Slapi_DN *sdn) return(NULL); } - new_suffix->cache_lock = slapi_new_mutex(); + new_suffix->cache_lock = PR_NewRWLock(PR_RWLOCK_RANK_NONE, "roles_def_lock"); new_suffix->change_lock = slapi_new_mutex(); new_suffix->stop_lock = slapi_new_mutex(); new_suffix->create_lock = slapi_new_mutex(); @@ -610,7 +612,7 @@ static int roles_cache_update(roles_cache_def *suffix_to_update) slapi_log_error( SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, "--> roles_cache_update \n"); - slapi_lock_mutex(suffix_to_update->cache_lock); + PR_RWLock_Wlock(suffix_to_update->cache_lock); operation = suffix_to_update->notified_operation; entry = suffix_to_update->notified_entry; @@ -646,7 +648,7 @@ static int roles_cache_update(roles_cache_def *suffix_to_update) suffix_to_update->notified_entry = NULL; } - slapi_unlock_mutex(suffix_to_update->cache_lock); + PR_RWLock_Unlock(suffix_to_update->cache_lock); if ( dn != NULL ) { @@ -1426,6 +1428,11 @@ static int roles_cache_object_nested_from_dn(Slapi_DN *role_dn, role_object_nest */ int roles_cache_listroles(Slapi_Entry *entry, int return_values, Slapi_ValueSet **valueset_out) { + return roles_cache_listroles_ext(NULL, entry, return_values, valueset_out); +} + +int roles_cache_listroles_ext(vattr_context *c, Slapi_Entry *entry, int return_values, Slapi_ValueSet **valueset_out) +{ roles_cache_def *roles_cache = NULL; int rc = 0; roles_cache_build_result arg; @@ -1464,13 +1471,14 @@ int roles_cache_listroles(Slapi_Entry *entry, int return_values, Slapi_ValueSet arg.need_value = return_values; arg.requested_entry = entry; arg.has_value = 0; + arg.context = c; /* XXX really need a mutex for this read operation ? */ - slapi_lock_mutex(roles_cache->cache_lock); + PR_RWLock_Rlock(roles_cache->cache_lock); avl_apply(roles_cache->avl_tree, (IFP)roles_cache_build_nsrole, &arg, -1, AVL_INORDER); - slapi_unlock_mutex(roles_cache->cache_lock); + PR_RWLock_Unlock(roles_cache->cache_lock); if( !arg.has_value ) { @@ -1507,53 +1515,59 @@ int roles_cache_listroles(Slapi_Entry *entry, int return_values, Slapi_ValueSet ------------------------ Traverse the tree containing roles definitions for a suffix and for each one of them, check wether the entry is a member of it or not - For ones which check out positive, we add their DN to the value - always return 0 to allow to trverse all the tree + For ones which check out positive, we add their DN to the value + always return 0 to allow to trverse all the tree */ static int roles_cache_build_nsrole( caddr_t data, caddr_t arg ) { Slapi_Value *value = NULL; roles_cache_build_result *result = (roles_cache_build_result*)arg; role_object *this_role = (role_object*)data; - roles_cache_search_in_nested get_nsrole; + roles_cache_search_in_nested get_nsrole; /* Return a value different from the stop flag to be able to go through all the tree */ - int rc = 0; + int rc = 0; + int tmprc = 0; - slapi_log_error(SLAPI_LOG_PLUGIN, - ROLES_PLUGIN_SUBSYSTEM, "--> roles_cache_build_nsrole: role %s\n", - (char*) slapi_sdn_get_ndn(this_role->dn)); + slapi_log_error(SLAPI_LOG_PLUGIN, + ROLES_PLUGIN_SUBSYSTEM, "--> roles_cache_build_nsrole: role %s\n", + (char*) slapi_sdn_get_ndn(this_role->dn)); value = slapi_value_new_string(""); - get_nsrole.is_entry_member_of = result->requested_entry; - get_nsrole.present = 0; - get_nsrole.hint = 0; + get_nsrole.is_entry_member_of = result->requested_entry; + get_nsrole.present = 0; + get_nsrole.hint = 0; - roles_is_entry_member_of_object((caddr_t)this_role, (caddr_t)&get_nsrole); + tmprc = roles_is_entry_member_of_object_ext(result->context, (caddr_t)this_role, (caddr_t)&get_nsrole); + if (SLAPI_VIRTUALATTRS_LOOP_DETECTED == tmprc) + { + /* all we want to detect and return is loop/stack overflow */ + rc = tmprc; + } /* If so, add its DN to the attribute */ if (get_nsrole.present) { result->has_value = 1; - if ( result->need_value ) - { - slapi_value_set_string(value,(char*) slapi_sdn_get_ndn(this_role->dn)); - slapi_valueset_add_value(*(result->nsrole_values),value); - } - else - { - /* we don't need the value but we already know there is one nsrole. - stop the traversal - */ - rc = -1; - } + if ( result->need_value ) + { + slapi_value_set_string(value,(char*) slapi_sdn_get_ndn(this_role->dn)); + slapi_valueset_add_value(*(result->nsrole_values),value); + } + else + { + /* we don't need the value but we already know there is one nsrole. + stop the traversal + */ + rc = -1; + } } slapi_value_free(&value); - slapi_log_error(SLAPI_LOG_PLUGIN, - ROLES_PLUGIN_SUBSYSTEM, "<-- roles_cache_build_nsrole\n"); + slapi_log_error(SLAPI_LOG_PLUGIN, + ROLES_PLUGIN_SUBSYSTEM, "<-- roles_cache_build_nsrole\n"); return rc; } @@ -1564,54 +1578,54 @@ static int roles_cache_build_nsrole( caddr_t data, caddr_t arg ) Checks if an entry has a presented role, assuming that we've already verified that the role both exists and is in scope - return 0: no processing error - return -1: error + return 0: no processing error + return -1: error */ int roles_check(Slapi_Entry *entry_to_check, Slapi_DN *role_dn, int *present) { roles_cache_def *roles_cache = NULL; role_object *this_role = NULL; - roles_cache_search_in_nested get_nsrole; + roles_cache_search_in_nested get_nsrole; int rc = 0; - slapi_log_error(SLAPI_LOG_PLUGIN, - ROLES_PLUGIN_SUBSYSTEM, "--> roles_check\n"); + slapi_log_error(SLAPI_LOG_PLUGIN, + ROLES_PLUGIN_SUBSYSTEM, "--> roles_check\n"); - *present = 0; + *present = 0; - PR_RWLock_Rlock(global_lock); + PR_RWLock_Rlock(global_lock); if ( roles_cache_find_roles_in_suffix(slapi_entry_get_sdn(entry_to_check), &roles_cache) != 0 ) { - PR_RWLock_Unlock(global_lock); + PR_RWLock_Unlock(global_lock); return -1; } - PR_RWLock_Unlock(global_lock); + PR_RWLock_Unlock(global_lock); this_role = (role_object *)avl_find(roles_cache->avl_tree, role_dn, (IFP)roles_cache_find_node); - /* MAB: For some reason the assumption made by this function (the role exists and is in scope) - * does not seem to be true... this_role might be NULL after the avl_find call (is the avl_tree - * broken? Anyway, this is crashing the 5.1 server on 29-Aug-01, so I am applying the following patch - * to avoid the crash inside roles_is_entry_member_of_object */ - /* Begin patch */ - if (!this_role) { - /* Assume that the entry is not member of the role (*present=0) and leave... */ - return rc; - } - /* End patch */ + /* MAB: For some reason the assumption made by this function (the role exists and is in scope) + * does not seem to be true... this_role might be NULL after the avl_find call (is the avl_tree + * broken? Anyway, this is crashing the 5.1 server on 29-Aug-01, so I am applying the following patch + * to avoid the crash inside roles_is_entry_member_of_object */ + /* Begin patch */ + if (!this_role) { + /* Assume that the entry is not member of the role (*present=0) and leave... */ + return rc; + } + /* End patch */ - get_nsrole.is_entry_member_of = entry_to_check; - get_nsrole.present = 0; - get_nsrole.hint = 0; + get_nsrole.is_entry_member_of = entry_to_check; + get_nsrole.present = 0; + get_nsrole.hint = 0; roles_is_entry_member_of_object((caddr_t)this_role, (caddr_t)&get_nsrole); - *present = get_nsrole.present; + *present = get_nsrole.present; - slapi_log_error(SLAPI_LOG_PLUGIN, - ROLES_PLUGIN_SUBSYSTEM, "<-- roles_check\n"); + slapi_log_error(SLAPI_LOG_PLUGIN, + ROLES_PLUGIN_SUBSYSTEM, "<-- roles_check\n"); return rc; } @@ -1691,6 +1705,11 @@ static int roles_cache_find_roles_in_suffix(Slapi_DN *target_entry_dn, roles_cac */ static int roles_is_entry_member_of_object(caddr_t data, caddr_t argument ) { + return roles_is_entry_member_of_object_ext(NULL, data, argument ); +} + +static int roles_is_entry_member_of_object_ext(vattr_context *c, caddr_t data, caddr_t argument ) +{ int rc = -1; roles_cache_search_in_nested *get_nsrole = (roles_cache_search_in_nested*)argument; @@ -1717,7 +1736,7 @@ static int roles_is_entry_member_of_object(caddr_t data, caddr_t argument ) rc = roles_check_managed(entry_to_check,this_role,&get_nsrole->present); break; case ROLE_TYPE_FILTERED: - rc = roles_check_filtered(entry_to_check,this_role,&get_nsrole->present); + rc = roles_check_filtered(c, entry_to_check,this_role,&get_nsrole->present); break; case ROLE_TYPE_NESTED: { @@ -1789,13 +1808,14 @@ static int roles_check_managed(Slapi_Entry *entry_to_check, role_object *role, i return 1: fail -> to check the presence, see present */ -static int roles_check_filtered(Slapi_Entry *entry_to_check, role_object *role, int *present) +static int roles_check_filtered(vattr_context *c, Slapi_Entry *entry_to_check, role_object *role, int *present) { int rc = 0; slapi_log_error(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, "--> roles_check_filtered\n"); - rc = slapi_filter_test_simple(entry_to_check,role->filter); + rc = slapi_vattr_filter_test_ext(slapi_vattr_get_pblock_from_context(c), + entry_to_check, role->filter, 0, 0); if ( rc == 0 ) { *present = 1; @@ -1991,7 +2011,7 @@ static void roles_cache_role_def_free(roles_cache_def *role_def) avl_free(role_def->avl_tree, (IFP)roles_cache_role_object_free); slapi_sdn_free(&(role_def->suffix_dn)); - slapi_destroy_mutex(role_def->cache_lock); + PR_DestroyRWLock(role_def->cache_lock); role_def->cache_lock = NULL; slapi_destroy_mutex(role_def->change_lock); role_def->change_lock = NULL; diff --git a/ldap/servers/plugins/roles/roles_cache.h b/ldap/servers/plugins/roles/roles_cache.h index c4049d9a..870f5a06 100644 --- a/ldap/servers/plugins/roles/roles_cache.h +++ b/ldap/servers/plugins/roles/roles_cache.h @@ -73,6 +73,7 @@ int roles_cache_init(); void roles_cache_stop(); void roles_cache_change_notify(Slapi_PBlock *pb); int roles_cache_listroles(Slapi_Entry *entry, int return_value, Slapi_ValueSet **valueset_out); +int roles_cache_listroles_ext(vattr_context *c, Slapi_Entry *entry, int return_value, Slapi_ValueSet **valueset_out); int roles_check(Slapi_Entry *entry_to_check, Slapi_DN *role_dn, int *present); diff --git a/ldap/servers/plugins/roles/roles_plugin.c b/ldap/servers/plugins/roles/roles_plugin.c index 4db36c42..9edc903c 100644 --- a/ldap/servers/plugins/roles/roles_plugin.c +++ b/ldap/servers/plugins/roles/roles_plugin.c @@ -248,7 +248,7 @@ int roles_sp_get_value(vattr_sp_handle *handle, { int rc = -1; - rc = roles_cache_listroles(e, 1, results); + rc = roles_cache_listroles_ext(c, e, 1, results); if (rc == 0) { *free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES; diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index 162e3960..7979b585 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -48,14 +48,14 @@ /* prototypes */ static int build_candidate_list( Slapi_PBlock *pb, backend *be, - struct backentry *e, const char * base, int scope, - int *lookup_returned_allidsp, IDList** candidates); + struct backentry *e, const char * base, int scope, + int *lookup_returned_allidsp, IDList** candidates); static IDList *base_candidates( Slapi_PBlock *pb, struct backentry *e ); static IDList *onelevel_candidates( Slapi_PBlock *pb, backend *be, const char *base, struct backentry *e, Slapi_Filter *filter, int managedsait, int *lookup_returned_allidsp, int *err ); static back_search_result_set* new_search_result_set(IDList* idl,int vlv, int lookthroughlimit); static void delete_search_result_set( back_search_result_set **sr ); static int can_skip_filter_test( Slapi_PBlock *pb, struct slapi_filter *f, - int scope, IDList *idl ); + int scope, IDList *idl ); /* This is for performance testing, allows us to disable ACL checking altogether */ #if defined(DISABLE_ACL_CHECK) @@ -69,38 +69,38 @@ static int can_skip_filter_test( Slapi_PBlock *pb, struct slapi_filter *f, static int compute_lookthrough_limit( Slapi_PBlock *pb, struct ldbminfo *li ) { - Slapi_Connection *conn = NULL; - int limit; - - slapi_pblock_get( pb, SLAPI_CONNECTION, &conn); - - if ( slapi_reslimit_get_integer_limit( conn, - li->li_reslimit_lookthrough_handle, &limit ) - != SLAPI_RESLIMIT_STATUS_SUCCESS ) { - /* - * no limit associated with binder/connection or some other error - * occurred. use the default. - */ - int isroot = 0; - - slapi_pblock_get( pb, SLAPI_REQUESTOR_ISROOT, &isroot ); - if (isroot) { - limit = -1; - } else { - PR_Lock(li->li_config_mutex); - limit = li->li_lookthroughlimit; - PR_Unlock(li->li_config_mutex); - } - } - - return( limit ); + Slapi_Connection *conn = NULL; + int limit; + + slapi_pblock_get( pb, SLAPI_CONNECTION, &conn); + + if ( slapi_reslimit_get_integer_limit( conn, + li->li_reslimit_lookthrough_handle, &limit ) + != SLAPI_RESLIMIT_STATUS_SUCCESS ) { + /* + * no limit associated with binder/connection or some other error + * occurred. use the default. + */ + int isroot = 0; + + slapi_pblock_get( pb, SLAPI_REQUESTOR_ISROOT, &isroot ); + if (isroot) { + limit = -1; + } else { + PR_Lock(li->li_config_mutex); + limit = li->li_lookthroughlimit; + PR_Unlock(li->li_config_mutex); + } + } + + return( limit ); } /* don't free the berval, just clean it */ static void berval_done(struct berval *val) { - slapi_ch_free_string(&val->bv_val); + slapi_ch_free_string(&val->bv_val); } /* @@ -116,20 +116,20 @@ int ldbm_back_search_cleanup(Slapi_PBlock *pb, struct ldbminfo *li, sort_spec_th { slapi_send_ldap_result( pb, ldap_result, NULL, ldap_result_description, 0, NULL ); } - { - /* hack hack --- code to free the result set if we don't need it */ - /* We get it and check to see if the structure was ever used */ - back_search_result_set *sr = NULL; - slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &sr ); - if ( (NULL != sr) && (function_result != 0) ) { - delete_search_result_set(&sr); - } - } - slapi_sdn_done(sdn); - if (vlv_request_control) - { - berval_done(&vlv_request_control->value); - } + { + /* hack hack --- code to free the result set if we don't need it */ + /* We get it and check to see if the structure was ever used */ + back_search_result_set *sr = NULL; + slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &sr ); + if ( (NULL != sr) && (function_result != 0) ) { + delete_search_result_set(&sr); + } + } + slapi_sdn_done(sdn); + if (vlv_request_control) + { + berval_done(&vlv_request_control->value); + } return function_result; } @@ -630,8 +630,8 @@ ldbm_back_search( Slapi_PBlock *pb ) */ static int build_candidate_list( Slapi_PBlock *pb, backend *be, struct backentry *e, - const char * base, int scope, int *lookup_returned_allidsp, - IDList** candidates) + const char * base, int scope, int *lookup_returned_allidsp, + IDList** candidates) { struct ldbminfo *li = (struct ldbminfo *) be->be_database->plg_private; int managedsait= 0; @@ -875,123 +875,123 @@ subtree_candidates( return( candidates ); } -static int grok_filter(struct slapi_filter *f); +static int grok_filter(struct slapi_filter *f); #if 0 /* Helper for grok_filter() */ static int -grok_filter_list(struct slapi_filter *flist) +grok_filter_list(struct slapi_filter *flist) { - struct slapi_filter *f; - - /* Scan the clauses of the AND filter, if any of them fails the grok, then we fail */ - for ( f = flist; f != NULL; f = f->f_next ) { - if ( !grok_filter(f) ) { - return( 0 ); - } - } - return( 1 ); + struct slapi_filter *f; + + /* Scan the clauses of the AND filter, if any of them fails the grok, then we fail */ + for ( f = flist; f != NULL; f = f->f_next ) { + if ( !grok_filter(f) ) { + return( 0 ); + } + } + return( 1 ); } #endif /* Helper function for can_skip_filter_test() */ -static int grok_filter(struct slapi_filter *f) +static int grok_filter(struct slapi_filter *f) { - switch ( f->f_choice ) { - case LDAP_FILTER_EQUALITY: - return 1; /* If there's an ID list and an equality filter, we can skip the filter test */ - case LDAP_FILTER_SUBSTRINGS: - return 0; + switch ( f->f_choice ) { + case LDAP_FILTER_EQUALITY: + return 1; /* If there's an ID list and an equality filter, we can skip the filter test */ + case LDAP_FILTER_SUBSTRINGS: + return 0; - case LDAP_FILTER_GE: - return 1; + case LDAP_FILTER_GE: + return 1; - case LDAP_FILTER_LE: - return 1; + case LDAP_FILTER_LE: + return 1; - case LDAP_FILTER_PRESENT: - return 1; /* If there's an ID list, and a presence filter, we can skip the filter test */ + case LDAP_FILTER_PRESENT: + return 1; /* If there's an ID list, and a presence filter, we can skip the filter test */ - case LDAP_FILTER_APPROX: - return 0; + case LDAP_FILTER_APPROX: + return 0; - case LDAP_FILTER_EXTENDED: - return 0; + case LDAP_FILTER_EXTENDED: + return 0; - case LDAP_FILTER_AND: - return 0; /* Unless we check to see whether the presence and equality branches - of the search filter were all indexed, we get things wrong here, - so let's punt for now */ - /* return grok_filter_list(f->f_and); AND clauses are potentially OK */ + case LDAP_FILTER_AND: + return 0; /* Unless we check to see whether the presence and equality branches + of the search filter were all indexed, we get things wrong here, + so let's punt for now */ + /* return grok_filter_list(f->f_and); AND clauses are potentially OK */ - case LDAP_FILTER_OR: - return 0; + case LDAP_FILTER_OR: + return 0; - case LDAP_FILTER_NOT: - return 0; + case LDAP_FILTER_NOT: + return 0; - default: - return 0; - } + default: + return 0; + } } /* Routine which says whether or not the indices produced a "correct" answer */ static int can_skip_filter_test( - Slapi_PBlock *pb, - struct slapi_filter *f, - int scope, - IDList *idl + Slapi_PBlock *pb, + struct slapi_filter *f, + int scope, + IDList *idl ) { - int rc = 0; - - /* Is the ID list ALLIDS ? */ - if ( ALLIDS(idl)) { - /* If so, then can't optimize */ - return rc; - } - - /* Is this a base scope search? */ - if ( scope == LDAP_SCOPE_BASE ) { - /* - * If so, then we can't optimize. Why not? Because we only consult - * the entrydn index in producing our 1 candidate, and that means - * we have not used the filter to produce the candidate list. - */ - return rc; - } - - /* Grok the filter and tell me if it has only equality components in it */ - rc = grok_filter(f); - - /* If we haven't determined that we can't skip the filter test already, - * do one last check for attribute subtypes. We don't need to worry - * about any complex filters here since grok_filter() will have already - * assumed that we can't skip the filter test in those cases. */ - if (rc != 0) { - char *type = NULL; - char *basetype = NULL; - - /* We don't need to free type since that's taken - * care of when the filter is free'd later. We - * do need to free basetype when we are done. */ - slapi_filter_get_attribute_type(f, &type); - basetype = slapi_attr_basetype(type, NULL, 0); - - /* Is the filter using an attribute subtype? */ - if (strcasecmp(type, basetype) != 0) { - /* If so, we can't optimize since attribute subtypes - * are simply indexed under their basetype attribute. - * The basetype index has no knowledge of the subtype - * itself. In the future, we should add support for - * indexing the subtypes so we can optimize this type - * of search. */ - rc = 0; - } - slapi_ch_free_string(&basetype); - } - - return rc; + int rc = 0; + + /* Is the ID list ALLIDS ? */ + if ( ALLIDS(idl)) { + /* If so, then can't optimize */ + return rc; + } + + /* Is this a base scope search? */ + if ( scope == LDAP_SCOPE_BASE ) { + /* + * If so, then we can't optimize. Why not? Because we only consult + * the entrydn index in producing our 1 candidate, and that means + * we have not used the filter to produce the candidate list. + */ + return rc; + } + + /* Grok the filter and tell me if it has only equality components in it */ + rc = grok_filter(f); + + /* If we haven't determined that we can't skip the filter test already, + * do one last check for attribute subtypes. We don't need to worry + * about any complex filters here since grok_filter() will have already + * assumed that we can't skip the filter test in those cases. */ + if (rc != 0) { + char *type = NULL; + char *basetype = NULL; + + /* We don't need to free type since that's taken + * care of when the filter is free'd later. We + * do need to free basetype when we are done. */ + slapi_filter_get_attribute_type(f, &type); + basetype = slapi_attr_basetype(type, NULL, 0); + + /* Is the filter using an attribute subtype? */ + if (strcasecmp(type, basetype) != 0) { + /* If so, we can't optimize since attribute subtypes + * are simply indexed under their basetype attribute. + * The basetype index has no knowledge of the subtype + * itself. In the future, we should add support for + * indexing the subtypes so we can optimize this type + * of search. */ + rc = 0; + } + slapi_ch_free_string(&basetype); + } + + return rc; } @@ -1014,24 +1014,25 @@ ldbm_back_next_search_entry( Slapi_PBlock *pb ) int ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) { - backend *be; - ldbm_instance *inst; + backend *be; + ldbm_instance *inst; struct ldbminfo *li; - int scope; - int managedsait; - Slapi_Attr *attr; - Slapi_Filter *filter; - char *base; - back_search_result_set *sr; - ID id; - struct backentry *e; - int nentries; - time_t curtime, stoptime, optime; - int tlimit, llimit, slimit, isroot; - struct berval **urls = NULL; - int err; - Slapi_DN basesdn; - char *target_uniqueid; + int scope; + int managedsait; + Slapi_Attr *attr; + Slapi_Filter *filter; + char *base; + back_search_result_set *sr; + ID id; + struct backentry *e; + int nentries; + time_t curtime, stoptime, optime; + int tlimit, llimit, slimit, isroot; + struct berval **urls = NULL; + int err; + Slapi_DN basesdn; + char *target_uniqueid; + int rc = 0; slapi_pblock_get( pb, SLAPI_BACKEND, &be ); slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li ); @@ -1083,8 +1084,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY_EXT, NULL ); } slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, NULL ); - slapi_sdn_done(&basesdn); - return -1; + rc = SLAPI_FAIL_GENERAL; + goto bail; } /* check time limit */ @@ -1097,8 +1098,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY_EXT, NULL ); } slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, NULL ); - slapi_sdn_done(&basesdn); - return -1; + rc = SLAPI_FAIL_GENERAL; + goto bail; } /* check lookthrough limit */ @@ -1110,8 +1111,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY_EXT, NULL ); } slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, NULL ); - slapi_sdn_done(&basesdn); - return -1; + rc = SLAPI_FAIL_GENERAL; + goto bail; } /* get the entry */ @@ -1124,8 +1125,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY_EXT, NULL ); } slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, NULL ); - slapi_sdn_done(&basesdn); - return 0; + rc = 0; + goto bail; } ++sr->sr_lookthroughcount; /* checked above */ @@ -1142,8 +1143,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) * is gonna be traumatic. unavoidable. */ slapi_send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL); - slapi_sdn_done(&basesdn); - return return_on_disk_full(li); + rc = return_on_disk_full(li); + goto bail; } } LDAPDebug( LDAP_DEBUG_ARGS, "candidate %lu not found\n", (u_long)id, 0, 0 ); @@ -1182,8 +1183,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY_EXT, e ); } slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, e->ep_entry ); - slapi_sdn_done(&basesdn); - return 0; + rc = 0; + goto bail; } } else @@ -1253,8 +1254,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) cache_return( &inst->inst_cache, &e ); delete_search_result_set( &sr ); slapi_send_ldap_result( pb, LDAP_SIZELIMIT_EXCEEDED, NULL, NULL, nentries, urls ); - slapi_sdn_done(&basesdn); - return -1; + rc = SLAPI_FAIL_GENERAL; + goto bail; } slapi_pblock_set( pb, SLAPI_SEARCH_SIZELIMIT, &slimit ); } @@ -1277,8 +1278,8 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) } slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, e->ep_entry ); } - slapi_sdn_done(&basesdn); - return 0; + rc = 0; + goto bail; } else { @@ -1289,11 +1290,19 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) { /* Failed the filter test, and this isn't a VLV Search */ cache_return( &inst->inst_cache, &(sr->sr_entry) ); + if (LDAP_UNWILLING_TO_PERFORM == filter_test) { + /* Need to catch this error to detect the vattr loop */ + slapi_send_ldap_result( pb, filter_test, NULL, + "Failed the filter test", 0, NULL ); + rc = SLAPI_FAIL_GENERAL; + goto bail; + } } } } - /*NOTREACHED*/ +bail: slapi_sdn_done(&basesdn); + return rc; } @@ -1333,19 +1342,19 @@ ldbm_back_entry_release( Slapi_PBlock *pb, void *backend_info_ptr ) { ldbm_instance *inst; if ( backend_info_ptr == NULL ) - return 1; + return 1; slapi_pblock_get( pb, SLAPI_BACKEND, &be ); - inst = (ldbm_instance *) be->be_instance_info; + inst = (ldbm_instance *) be->be_instance_info; cache_return( &inst->inst_cache, (struct backentry **)&backend_info_ptr ); if( ((struct backentry *) backend_info_ptr)->ep_vlventry != NULL ) { - /* This entry was created during a vlv search whose acl check failed. It needs to be - * freed here */ + /* This entry was created during a vlv search whose acl check failed. It needs to be + * freed here */ slapi_entry_free( ((struct backentry *) backend_info_ptr)->ep_vlventry ); - ((struct backentry *) backend_info_ptr)->ep_vlventry = NULL; + ((struct backentry *) backend_info_ptr)->ep_vlventry = NULL; } return 0; } diff --git a/ldap/servers/slapd/filterentry.c b/ldap/servers/slapd/filterentry.c index 1669bdc3..317c4f24 100644 --- a/ldap/servers/slapd/filterentry.c +++ b/ldap/servers/slapd/filterentry.c @@ -861,7 +861,7 @@ slapi_vattr_filter_test_ext_internal( if ( only_check_access || rc != LDAP_SUCCESS ) { return( rc ); } - rc = vattr_test_filter( e, f, FILTER_TYPE_AVA, f->f_ava.ava_type ); + rc = vattr_test_filter( pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type ); break; case LDAP_FILTER_SUBSTRINGS: @@ -873,7 +873,7 @@ slapi_vattr_filter_test_ext_internal( if ( only_check_access || rc != LDAP_SUCCESS ) { return( rc ); } - rc = vattr_test_filter( e, f, FILTER_TYPE_SUBSTRING, f->f_sub_type); + rc = vattr_test_filter( pb, e, f, FILTER_TYPE_SUBSTRING, f->f_sub_type); break; case LDAP_FILTER_GE: @@ -886,7 +886,7 @@ slapi_vattr_filter_test_ext_internal( if ( only_check_access || rc != LDAP_SUCCESS ) { return( rc ); } - rc = vattr_test_filter( e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); + rc = vattr_test_filter( pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); break; case LDAP_FILTER_LE: @@ -899,7 +899,7 @@ slapi_vattr_filter_test_ext_internal( if ( only_check_access || rc != LDAP_SUCCESS ) { return( rc ); } - rc = vattr_test_filter( e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); + rc = vattr_test_filter( pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); break; case LDAP_FILTER_PRESENT: @@ -911,7 +911,7 @@ slapi_vattr_filter_test_ext_internal( if ( only_check_access || rc != LDAP_SUCCESS ) { return( rc ); } - rc = vattr_test_filter( e, f, FILTER_TYPE_PRES, f->f_type); + rc = vattr_test_filter( pb, e, f, FILTER_TYPE_PRES, f->f_type); break; case LDAP_FILTER_APPROX: @@ -924,7 +924,7 @@ slapi_vattr_filter_test_ext_internal( if ( only_check_access || rc != LDAP_SUCCESS ) { return( rc ); } - rc = vattr_test_filter( e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); + rc = vattr_test_filter( pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); break; case LDAP_FILTER_EXTENDED: diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index 7a6b1374..020514f5 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -1418,6 +1418,7 @@ typedef struct slapi_pblock { /* For password policy control */ int pb_pwpolicy_ctrl; + void *pb_vattr_context; /* hold the vattr_context for roles/cos */ } slapi_pblock; /* The referral element */ diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index 498b68e9..85aa85c5 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -475,7 +475,8 @@ int slapi_vattrcache_iscacheable( const char * type ); void slapi_vattrcache_cache_all(); void slapi_vattrcache_cache_none(); -int vattr_test_filter(/* Entry we're interested in */ Slapi_Entry *e, +int vattr_test_filter( Slapi_PBlock *pb, + /* Entry we're interested in */ Slapi_Entry *e, Slapi_Filter *f, filter_type_t filter_type, char *type); diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c index d18951f5..8e34f305 100644 --- a/ldap/servers/slapd/vattr.c +++ b/ldap/servers/slapd/vattr.c @@ -102,7 +102,7 @@ struct _vattr_context { unsigned int vattr_context_loop_count; unsigned int error_displayed; }; -#define VATTR_LOOP_COUNT_MAX 50 +#define VATTR_LOOP_COUNT_MAX 256 typedef vattr_sp_handle vattr_sp_handle_list; @@ -300,11 +300,19 @@ static int vattr_helper_get_entry_conts_ex(Slapi_Entry *e,const char *type, vatt vattr_context *vattr_context_new( Slapi_PBlock *pb ) { - vattr_context *c = (vattr_context *)slapi_ch_calloc(1, sizeof(vattr_context)); + vattr_context *c = NULL; + if (pb && pb->pb_vattr_context) { + c = (vattr_context *)pb->pb_vattr_context; + } else { + c = (vattr_context *)slapi_ch_calloc(1, sizeof(vattr_context)); + } /* The payload is zero, which is what we want */ if ( c ) { c->pb = pb; } + if ( pb && c != (vattr_context *)pb->pb_vattr_context ) { + pb->pb_vattr_context = (void *)c; + } return c; } @@ -333,15 +341,33 @@ static void vattr_context_ungrok(vattr_context **c) /* Decrement the loop count */ if (0 == vattr_context_unmark(*c)) { /* If necessary, delete the structure */ + if ((*c)->pb) { + (*c)->pb->pb_vattr_context = NULL; + } slapi_ch_free((void **)c); } } +static int vattr_context_grok_pb( Slapi_PBlock *pb, vattr_context **c ) +{ + int rc = -1; + if (NULL == c) { + return rc; + } + *c = vattr_context_new( pb ); + if (NULL == *c) { + return ENOMEM; + } + rc = vattr_context_check(*c); + vattr_context_mark(*c); /* increment loop count */ + return rc; +} + /* Check and mess with the context structure on entry to a vattr sp function */ static int vattr_context_grok(vattr_context **c) { int rc = 0; - /* First check that we've not got into an infinite loop. + /* First check that we've not got into an infinite loop. We do so by means of the vattr_context structure. */ @@ -388,10 +414,12 @@ static int vattr_context_is_loop_msg_displayed(vattr_context **c) * >0 an ldap error code * */ -int vattr_test_filter( /* Entry we're interested in */ Slapi_Entry *e, +int vattr_test_filter( Slapi_PBlock *pb, + /* Entry we're interested in */ Slapi_Entry *e, Slapi_Filter *f, filter_type_t filter_type, - char * type) { + char * type) +{ int rc = -1; int sp_bit = 0; /* Set if an SP supplied an answer */ vattr_sp_handle_list *list = NULL; @@ -445,26 +473,23 @@ int vattr_test_filter( /* Entry we're interested in */ Slapi_Entry *e, char **actual_type_name; int buffer_flags; vattr_get_thang my_get = {0}; - vattr_context ctx; /* bit cacky, but need to make a null terminated lists for now * for the (unimplemented and so fake) batch attribute request */ char *types[2]; void *hint_list[2]; + vattr_context *ctx; + vattr_context_grok_pb( pb, &ctx ); /* get or new context */ types[0] = type; types[1] = 0; hint_list[1] = 0; - /* set up some local context */ - ctx.vattr_context_loop_count=1; - ctx.error_displayed = 0; - for (current_handle = vattr_map_sp_first(list,&hint); current_handle; current_handle = vattr_map_sp_next(current_handle,&hint)) { hint_list[0] = hint; - rc = vattr_call_sp_get_batch_values(current_handle,&ctx,e, + rc = vattr_call_sp_get_batch_values(current_handle,ctx,e, &my_get,types,&results,&type_name_disposition, &actual_type_name,flags,&buffer_flags, hint_list); @@ -474,6 +499,7 @@ int vattr_test_filter( /* Entry we're interested in */ Slapi_Entry *e, break; } } + vattr_context_ungrok(&ctx); if(!sp_bit) { @@ -483,7 +509,6 @@ int vattr_test_filter( /* Entry we're interested in */ Slapi_Entry *e, * but first lets cache the no result */ slapi_entry_vattrcache_merge_sv(e, type, NULL ); - } else { @@ -491,7 +516,7 @@ int vattr_test_filter( /* Entry we're interested in */ Slapi_Entry *e, * A vattr sp supplied an answer. * so turn the value into a Slapi_Attr, pass * to the syntax plugin for comparison. - */ + */ if ( filter_type == FILTER_TYPE_AVA || filter_type == FILTER_TYPE_SUBSTRING ) { @@ -566,14 +591,13 @@ int vattr_test_filter( /* Entry we're interested in */ Slapi_Entry *e, slapi_ch_free((void**)&type_name_disposition); } } - break; - } + } }/* switch */ } /* If no SP supplied the answer, take it from the entry */ - if (!sp_bit) - { + if (rc <= 1 && !sp_bit) /* if LDAP ERROR is set, skip further testing */ + { int acl_test_done; if ( filter_type == FILTER_TYPE_AVA ) { @@ -597,7 +621,7 @@ int vattr_test_filter( /* Entry we're interested in */ Slapi_Entry *e, 0 /* do test filter */, &acl_test_done); } - } + } return rc; } /* @@ -1690,7 +1714,7 @@ int vattr_call_sp_get_batch_values(vattr_sp_handle *handle, vattr_context *c, Sl *actual_type_name = (char**)slapi_ch_calloc(2, sizeof(*actual_type_name)); ret =((handle->sp->sp_get_fn)(handle,c,e,*type,*results,*type_name_disposition,*actual_type_name,flags,buffer_flags, hint)); - if(ret) + if (ret) { slapi_ch_free((void**)results ); slapi_ch_free((void**)type_name_disposition ); @@ -2332,6 +2356,16 @@ void vattrcache_entry_WRITE_UNLOCK(const Slapi_Entry *e){ PR_RWLock_Unlock(e->e_virtual_lock); } +Slapi_PBlock * +slapi_vattr_get_pblock_from_context(vattr_context *c) +{ + if (c) { + return c->pb; + } else { + return NULL; + } +} + #ifdef VATTR_TEST_CODE /* Prototype SP begins here */ diff --git a/ldap/servers/slapd/vattr_spi.h b/ldap/servers/slapd/vattr_spi.h index 50d75901..a7a670b3 100644 --- a/ldap/servers/slapd/vattr_spi.h +++ b/ldap/servers/slapd/vattr_spi.h @@ -88,4 +88,6 @@ int slapi_vattr_values_get_sp_ex(vattr_context *c, /* Entry we're interested in int slapi_vattr_namespace_values_get_sp(vattr_context *c, /* Entry we're interested in */ Slapi_Entry *e, /* backend namespace dn */ Slapi_DN *namespace_dn, /* attr type name */ char *type, /* pointer to result set */ Slapi_ValueSet*** results,int **type_name_disposition, char ***actual_type_name, int flags, int *free_flags, int *subtype_count); int slapi_vattr_value_compare_sp(vattr_context *c, Slapi_Entry *e,char *type, Slapi_Value *test_this, int *result, int flags); int slapi_vattr_namespace_value_compare_sp(vattr_context *c,/* Entry we're interested in */ Slapi_Entry *e, /* backend namespace dn*/Slapi_DN *namespace_dn, /* attr type name */ const char *type, Slapi_Value *test_this,/* pointer to result */ int *result, int flags); +Slapi_PBlock *slapi_vattr_get_pblock_from_context( vattr_context *c ); + |