Import add/delete/modify fixes from devel.

Define SLAPD_CHILD_MODIFICATION_WITH_ENTRY_ACL in back-ldbm to
keep delete behavior from changing.  Should be removed in next release.

servers/slapd/back-ldbm/add.c

@@ -20,14 +20,16 @@ ldbm_back_add(
 )
 {
 	struct ldbminfo	*li = (struct ldbminfo *) be->be_private;
-	char		*dn = NULL, *pdn = NULL;
+	char		*dn = NULL, *pdn;
 	Entry		*p = NULL;
-	int			rc;
+	int			rootlock = 0;
+	int			rc = -1; 
 
 	dn = dn_normalize( ch_strdup( e->e_dn ) );
 
 	Debug(LDAP_DEBUG_ARGS, "==> ldbm_back_add: %s\n", dn, 0, 0);
 
+	/* nobody else can add until we lock our parent */
 	pthread_mutex_lock(&li->li_add_mutex);
 
 	if ( ( dn2id( be, dn ) ) != NOID ) {
@@ -58,11 +60,10 @@ ldbm_back_add(
 	 */
 
 	if ( (pdn = dn_parent( be, dn )) != NULL ) {
-		char *matched;
-		/* no parent */
+		char *matched = NULL;
 
-		/* get entry with reader lock */
-		if ( (p = dn2entry_r( be, pdn, &matched )) == NULL ) {
+		/* get parent with writer lock */
+		if ( (p = dn2entry_w( be, pdn, &matched )) == NULL ) {
 			pthread_mutex_unlock(&li->li_add_mutex);
 			Debug( LDAP_DEBUG_TRACE, "parent does not exist\n", 0,
 			    0, 0 );
@@ -75,24 +76,37 @@ ldbm_back_add(
 
 			entry_free( e );
 			free( dn );
+			free( pdn );
 			return -1;
 		}
 
+		/* don't need the add lock anymore */
+		pthread_mutex_unlock(&li->li_add_mutex);
+
+		free(pdn);
+
+		if ( matched != NULL ) {
+			free( matched );
+		}
+
 		if ( ! access_allowed( be, conn, op, p, "children", NULL,
 		    op->o_dn, ACL_WRITE ) )
 		{
-			pthread_mutex_unlock(&li->li_add_mutex);
 			Debug( LDAP_DEBUG_TRACE, "no access to parent\n", 0,
 			    0, 0 );
 			send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
 			    "", "" );
 
+			/* free parent and writer lock */
+			cache_return_entry_w( &li->li_cache, p ); 
+
 			entry_free( e );
 			free( dn );
 			return -1;
 		}
 
 	} else {
+		/* no parent, must be adding entry to root */
 		if ( ! be_isroot( be, op->o_dn ) ) {
 			pthread_mutex_unlock(&li->li_add_mutex);
 			Debug( LDAP_DEBUG_TRACE, "no parent & not root\n", 0,
@@ -104,6 +118,14 @@ ldbm_back_add(
 			free( dn );
 			return -1;
 		}
+
+		/*
+		 * no parent, acquire the root write lock
+		 * and release the add lock.
+		 */
+		pthread_mutex_lock(&li->li_root_mutex);
+		pthread_mutex_unlock(&li->li_add_mutex);
+		rootlock=1;
 	}
 
 	/*
@@ -114,7 +136,13 @@ ldbm_back_add(
 
 	e->e_id = next_id( be );
 	if ( cache_add_entry_lock( &li->li_cache, e, ENTRY_STATE_CREATING ) != 0 ) {
-		pthread_mutex_unlock(&li->li_add_mutex);
+		if( p != NULL) {
+			/* free parent and writer lock */
+			cache_return_entry_w( &li->li_cache, p ); 
+		} else if ( rootlock ) {
+			/* release root lock */
+			pthread_mutex_unlock(&li->li_root_mutex);
+		}
 
 		Debug( LDAP_DEBUG_ANY, "cache_add_entry_lock failed\n", 0, 0,
 		    0 );
@@ -129,6 +157,9 @@ ldbm_back_add(
 		return( -1 );
 	}
 
+	/* acquire writer lock */
+	entry_rdwr_lock(e, 1);
+
 	/*
 	 * add it to the id2children index for the parent
 	 */
@@ -138,7 +169,6 @@ ldbm_back_add(
 		    0, 0 );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
 
-		rc = -1;
 		goto return_results;
 	}
 
@@ -153,7 +183,6 @@ ldbm_back_add(
 		    0, 0 );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
 
-		rc = -1;
 		goto return_results;
 	}
 
@@ -163,13 +192,9 @@ ldbm_back_add(
 		    0, 0 );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
 
-		rc = -1;
 		goto return_results;
 	}
 
-	/* acquire writer lock */
-	entry_rdwr_lock(e, 1);
-
 	/* id2entry index */
 	if ( id2entry_add( be, e ) != 0 ) {
 		Debug( LDAP_DEBUG_TRACE, "id2entry_add failed\n", 0,
@@ -177,7 +202,6 @@ ldbm_back_add(
 		(void) dn2id_delete( be, dn );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
 
-		rc = -1;
 		goto return_results;
 	}
 
@@ -185,24 +209,22 @@ ldbm_back_add(
 	rc = 0;
 
 return_results:;
-
 	if ( dn != NULL )
 		free( dn );
-	if ( pdn != NULL )
-		free( pdn );
 
 	cache_set_state( &li->li_cache, e, 0 );
 
-	/* free entry and writer lock */
-	cache_return_entry_w( &li->li_cache, e ); 
-
-	/* free entry and reader lock */
 	if (p != NULL) {
-		cache_return_entry_r( &li->li_cache, p ); 
+		/* free parent and writer lock */
+		cache_return_entry_w( &li->li_cache, p ); 
+
+	} else if ( rootlock ) {
+		/* release root lock */
+		pthread_mutex_unlock(&li->li_root_mutex);
 	}
 
-	/* it might actually be okay to release this lock sooner */
-	pthread_mutex_unlock(&li->li_add_mutex);
+	/* free entry and writer lock */
+	cache_return_entry_w( &li->li_cache, e ); 
 
 	return( rc );
 }

servers/slapd/back-ldbm/back-ldbm.h

@@ -9,6 +9,9 @@ LDAP_BEGIN_DECL
 
 #define DEFAULT_CACHE_SIZE	1000
 
+/* SHOULD BE REMOVED IN NEXT RELEASE */
+#define SLAPD_CHILD_MODIFICATION_WITH_ENTRY_ACL 1
+
 #ifdef HAVE_BERKELEY_DB2
 #	define DEFAULT_DBCACHE_SIZE (100 * DEFAULT_DB_PAGE_SIZE)
 #else
@@ -106,6 +109,7 @@ struct attrinfo {
 
 struct ldbminfo {
 	ID			li_nextid;
+	pthread_mutex_t		li_root_mutex;
 	pthread_mutex_t		li_add_mutex;
 	pthread_mutex_t		li_nextid_mutex;
 	int			li_mode;

servers/slapd/back-ldbm/delete.c

@@ -20,9 +20,11 @@ ldbm_back_delete(
 )
 {
 	struct ldbminfo	*li = (struct ldbminfo *) be->be_private;
-	char		*matched;
-        char            *pdn = NULL;
-	Entry		*e, *p = NULL;
+	char	*matched = NULL;
+	char	*pdn = NULL;
+	Entry	*e, *p = NULL;
+	int rootlock = 0;
+	int	rc = -1;
 
 	Debug(LDAP_DEBUG_ARGS, "==> ldbm_back_delete: %s\n", dn, 0, 0);
 
@@ -48,29 +50,67 @@ ldbm_back_delete(
 			dn, 0, 0);
 		send_ldap_result( conn, op, LDAP_NOT_ALLOWED_ON_NONLEAF, "",
 		    "" );
-		goto error_return;
+		goto return_results;
 	}
 
+#ifdef SLAPD_CHILD_MODIFICATION_WITH_ENTRY_ACL
 	if ( ! access_allowed( be, conn, op, e, "entry", NULL, op->o_dn,
 	    ACL_WRITE ) ) {
 		Debug(LDAP_DEBUG_ARGS,
 			"<=- ldbm_back_delete: insufficient access %s\n",
 			dn, 0, 0);
 		send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS, "", "" );
-		goto error_return;
+		goto return_results;
 	}
+#endif
 
 	Debug (LDAP_DEBUG_TRACE,
 		"rdwr_Xchk: readers_reading: %d writer_writing: %d\n",
 		e->e_rdwr.readers_reading, e->e_rdwr.writer_writing, 0);
 
-	/* XXX delete from parent's id2children entry XXX */
-	pdn = dn_parent( be, dn );
-	p = dn2entry_r( be, pdn, &matched );
-	free( pdn );
+	/* delete from parent's id2children entry */
+	if( (pdn = dn_parent( be, dn )) != NULL ) {
+		if( (p = dn2entry_w( be, pdn, &matched )) == NULL) {
+			Debug( LDAP_DEBUG_TRACE, "parent does not exist\n",
+				0, 0, 0);
+			send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR,
+				"", "");
+			goto return_results;
+		}
+
+#ifndef SLAPD_CHILD_MODIFICATION_WITH_ENTRY_ACL
+		/* check parent for "children" acl */
+		if ( ! access_allowed( be, conn, op, p, "children", NULL,
+			op->o_dn, ACL_WRITE ) )
+		{
+			Debug( LDAP_DEBUG_TRACE, "no access to parent\n", 0,
+				0, 0 );
+			send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
+				"", "" );
+			goto return_results;
+		}
+#endif
+
+	} else {
+		/* no parent, must be root to delete */
+		if( ! be_isroot( be, op->o_dn ) ) {
+			Debug( LDAP_DEBUG_TRACE, "no parent & not root\n",
+				0, 0, 0);
+			send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
+				"", "");
+			goto return_results;
+		}
+
+		pthread_mutex_lock(&li->li_root_mutex);
+		rootlock = 1;
+	}
+
 	if ( id2children_remove( be, p, e ) != 0 ) {
+		Debug(LDAP_DEBUG_ARGS,
+			"<=- ldbm_back_delete: operations error %s\n",
+			dn, 0, 0);
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "","" );
-                goto error_return;
+		goto return_results;
 	}
 
 	/* delete from dn2id mapping */
@@ -79,7 +119,7 @@ ldbm_back_delete(
 			"<=- ldbm_back_delete: operations error %s\n",
 			dn, 0, 0);
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
-		goto error_return;
+		goto return_results;
 	}
 
 	/* delete from disk and cache */
