Author: nhosoi
Update of /cvs/dirsec/ldapserver/ldap/servers/slapd
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv27588/slapd
Modified Files:
slap.h slapi-private.h vattr_spi.h filterentry.c vattr.c
Log Message:
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.
Index: slap.h
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/slap.h,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- slap.h 2 Oct 2007 18:39:50 -0000 1.27
+++ slap.h 12 Oct 2007 18:03:42 -0000 1.28
@@ -1418,6 +1418,7 @@
/* For password policy control */
int pb_pwpolicy_ctrl;
+ void *pb_vattr_context; /* hold the vattr_context for roles/cos */
} slapi_pblock;
/* The referral element */
Index: slapi-private.h
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/slapi-private.h,v
retrieving revision 1.17
retrieving revision 1.18
diff -u -r1.17 -r1.18
--- slapi-private.h 5 Oct 2007 23:31:07 -0000 1.17
+++ slapi-private.h 12 Oct 2007 18:03:42 -0000 1.18
@@ -475,7 +475,8 @@
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);
Index: vattr_spi.h
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/vattr_spi.h,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- vattr_spi.h 10 Nov 2006 23:45:40 -0000 1.5
+++ vattr_spi.h 12 Oct 2007 18:03:42 -0000 1.6
@@ -88,4 +88,6 @@
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 );
+
Index: filterentry.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/filterentry.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- filterentry.c 10 Nov 2006 23:45:40 -0000 1.5
+++ filterentry.c 12 Oct 2007 18:03:42 -0000 1.6
@@ -861,7 +861,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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:
Index: vattr.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/vattr.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- vattr.c 10 Nov 2006 23:45:40 -0000 1.6
+++ vattr.c 12 Oct 2007 18:03:42 -0000 1.7
@@ -102,7 +102,7 @@
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 @@
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 @@
/* 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 @@
* >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 @@
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 @@
break;
}
}
+ vattr_context_ungrok(&ctx);
if(!sp_bit)
{
@@ -483,7 +509,6 @@
* but first lets cache the no result
*/
slapi_entry_vattrcache_merge_sv(e, type, NULL );
-
}
else
{
@@ -491,7 +516,7 @@
* 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 @@
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 @@
0 /* do test filter */,
&acl_test_done);
}
- }
+ }
return rc;
}
/*
@@ -1690,7 +1714,7 @@
*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 @@
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 */