From 55e8358edd9c53cf96f7745e3cf241fd72b11b77 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Wed, 16 Apr 2014 15:21:21 -0400
Subject: [PATCH 202/225] 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!)
(cherry picked from commit ac8ada32de4bcc3dfd0a3e2958cefd91f98c4170)
---
 ldap/servers/slapd/back-ldbm/ldbm_add.c        | 13 +++++++++
 ldap/servers/slapd/back-ldbm/ldbm_delete.c     | 12 ++++----
 ldap/servers/slapd/back-ldbm/ldbm_modify.c     | 38 ++++++++++++++++++++++++++
 ldap/servers/slapd/back-ldbm/proto-back-ldbm.h |  1 +
 4 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c
index 554822c..0ade23c 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
@@ -117,6 +117,7 @@ ldbm_back_add( Slapi_PBlock *pb )
 	int is_ruv = 0;				 /* True if the current entry is RUV */
 	CSN *opcsn = NULL;
 	entry_address addr = {0};
+	int parent_switched = 0;
 
 	slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li );
 	slapi_pblock_get( pb, SLAPI_ADD_ENTRY, &e );
@@ -974,6 +975,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) {
@@ -1046,6 +1048,17 @@ error_return:
 		dblayer_remember_disk_filled(li);
 		ldbm_nasty("Add",80,rc);
 		disk_full = 1;
+	} 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:
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index 7c036df..c80d75a 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -1095,12 +1095,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 ) {
@@ -1112,6 +1106,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;
 
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index b8b6114..be47fcc 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
+++ b/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 
diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
index ee30c45..02b261b 100644
--- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
@@ -350,6 +350,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);
 
 /*
-- 
1.8.1.4