@@ -88,28 +128,28 @@ ldbm_back_delete(
 			"<=- ldbm_back_delete: operations error %s\n",
 			dn, 0, 0);
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
-		goto error_return;
+		goto return_results;
 	}
 
-	/* free entry and writer lock */
-	cache_return_entry_w( &li->li_cache, e );
-	if ( p != NULL )
-		cache_return_entry_r( &li->li_cache, p );
+	send_ldap_result( conn, op, LDAP_SUCCESS, "", "" );
+	rc = 0;
 
-	if ( matched != NULL ) free(matched);
+return_results:;
+	if ( pdn != NULL ) free(pdn);
 
-	send_ldap_result( conn, op, LDAP_SUCCESS, "", "" );
+	if( p != NULL ) {
+		/* free parent and writer lock */
+		cache_return_entry_w( &li->li_cache, p );
 
-	return( 0 );
+	} else if ( rootlock ) {
+		/* release root lock */
+		pthread_mutex_unlock(&li->li_root_mutex);
+	}
 
-error_return:;
 	/* free entry and writer lock */
 	cache_return_entry_w( &li->li_cache, e );
 
-	if( p != NULL )
-		cache_return_entry_r( &li->li_cache, p );
-
 	if ( matched != NULL ) free(matched);
 
-	return( -1 );
+	return rc;
 }

