From 41f95267697295a3b9837cab1e049411c9b792b1 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Mar 06 2012 23:21:49 +0000 Subject: Ticket #3: acl cache overflown problem Bug 782414 - acl cache overflown problem https://bugzilla.redhat.com/show_bug.cgi?id=782414 Fix Description: We have made ACLPB_MAX_SELECTED_ACLS and ACLPB_MAX_CACHE_RESULTS configurable. Their default value is still 200 (same as before). To modify this value, you can add or modify the attribute "nsslapd-pluginarg-aclpb-max-selected-acls" in the ACL plugin config entry "cn=ACL Plugin,cn=plugins,cn=config". - The constants were replaced with variables (same name in lower case) - On init: the variables are initialized with the value contained in the mentioned attribute, if it exists in config. Otherwise they are set to the default value. - The arrays that depend on these values are now dynamically allocated - On init: acl__malloc_aclpb ( ) - On pre-operation: acl_conn_ext_constructor ( ... ) - The memory is freed: - On shutdown: acl_destroy_aclpb_pool() - On post-operation: acl_conn_ext_destructor ( ... ) - I also free the space for aclQueue in acl_destroy_aclpb_pool(), since it seems it is not done anywhere. Platforms tested: Fedora 16 (cherry picked from commit 0070a45411ad8909389418b868638d08682f701c) --- diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c index 3b9f057..77f8f0a 100644 --- a/ldap/servers/plugins/acl/acl.c +++ b/ldap/servers/plugins/acl/acl.c @@ -1825,8 +1825,8 @@ acl__scan_for_acis(Acl_PBlock *aclpb, int *err) if ( aclpb->aclpb_state & ACLPB_SEARCH_BASED_ON_LIST || aclpb->aclpb_handles_index[0] != -1 ) { int kk = 0; - while ( kk < ACLPB_MAX_SELECTED_ACLS && aclpb->aclpb_handles_index[kk] != -1 ) { - slapi_log_error(SLAPI_LOG_ACL, plugin_name, "Using ACL Cointainer:%d for evaluation\n", kk); + while ( kk < aclpb_max_selected_acls && aclpb->aclpb_handles_index[kk] != -1 ) { + slapi_log_error(SLAPI_LOG_ACL, plugin_name, "Using ACL Container:%d for evaluation\n", kk); kk++; } } @@ -2467,8 +2467,8 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int * ((aci->aci_access & SLAPI_ACL_READ) || (aci->aci_access & SLAPI_ACL_SEARCH))) { int kk=0; - while ( kk < ACLPB_MAX_SELECTED_ACLS && aclpb->aclpb_handles_index[kk] >=0 ) kk++; - if (kk >= ACLPB_MAX_SELECTED_ACLS) { + while ( kk < aclpb_max_selected_acls && aclpb->aclpb_handles_index[kk] >=0 ) kk++; + if (kk >= aclpb_max_selected_acls) { aclpb->aclpb_state &= ~ACLPB_SEARCH_BASED_ON_ENTRY_LIST; } else { aclpb->aclpb_handles_index[kk++] = aci->aci_index; @@ -2483,7 +2483,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int * aclEvalContext *c_evalContext = &aclpb->aclpb_curr_entryEval_context; int nhandle = c_evalContext->acle_numof_tmatched_handles; - if ( nhandle < ACLPB_MAX_SELECTED_ACLS) { + if ( nhandle < aclpb_max_selected_acls) { c_evalContext->acle_handles_matched_target[nhandle] = aci->aci_index; c_evalContext->acle_numof_tmatched_handles++; } @@ -2854,7 +2854,7 @@ acl__TestRights(Acl_PBlock *aclpb,int access, char **right, char ** map_generic, if ( j < aclpb->aclpb_last_cache_result) { /* already in cache */ - } else if ( j < ACLPB_MAX_CACHE_RESULTS ) { + } else if ( j < aclpb_max_cache_results ) { /* j == aclpb->aclpb_last_cache_result && j < ACLPB_MAX_CACHE_RESULTS */ aclpb->aclpb_last_cache_result++; @@ -3054,7 +3054,7 @@ acl__TestRights(Acl_PBlock *aclpb,int access, char **right, char ** map_generic, if ( j < aclpb->aclpb_last_cache_result) { /* already in cache */ - } else if ( j < ACLPB_MAX_CACHE_RESULTS ) { + } else if ( j < aclpb_max_cache_results ) { /* j == aclpb->aclpb_last_cache_result && j < ACLPB_MAX_CACHE_RESULTS */ aclpb->aclpb_last_cache_result++; @@ -4010,7 +4010,7 @@ acl__recompute_acl ( Acl_PBlock *aclpb, if ( j < aclpb->aclpb_last_cache_result) { /* already in cache */ - } else if ( j < ACLPB_MAX_CACHE_RESULTS-1) { + } else if ( j < aclpb_max_cache_results-1) { /* rbyrneXXX: make this same as other last_cache_result code! */ j = ++aclpb->aclpb_last_cache_result; aclpb->aclpb_cache_result[j].aci_index = aci->aci_index; diff --git a/ldap/servers/plugins/acl/acl.h b/ldap/servers/plugins/acl/acl.h index 3f4b4e6..e3dda08 100644 --- a/ldap/servers/plugins/acl/acl.h +++ b/ldap/servers/plugins/acl/acl.h @@ -339,8 +339,18 @@ typedef struct aci { #define ACI_MAX_ELEVEL ACI_ELEVEL_GROUPDN +1 #define ACI_DEFAULT_ELEVEL ACI_MAX_ELEVEL +#define ACL_PLUGIN_CONFIG_ENTRY_DN "cn=ACL Plugin,cn=plugins,cn=config" -#define ACLPB_MAX_SELECTED_ACLS 200 +/* + * In plugin config entry, set this attribute to change the value + * of aclpb_max_selected_acls and aclpb_max_cache_results. + * If not set, DEFAULT_ACLPB_MAX_SELECTED_ACLS will be used. + */ +#define ATTR_ACLPB_MAX_SELECTED_ACLS "nsslapd-aclpb-max-selected-acls" +#define DEFAULT_ACLPB_MAX_SELECTED_ACLS 200 + +int aclpb_max_selected_acls; /* initialized from plugin config entry */ +int aclpb_max_cache_results; /* initialized from plugin config entry */ typedef struct result_cache { int aci_index; @@ -353,7 +363,7 @@ typedef struct result_cache { #define ACLPB_CACHE_SEARCH_RES_SKIP (short)0x0010 /* used for both types */ #define ACLPB_CACHE_READ_RES_SKIP (short)0x0020 /* used for both types */ }r_cache_t; -#define ACLPB_MAX_CACHE_RESULTS ACLPB_MAX_SELECTED_ACLS + /* * This is use to keep the result of the evaluation of the attr. @@ -392,7 +402,7 @@ struct acleval_context { /* Handles information */ short acle_numof_tmatched_handles; - int acle_handles_matched_target[ACLPB_MAX_SELECTED_ACLS]; + int *acle_handles_matched_target; }; typedef struct acleval_context aclEvalContext; @@ -539,12 +549,12 @@ struct acl_pblock { /* To keep the results in the cache */ int aclpb_last_cache_result; - struct result_cache aclpb_cache_result[ACLPB_MAX_CACHE_RESULTS]; + struct result_cache *aclpb_cache_result; /* Index numbers of ACLs selected based on a locality search*/ char *aclpb_search_base; - int aclpb_base_handles_index[ACLPB_MAX_SELECTED_ACLS]; - int aclpb_handles_index[ACLPB_MAX_SELECTED_ACLS]; + int *aclpb_base_handles_index; + int *aclpb_handles_index; /* Evaluation context info ** 1) Context cached from aclcb ( from connection struct ) @@ -827,6 +837,7 @@ int acl_get_proxyauth_dn( Slapi_PBlock *pb, char **proxydnp, void acl_init_aclpb ( Slapi_PBlock *pb , Acl_PBlock *aclpb, const char *dn, int copy_from_aclcb); int acl_create_aclpb_pool (); +void acl_destroy_aclpb_pool (); int acl_skip_access_check ( Slapi_PBlock *pb, Slapi_Entry *e ); int aclext_alloc_lockarray (); diff --git a/ldap/servers/plugins/acl/acl_ext.c b/ldap/servers/plugins/acl/acl_ext.c index d9494ec..644fc79 100644 --- a/ldap/servers/plugins/acl/acl_ext.c +++ b/ldap/servers/plugins/acl/acl_ext.c @@ -50,6 +50,7 @@ static char * acl__get_aclpb_type ( Acl_PBlock *aclpb ); static Acl_PBlock * acl__get_aclpb_from_pool ( ); static int acl__put_aclpb_back_to_pool ( Acl_PBlock *aclpb ); static Acl_PBlock * acl__malloc_aclpb ( ); +static void acl__free_aclpb ( Acl_PBlock **aclpb_ptr); static PRLock *aclext_get_lock (); @@ -198,6 +199,9 @@ acl_conn_ext_constructor ( void *object, void *parent ) ext->aclcb_sdn = slapi_sdn_new (); /* store the signatures */ ext->aclcb_aclsignature = acl_get_aclsignature(); + /* eval_context */ + ext->aclcb_eval_context.acle_handles_matched_target = (int *) + slapi_ch_calloc (aclpb_max_selected_acls, sizeof (int)); ext->aclcb_state = -1; return ext; @@ -215,6 +219,7 @@ acl_conn_ext_destructor ( void *ext, void *object, void *parent ) shared_lock = aclcb->aclcb_lock; acl_clean_aclEval_context ( &aclcb->aclcb_eval_context, 0 /* clean*/ ); slapi_sdn_free ( &aclcb->aclcb_sdn ); + slapi_ch_free ( (void **)&(aclcb->aclcb_eval_context.acle_handles_matched_target)); aclcb->aclcb_lock = NULL; slapi_ch_free ( (void **) &aclcb ); @@ -414,6 +419,21 @@ acl__handle_config_entry (Slapi_Entry *e, void *callback_data ) return 0; } +static int +acl__handle_plugin_config_entry (Slapi_Entry *e, void *callback_data ) +{ + int value = slapi_entry_attr_get_int(e, ATTR_ACLPB_MAX_SELECTED_ACLS); + if (value) { + aclpb_max_selected_acls = value; + aclpb_max_cache_results = value; + } else { + aclpb_max_selected_acls = DEFAULT_ACLPB_MAX_SELECTED_ACLS; + aclpb_max_cache_results = DEFAULT_ACLPB_MAX_SELECTED_ACLS; + } + + return 0; +} + /* * Create a pool of acl pblock. Created during the ACL plugin * initialization. @@ -427,6 +447,7 @@ acl_create_aclpb_pool () Acl_PBlock *first_aclpb; int i; int maxThreads= 0; + int callbackData= 0; slapi_search_internal_callback( "cn=config", LDAP_SCOPE_BASE, "(objectclass=*)", NULL, 0 /* attrsonly */, @@ -436,6 +457,14 @@ acl_create_aclpb_pool () acl__handle_config_entry, NULL /* referral_callback */); + slapi_search_internal_callback( ACL_PLUGIN_CONFIG_ENTRY_DN, LDAP_SCOPE_BASE, "(objectclass=*)", + NULL, 0 /* attrsonly */, + &callbackData /* callback_data, not used in this case */, + NULL /* controls */, + NULL /* result_callback */, + acl__handle_plugin_config_entry, + NULL /* referral_callback */); + /* Create a pool pf aclpb */ maxThreads = 2 * maxThreads; @@ -466,6 +495,40 @@ acl_create_aclpb_pool () } /* + * Destroys the Acl_PBlock pool. To be called at shutdown, + * from function registered as SLAPI_PLUGIN_CLOSE_FN + */ +void +acl_destroy_aclpb_pool () +{ + Acl_PBlock *currentPbBlock; + Acl_PBlock *nextPbBlock; + + if (!aclQueue) { + /* Nothing to do */ + return; + } + + /* Free all busy pbBlocks in queue */ + currentPbBlock = aclQueue->aclq_busy; + while (currentPbBlock) { + nextPbBlock = currentPbBlock->aclpb_next; + acl__free_aclpb(¤tPbBlock); + currentPbBlock = nextPbBlock; + } + + /* Free all free pbBlocks in queue */ + currentPbBlock = aclQueue->aclq_free; + while (currentPbBlock) { + nextPbBlock = currentPbBlock->aclpb_next; + acl__free_aclpb(¤tPbBlock); + currentPbBlock = nextPbBlock; + } + + slapi_ch_free((void**)&aclQueue); +} + +/* * Get a FREE acl pblock from the pool. * */ @@ -562,33 +625,33 @@ acl__malloc_aclpb ( ) if ((aclpb->aclpb_proplist = PListNew(NULL)) == NULL) { slapi_log_error (SLAPI_LOG_FATAL, plugin_name, "Unable to allocate the aclprop PList\n"); - return NULL; + goto error; } if (PListInitProp(aclpb->aclpb_proplist, 0, DS_PROP_ACLPB, aclpb, 0) < 0) { slapi_log_error(SLAPI_LOG_FATAL, plugin_name, "Unable to set the ACL PBLOCK in the Plist\n"); - return NULL; + goto error; } if (PListInitProp(aclpb->aclpb_proplist, 0, DS_ATTR_USERDN, aclpb, 0) < 0) { slapi_log_error(SLAPI_LOG_FATAL, plugin_name, "Unable to set the USER DN in the Plist\n"); - return NULL; + goto error; } if (PListInitProp(aclpb->aclpb_proplist, 0, DS_ATTR_AUTHTYPE, aclpb, 0) < 0) { slapi_log_error(SLAPI_LOG_FATAL, plugin_name, "Unable to set the AUTH TYPE in the Plist\n"); - return NULL; + goto error; } if (PListInitProp(aclpb->aclpb_proplist, 0, DS_ATTR_ENTRY, aclpb, 0) < 0) { slapi_log_error(SLAPI_LOG_FATAL, plugin_name, "Unable to set the ENTRY TYPE in the Plist\n"); - return NULL; + goto error; } if (PListInitProp(aclpb->aclpb_proplist, 0, DS_ATTR_SSF, aclpb, 0) < 0) { slapi_log_error(SLAPI_LOG_FATAL, plugin_name, "Unable to set the SSF in the Plist\n"); - return NULL; + goto error; } /* @@ -603,7 +666,7 @@ acl__malloc_aclpb ( ) if (aclpb->aclpb_acleval == NULL) { slapi_log_error(SLAPI_LOG_FATAL, plugin_name, "Unable to allocate the acleval block\n"); - return NULL; + goto error; } /* * This is a libaccess routine. @@ -641,8 +704,63 @@ acl__malloc_aclpb ( ) /* hash table to store macro matched values from targets */ aclpb->aclpb_macro_ht = acl_ht_new(); - return aclpb; + /* allocate arrays for handles */ + aclpb->aclpb_handles_index = (int *) + slapi_ch_calloc (aclpb_max_selected_acls, sizeof (int)); + aclpb->aclpb_base_handles_index = (int *) + slapi_ch_calloc (aclpb_max_selected_acls, sizeof (int)); + + /* allocate arrays for result cache */ + aclpb->aclpb_cache_result = (r_cache_t *) + slapi_ch_calloc (aclpb_max_cache_results, sizeof (r_cache_t)); + + /* allocate arrays for target handles in eval_context */ + aclpb->aclpb_curr_entryEval_context.acle_handles_matched_target = (int *) + slapi_ch_calloc (aclpb_max_selected_acls, sizeof (int)); + aclpb->aclpb_prev_entryEval_context.acle_handles_matched_target = (int *) + slapi_ch_calloc (aclpb_max_selected_acls, sizeof (int)); + aclpb->aclpb_prev_opEval_context.acle_handles_matched_target = (int *) + slapi_ch_calloc (aclpb_max_selected_acls, sizeof (int)); + + return aclpb; + +error: + acl__free_aclpb(&aclpb); + + return NULL; +} + +/* + * Free the acl pb. To be used at shutdown (SLAPI_PLUGIN_CLOSE_FN) + * when we free the aclQueue + */ +static void +acl__free_aclpb ( Acl_PBlock **aclpb_ptr) +{ + Acl_PBlock *aclpb = NULL; + + if (aclpb_ptr == NULL || *aclpb_ptr == NULL) + return; // Nothing to do + + aclpb = *aclpb_ptr; + + if (aclpb->aclpb_acleval) + ACL_EvalDestroy(NULL, NULL, aclpb->aclpb_acleval); + + if (aclpb->aclpb_proplist) + PListDestroy(aclpb->aclpb_proplist); + + slapi_ch_free((void**)&(aclpb->aclpb_handles_index)); + slapi_ch_free((void**)&(aclpb->aclpb_base_handles_index)); + slapi_ch_free((void**)&(aclpb->aclpb_cache_result)); + slapi_ch_free((void**) + &(aclpb->aclpb_curr_entryEval_context.acle_handles_matched_target)); + slapi_ch_free((void**) + &(aclpb->aclpb_prev_entryEval_context.acle_handles_matched_target)); + slapi_ch_free((void**) + &(aclpb->aclpb_prev_opEval_context.acle_handles_matched_target)); + slapi_ch_free((void**)aclpb_ptr); } /* Initializes the aclpb */ diff --git a/ldap/servers/plugins/acl/acllist.c b/ldap/servers/plugins/acl/acllist.c index 755ce64..062d998 100644 --- a/ldap/servers/plugins/acl/acllist.c +++ b/ldap/servers/plugins/acl/acllist.c @@ -642,10 +642,10 @@ acllist_init_scan (Slapi_PBlock *pb, int scope, char *base) slapi_sdn_set_ndn_byref ( aclpb->aclpb_aclContainer->acic_sdn, basedn ); - root = (AciContainer *) avl_find( acllistRoot, - (caddr_t) aclpb->aclpb_aclContainer, - (IFP) __acllist_aciContainer_node_cmp); - if ( index >= ACLPB_MAX_SELECTED_ACLS -2 ) { + root = (AciContainer *) avl_find(acllistRoot, + (caddr_t) aclpb->aclpb_aclContainer, + (IFP) __acllist_aciContainer_node_cmp); + if ( index >= aclpb_max_selected_acls -2 ) { aclpb->aclpb_handles_index[0] = -1; slapi_ch_free ( (void **) &basedn); break; @@ -666,7 +666,7 @@ acllist_init_scan (Slapi_PBlock *pb, int scope, char *base) acllist_acicache_READ_UNLOCK(); i = 0; - while ( i < ACLPB_MAX_SELECTED_ACLS && aclpb->aclpb_base_handles_index[i] != -1 ) { + while ( i < aclpb_max_selected_acls && aclpb->aclpb_base_handles_index[i] != -1 ) { i++; } } @@ -703,6 +703,10 @@ acllist_aciscan_update_scan ( Acl_PBlock *aclpb, char *edn ) if ( strcasecmp ( edn, aclpb->aclpb_search_base) == 0) { is_not_search_base = 0; } + for (index = 0; (aclpb->aclpb_base_handles_index[index] != -1) && + (index < aclpb_max_selected_acls - 2); index++) ; + memcpy(aclpb->aclpb_handles_index, aclpb->aclpb_base_handles_index, + sizeof(*aclpb->aclpb_handles_index) * index); } aclpb->aclpb_handles_index[index] = -1; @@ -734,7 +738,7 @@ acllist_aciscan_update_scan ( Acl_PBlock *aclpb, char *edn ) slapi_log_error ( SLAPI_LOG_ACL, plugin_name, "Searching AVL tree for update:%s: container:%d\n", basedn , root ? root->acic_index: -1); - if ( index >= ACLPB_MAX_SELECTED_ACLS -2 ) { + if ( index >= aclpb_max_selected_acls -2 ) { aclpb->aclpb_handles_index[0] = -1; slapi_ch_free ( (void **) &basedn); break; @@ -829,7 +833,7 @@ start: return NULL; /* reached the end of the array */ - if ((!scan_entire_list && (*cookie >= (ACLPB_MAX_SELECTED_ACLS-1))) || + if ((!scan_entire_list && (*cookie >= (aclpb_max_selected_acls-1))) || (*cookie >= currContainerIndex)) { return NULL; } diff --git a/ldap/servers/plugins/acl/aclplugin.c b/ldap/servers/plugins/acl/aclplugin.c index 12c435d..fa0e135 100644 --- a/ldap/servers/plugins/acl/aclplugin.c +++ b/ldap/servers/plugins/acl/aclplugin.c @@ -292,7 +292,7 @@ aclplugin_stop ( Slapi_PBlock *pb ) { int rc = 0; /* OK */ - /* nothing to be done now */ + acl_destroy_aclpb_pool (); return rc; }