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