[Fedora-directory-commits] ldapserver/ldap/servers/slapd slap.h, 1.27, 1.28 slapi-private.h, 1.17, 1.18 vattr_spi.h, 1.5, 1.6 filterentry.c, 1.5, 1.6 vattr.c, 1.6, 1.7

[Noriko Hosoi (nhosoi)]

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 */