servers/slapd/back-ldbm/init.c

@@ -63,6 +63,7 @@ ldbm_back_init(
 	free( argv[ 1 ] );
 
 	/* initialize various mutex locks & condition variables */
+	pthread_mutex_init( &li->li_root_mutex, pthread_mutexattr_default );
 	pthread_mutex_init( &li->li_add_mutex, pthread_mutexattr_default );
 	pthread_mutex_init( &li->li_cache.c_mutex, pthread_mutexattr_default );
 	pthread_mutex_init( &li->li_nextid_mutex, pthread_mutexattr_default );

servers/slapd/back-ldbm/modrdn.c

@@ -22,12 +22,12 @@ ldbm_back_modrdn(
 )
 {
 	struct ldbminfo	*li = (struct ldbminfo *) be->be_private;
-	char		*matched;
-	char		*pdn, *newdn, *p;
+	char		*matched = NULL;
+	char		*pdn = NULL, *newdn = NULL;
 	char		sep[2];
-	Entry		*e;
-
-	matched = NULL;
+	Entry		*e, *p = NULL;
+	int			rootlock = 0;
+	int			rc = -1;
 
 	/* get entry with writer lock */
 	if ( (e = dn2entry_w( be, dn, &matched )) == NULL ) {
@@ -38,8 +38,42 @@ ldbm_back_modrdn(
 		return( -1 );
 	}
 
+#ifdef SLAPD_CHILD_MODIFICATION_WITH_ENTRY_ACL
+		/* check parent for "children" acl */
+	if ( ! access_allowed( be, conn, op, e, "entry", NULL,
+		op->o_dn, ACL_WRITE ) )
+	{
+		Debug( LDAP_DEBUG_TRACE, "no access to entry\n", 0,
+			0, 0 );
+		send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
+			"", "" );
+		goto return_results;
+	}
+#endif
+
 	if ( (pdn = dn_parent( be, dn )) != NULL ) {
 		/* parent + rdn + separator(s) + null */
+		if( (p = dn2entry_w( be, pdn, &matched )) == NULL) {
+			Debug( LDAP_DEBUG_TRACE, "parent does not exist\n",
+				0, 0, 0);
+			send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR,
+				"", "");
+			goto return_results;
+		}
+
+#ifndef SLAPD_CHILD_MODIFICATION_WITH_ENTRY_ACL
+		/* check parent for "children" acl */
+		if ( ! access_allowed( be, conn, op, p, "children", NULL,
+			op->o_dn, ACL_WRITE ) )
+		{
+			Debug( LDAP_DEBUG_TRACE, "no access to parent\n", 0,
+				0, 0 );
+			send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
+				"", "" );
+			goto return_results;
+		}
+#endif
+
 		newdn = (char *) ch_malloc( strlen( pdn ) + strlen( newrdn )
 		    + 3 );
 		if ( dn_type( dn ) == DN_X500 ) {
@@ -47,12 +81,13 @@ ldbm_back_modrdn(
 			strcat( newdn, ", " );
 			strcat( newdn, pdn );
 		} else {
+			char *s;
 			strcpy( newdn, newrdn );
-			p = strchr( newrdn, '\0' );
-			p--;
-			if ( *p != '.' && *p != '@' ) {
-				if ( (p = strpbrk( dn, ".@" )) != NULL ) {
-					sep[0] = *p;
+			s = strchr( newrdn, '\0' );
+			s--;
+			if ( *s != '.' && *s != '@' ) {
+				if ( (s = strpbrk( dn, ".@" )) != NULL ) {
+					sep[0] = *s;
 					sep[1] = '\0';
 					strcat( newdn, sep );
 				}
@@ -60,42 +95,46 @@ ldbm_back_modrdn(
 			strcat( newdn, pdn );
 		}
 	} else {
+		/* no parent, modrdn entry directly under root */
+		if( ! be_isroot( be, op->o_dn ) ) {
+			Debug( LDAP_DEBUG_TRACE, "no parent & not root\n",
+				0, 0, 0);
+			send_ldap_result( conn, op, LDAP_INSUFFICIENT_ACCESS,
+				"", "");
+			goto return_results;
+		}
+
+		pthread_mutex_lock(&li->li_root_mutex);
+		rootlock = 1;
+
 		newdn = ch_strdup( newrdn );
 	}
+
 	(void) dn_normalize( newdn );
 
-	/* get entry with writer lock */
 	if ( (dn2id ( be, newdn ) ) != NOID ) {
-		free( newdn );
-		free( pdn );
 		send_ldap_result( conn, op, LDAP_ALREADY_EXISTS, NULL, NULL );
-		goto error_return;
+		goto return_results;
 	}
 
 	/* check for abandon */
 	pthread_mutex_lock( &op->o_abandonmutex );
 	if ( op->o_abandon ) {
 		pthread_mutex_unlock( &op->o_abandonmutex );
-		free( newdn );
-		free( pdn );
-		goto error_return;
+		goto return_results;
 	}
 	pthread_mutex_unlock( &op->o_abandonmutex );
 
 	/* add new one */
 	if ( dn2id_add( be, newdn, e->e_id ) != 0 ) {
-		free( newdn );
-		free( pdn );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, NULL, NULL );
-		goto error_return;
+		goto return_results;
 	}
 
 	/* delete old one */
 	if ( dn2id_delete( be, dn ) != 0 ) {
-		free( newdn );
-		free( pdn );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, NULL, NULL );
-		goto error_return;
+		goto return_results;
 	}
 
 	(void) cache_delete_entry( &li->li_cache, e );
@@ -115,18 +154,27 @@ ldbm_back_modrdn(
 	if ( id2entry_add( be, e ) != 0 ) {
 		entry_free( e );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
-		goto error_return;
+		goto return_results;
 	}
-	free( pdn );
 
-	/* free entry and writer lock */
-	cache_return_entry_w( &li->li_cache, e );
 	send_ldap_result( conn, op, LDAP_SUCCESS, NULL, NULL );
+	rc = 0;
+
+return_results:
+	if( newdn != NULL ) free( newdn );
+	if( pdn != NULL ) free( pdn );
+	if( matched != NULL ) free( matched );
 
-	return( 0 );
+	if( p != NULL ) {
+		/* free parent and writer lock */
+		cache_return_entry_w( &li->li_cache, p );
+
+	} else if ( rootlock ) {
+		/* release root writer lock */
+		pthread_mutex_unlock(&li->li_root_mutex);
+	}
 
-error_return:
 	/* free entry and writer lock */
 	cache_return_entry_w( &li->li_cache, e );
-	return( -1 );
+	return( rc );
 }