From 82ce3710428bcb769c6a360ae7e07510c5394571 Mon Sep 17 00:00:00 2001 From: nturpin Date: Tue, 27 Dec 2011 21:31:53 +0100 Subject: [PATCH] Ticket #3: acl cache overflown problem 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 --- ldap/servers/plugins/acl/acl.c | 14 ++-- ldap/servers/plugins/acl/acl.h | 23 +++++-- ldap/servers/plugins/acl/acl_ext.c | 120 ++++++++++++++++++++++++++++++++- ldap/servers/plugins/acl/acllist.c | 10 ++-- ldap/servers/plugins/acl/aclplugin.c | 2 +- 5 files changed, 146 insertions(+), 23 deletions(-) diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c index 9c3db10..c8a221e 100644 --- a/ldap/servers/plugins/acl/acl.c +++ b/ldap/servers/plugins/acl/acl.c @@ -1847,7 +1847,7 @@ 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 ) { + 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++; } @@ -2487,8 +2487,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; @@ -2503,7 +2503,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++; } @@ -2882,7 +2882,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++; @@ -3086,7 +3086,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++; @@ -4042,7 +4042,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 8747de1..e838196 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-pluginarg-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 ) @@ -825,6 +835,7 @@ void acl_set_authorization_dn( Slapi_PBlock *pb, char *dn, int type ); 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 df7d1ed..b61683c 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 ( &(aclcb->aclcb_eval_context.acle_handles_matched_target)); aclcb->aclcb_lock = NULL; slapi_ch_free ( (void **) &aclcb ); @@ -419,6 +424,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. @@ -432,6 +452,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 */, @@ -441,6 +462,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; @@ -471,6 +500,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. * */ @@ -646,16 +709,65 @@ 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: - if (aclpb->aclpb_acleval) ACL_EvalDestroy(NULL, NULL, aclpb->aclpb_acleval); - if (aclpb->aclpb_proplist) PListDestroy(aclpb->aclpb_proplist); - slapi_ch_free((void**)&aclpb); + 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 */ void acl_init_aclpb ( Slapi_PBlock *pb , Acl_PBlock *aclpb, const char *ndn, int copy_from_aclcb) diff --git a/ldap/servers/plugins/acl/acllist.c b/ldap/servers/plugins/acl/acllist.c index 7e5221d..8eb39b9 100644 --- a/ldap/servers/plugins/acl/acllist.c +++ b/ldap/servers/plugins/acl/acllist.c @@ -646,7 +646,7 @@ acllist_init_scan (Slapi_PBlock *pb, int scope, const char *base) root = (AciContainer *) avl_find(acllistRoot, (caddr_t) aclpb->aclpb_aclContainer, (IFP) __acllist_aciContainer_node_cmp); - 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; @@ -673,7 +673,7 @@ acllist_init_scan (Slapi_PBlock *pb, int scope, const 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++; } } @@ -711,7 +711,7 @@ acllist_aciscan_update_scan ( Acl_PBlock *aclpb, char *edn ) is_not_search_base = 0; } for (index = 0; (aclpb->aclpb_base_handles_index[index] != -1) && - (index < ACLPB_MAX_SELECTED_ACLS - 2); index++) ; + (index < aclpb_max_selected_acls - 2); index++) ; memcpy(aclpb->aclpb_handles_index, aclpb->aclpb_base_handles_index, sizeof(*aclpb->aclpb_handles_index) * index); } @@ -745,7 +745,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; @@ -835,7 +835,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 ecd1e11..0d35425 100644 --- a/ldap/servers/plugins/acl/aclplugin.c +++ b/ldap/servers/plugins/acl/aclplugin.c @@ -300,7 +300,7 @@ aclplugin_stop ( Slapi_PBlock *pb ) { int rc = 0; /* OK */ - /* nothing to be done now */ + acl_destroy_aclpb_pool (); return rc; } -- 1.7.7.1.msysgit.0