Ticket 47782 - Parent numbordinate count can be incorrectly updated if an error occurs

Bug Description:  When adding and deleting entries there is a small chance that the modified
                  parent entry(numbsubordinates) could be replaced in the entry cache, even
                  if the operation fails.

Fix Description:  For deletes we can simply move the cache entry switching to a safe location,
                  but for adds we need to unswitch the parent's entry in the cache.

https://fedorahosted.org/389/ticket/47782

Reviewed by: nhosoi(Thanks!)

ldap/servers/slapd/back-ldbm/ldbm_add.c

@@ -118,6 +118,7 @@ ldbm_back_add( Slapi_PBlock *pb )
 	CSN *opcsn = NULL;
 	entry_address addr = {0};
 	int not_an_error = 0;
+	int parent_switched = 0;
 
 	slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li );
 	slapi_pblock_get( pb, SLAPI_ADD_ENTRY, &e );
@@ -1020,6 +1021,7 @@ ldbm_back_add( Slapi_PBlock *pb )
 	{
 		/* switch the parent entry copy into play */
 		modify_switch_entries( &parent_modify_c,be);
+		parent_switched = 1;
 	}
 
 	if (ruv_c_init) {
@@ -1099,6 +1101,14 @@ error_return:
 	} else if (0 == rc) {
 		rc = SLAPI_FAIL_GENERAL;
 	}
+	if (parent_switched){
+		/*
+		 * Restore the old parent entry, switch the new with the original.
+		 * Otherwise the numsubordinate count will be off, and could later
+		 * be written to disk.
+		 */
+		modify_unswitch_entries( &parent_modify_c,be);
+	}
 diskfull_return:
 	if (disk_full) {
 		rc= return_on_disk_full(li);

ldap/servers/slapd/back-ldbm/ldbm_delete.c

@@ -1155,12 +1155,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
 		CACHE_RETURN(&inst->inst_cache, &e);
 		e = NULL;
 	}
-
-	if (parent_found)
-	{
-		 /* Replace the old parent entry with the newly modified one */
-		modify_switch_entries( &parent_modify_c,be);
-	}
 	
 	if (ruv_c_init) {
 		if (modify_switch_entries(&ruv_c, be) != 0 ) {
@@ -1172,6 +1166,12 @@ ldbm_back_delete( Slapi_PBlock *pb )
 		}
 	}
 
+	if (parent_found)
+	{
+		/* Replace the old parent entry with the newly modified one */
+		modify_switch_entries( &parent_modify_c,be);
+	}
+
 	rc= 0;
 	goto common_return;
 

ldap/servers/slapd/back-ldbm/ldbm_modify.c

@@ -122,6 +122,44 @@ int modify_switch_entries(modify_context *mc,backend *be)
 	return ret;
 }
 
+/*
+ * Switch the new with the old(original) - undoing modify_switch_entries()
+ * This expects modify_term() to be called next, as the old "new" entry
+ * is now gone(replaced by the original entry).
+ */
+int
+modify_unswitch_entries(modify_context *mc,backend *be)
+{
+	struct backentry *tmp_be;
+	ldbm_instance *inst = (ldbm_instance *) be->be_instance_info;
+	int ret = 0;
+
+	if (mc->old_entry!=NULL && mc->new_entry!=NULL) {
+		/* switch the entries, and reset the new, new, entry */
+		tmp_be = mc->new_entry;
+		mc->new_entry = mc->old_entry;
+		mc->new_entry->ep_state = 0;
+		mc->new_entry->ep_refcnt = 0;
+		mc->new_entry_in_cache = 0;
+		mc->old_entry = tmp_be;
+
+		ret = cache_replace(&(inst->inst_cache), mc->old_entry, mc->new_entry);
+		if (ret == 0) {
+			/*
+			 * The new entry was originally locked, so since we did the
+			 * switch we need to unlock the "new" entry, and return the
+			 * "old" one.  modify_term() will then return the "new" entry.
+			 */
+			cache_unlock_entry(&inst->inst_cache, mc->new_entry);
+			CACHE_RETURN( &(inst->inst_cache), &(mc->old_entry) );
+			mc->new_entry_in_cache = 1;
+			mc->old_entry = NULL;
+		}
+	}
+
+	return ret;
+}
+
 /* This routine does that part of a modify operation which involves
    updating the on-disk data: updates idices, id2entry. 
    Copes properly with DB_LOCK_DEADLOCK. The caller must be able to cope with 

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

@@ -365,6 +365,7 @@ void modify_init(modify_context *mc,struct backentry *old_entry);
 int modify_apply_mods(modify_context *mc, Slapi_Mods *smods);
 int modify_term(modify_context *mc,backend *be);
 int modify_switch_entries(modify_context *mc,backend *be);
+int modify_unswitch_entries(modify_context *mc,backend *be);
 int modify_apply_mods_ignore_error(modify_context *mc, Slapi_Mods *smods, int error);
 
 /*