Bug 631862 - crash - delete entries not in cache + referint

https://bugzilla.redhat.com/show_bug.cgi?id=631862
Resolves: bug 631862
Bug Description: crash - delete entries not in cache + referint
Reviewed by: rmeggins and nhosoi
Branch: master
Fix Description: When deleting an entry, the referential integrity (referint)
plugin does an internal search to find references to this entry (e.g. in
group entries) and removes them.  The search code wants to ensure that the
entrydn attribute is present in the entry when using entryrdn (subtree
rename).  The search code sets a flag to tell the id2entry code to add the
entrydn attribute if it is not present.  However, it was doing this to an
entry in the cache, which may be in use by another thread.  The solution is
to add the entrydn attribute before adding the entry to the cache.  In the
id2entry code, this is after the entry has been read from the id2entry db
successfully, but before the entry is added to the cache.  In the LDAP ADD
code, this is done when the other computed operational attributes are added
to the new entry.
In addition to the above fix by [email protected], following changes are
made:
1) entrydn attribute is always added to the entry in memory before putting
   it in the entry cache, and the attribute is removed before writing the
   entry to the database.
2) eliminating id2entry_ext, which was introduced to pass flags, but it is
   no longer needed since only a flag ID2ENTRY_ADD_ENTRYDN was removed.
   Platforms tested: RHEL5 x86_64
   Flag Day: no
   Doc impact: no

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

@@ -829,9 +829,6 @@ typedef struct _back_search_result_set
 /* whether we call fat lock or not [608146] */
 #define SERIALLOCK(li)	(li->li_fat_lock)
 
-/* id2entry_ext flags */
-#define ID2ENTRY_ADD_ENTRYDN 0x1
-
 /* 
  * 0: SUCCESS
  * libdb returns negative error codes

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

@@ -60,6 +60,7 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
     int    len, rc;
     char   temp_id[sizeof(ID)];
     struct backentry *encrypted_entry = NULL;
+    char *entrydn = NULL;
 
     LDAPDebug( LDAP_DEBUG_TRACE, "=> id2entry_add( %lu, \"%s\" )\n",
                                  (u_long)e->ep_id, backentry_get_ndn(e), 0 );
@@ -94,6 +95,7 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
         memset(&data, 0, sizeof(data));
         if (entryrdn_get_switch())
         {
+            Slapi_Attr *eattr = NULL;
             struct backdn *oldbdn = NULL;
             Slapi_DN *sdn =
                           slapi_sdn_dup(slapi_entry_get_sdn_const(e->ep_entry));
@@ -107,7 +109,7 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
                     if (cache_replace( &inst->inst_dncache, oldbdn, bdn ) != 0) {
                         /* The entry was not in the cache for some reason (this
                          * should not happen since CACHE_ADD said it existed above). */
-			LDAPDebug( LDAP_DEBUG_ANY, "id2entry_add_ext(): Entry disappeared "
+                        LDAPDebug( LDAP_DEBUG_ANY, "id2entry_add_ext(): Entry disappeared "
                                    "from cache (%s)\n", oldbdn->dn_sdn, 0, 0 );
                     }
                 }
@@ -118,6 +120,13 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
             LDAPDebug( LDAP_DEBUG_TRACE,
                    "=> id2entry_add (dncache) ( %lu, \"%s\" )\n",
                    (u_long)e->ep_id, slapi_entry_get_dn_const(e->ep_entry), 0 );
+            /* If entrydn exists in the entry, we have to remove it before
+             * writing the entry to the database. */
+            if (0 == slapi_entry_attr_find(e->ep_entry,
+                                           LDBM_ENTRYDN_STR, &eattr)) {
+                /* entrydn exists in the entry.  let's removed it. */
+                slapi_entry_delete_values(e->ep_entry, LDBM_ENTRYDN_STR, NULL);
+            }
         }
         data.dptr = slapi_entry2str_with_options(entry_to_use, &len, options);
         data.dsize = len + 1;
@@ -145,6 +154,7 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
             const char *myrdn = slapi_entry_get_rdn_const(e->ep_entry);
             const char *parentdn = NULL;
             char *myparentdn = NULL;
+            Slapi_Attr *eattr = NULL;
             /* If the parent is in the cache, check the parent's DN and 
              * adjust to it if they don't match. (bz628300) */
             if (parentid && myrdn) {
@@ -167,6 +177,24 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
                     CACHE_RETURN(&inst->inst_cache, &parententry);
                 }
             }
+            /* 
+             * Adding entrydn attribute value to the entry,
+             * which should be done before adding the entry to the entry cache.
+             * Note: since we removed entrydn from the entry before writing
+             * it to the database, it is guaranteed not in the entry.
+             */
+            /* slapi_ch_strdup and slapi_dn_ignore_case never returns NULL */
+            entrydn = slapi_ch_strdup(slapi_entry_get_dn_const(e->ep_entry));
+            entrydn = slapi_dn_ignore_case(entrydn);
+            slapi_entry_attr_set_charptr (e->ep_entry,
+                                          LDBM_ENTRYDN_STR, entrydn);
+            if (0 == slapi_entry_attr_find(e->ep_entry,
+                                           LDBM_ENTRYDN_STR, &eattr)) {
+                /* now entrydn should exist in the entry */
+                /* Set it to operational attribute */
+                eattr->a_flags = SLAPI_ATTR_FLAG_OPATTR;
+            }
+            slapi_ch_free_string(&entrydn);
         }
         /* 
          * For ldbm_back_add and ldbm_back_modify, this entry had been already
@@ -245,7 +273,7 @@ id2entry_delete( backend *be, struct backentry *e, back_txn *txn )
 }
 
 struct backentry *
-id2entry_ext( backend *be, ID id, back_txn *txn, int *err, int flags  )
+id2entry( backend *be, ID id, back_txn *txn, int *err  )
 {
     ldbm_instance    *inst = (ldbm_instance *) be->be_instance_info;
     DB               *db = NULL;
@@ -387,6 +415,30 @@ id2entry_ext( backend *be, ID id, back_txn *txn, int *err, int flags  )
                             "attrcrypt_decrypt_entry failed in id2entry\n");
         }
         
+        /* 
+         * If return entry exists AND entryrdn switch is on,
+         * add the entrydn value.
+         */
+        if (entryrdn_get_switch()) {
+            Slapi_Attr *eattr = NULL;
+            /* Check if entrydn is in the entry or not */
+            if (slapi_entry_attr_find(e->ep_entry, LDBM_ENTRYDN_STR, &eattr)) {
+                /* entrydn does not exist in the entry */
+                char *entrydn = NULL;
+                /* slapi_ch_strdup and slapi_dn_ignore_case never returns NULL */
+                entrydn = slapi_ch_strdup(slapi_entry_get_dn_const(e->ep_entry));
+                entrydn = slapi_dn_ignore_case(entrydn);
+                slapi_entry_attr_set_charptr (e->ep_entry,
+                                              LDBM_ENTRYDN_STR, entrydn);
+                if (0 == slapi_entry_attr_find(e->ep_entry,
+                                               LDBM_ENTRYDN_STR, &eattr)) {
+                    /* now entrydn should exist in the entry */
+                    /* Set it to operational attribute */
+                    eattr->a_flags = SLAPI_ATTR_FLAG_OPATTR;
+                }
+                slapi_ch_free_string(&entrydn);
+            }
+        }
         retval = CACHE_ADD( &inst->inst_cache, e, &imposter );
         if (1 == retval) {
             /* This means that someone else put the entry in the cache
@@ -413,29 +465,6 @@ id2entry_ext( backend *be, ID id, back_txn *txn, int *err, int flags  )
     }
 
 bail:
-    /* 
-     * If return entry exists AND adding entrydn is requested AND
-     * entryrdn switch is on, add the entrydn value.
-     */
-    if (e && e->ep_entry && (flags & ID2ENTRY_ADD_ENTRYDN) &&
-        entryrdn_get_switch()) {
-        Slapi_Attr *eattr = NULL;
-        /* Check if entrydn is in the entry or not */
-        if (slapi_entry_attr_find(e->ep_entry, "entrydn", &eattr)) {
-            /* entrydn does not exist in the entry */
-            char *entrydn = NULL;
-            /* slapi_ch_strdup and slapi_dn_ignore_case never returns NULL */
-            entrydn = slapi_ch_strdup(slapi_entry_get_dn_const(e->ep_entry));
-            entrydn = slapi_dn_ignore_case(entrydn);
-            slapi_entry_attr_set_charptr (e->ep_entry, "entrydn", entrydn);
-            if (0 == slapi_entry_attr_find(e->ep_entry, "entrydn", &eattr)) {
-                /* now entrydn should exist in the entry */
-                /* Set it to operational attribute */
-                eattr->a_flags = SLAPI_ATTR_FLAG_OPATTR;
-            }
-            slapi_ch_free_string(&entrydn);
-        }
-    }
     slapi_ch_free( &(data.data) );
 
     dblayer_release_id2entry( be, db );
@@ -444,10 +473,3 @@ bail:
                     "<= id2entry( %lu ) %p (disk)\n", (u_long)id, e);
     return( e );
 }
-
-struct backentry *
-id2entry( backend *be, ID id, back_txn *txn, int *err  )
-{
-    return id2entry_ext(be, id, txn, err, 0);
-}
-

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

@@ -575,17 +575,12 @@ ldbm_back_add( Slapi_PBlock *pb )
 	
 	if(is_resurect_operation)
 	{
-		if (!entryrdn_get_switch()) { /* subtree-rename: off */
-			/* add the entrydn operational attributes to the addingentry */
-			add_update_entrydn_operational_attributes(addingentry);
-		}
+		add_update_entrydn_operational_attributes(addingentry);
 	}
 	else if (is_tombstone_operation)
 	{
-		if (!entryrdn_get_switch()) { /* subtree-rename: off */
-			/* Remove the entrydn operational attributes from the addingentry */
-			delete_update_entrydn_operational_attributes(addingentry);
-		}
+		/* Remove the entrydn operational attributes from the addingentry */
+		delete_update_entrydn_operational_attributes(addingentry);
 	}
 	else
 	{
@@ -978,10 +973,8 @@ add_update_entry_operational_attributes(struct backentry *ep, ID pid)
 	bv.bv_len = strlen( buf );
 	entry_replace_values( ep->ep_entry, "entryid", bvp );
 
-    if (!entryrdn_get_switch()) { /* subtree-rename: off */
-		/* add the entrydn operational attribute to the entry. */
-		add_update_entrydn_operational_attributes(ep);
-	}
+	/* add the entrydn operational attribute to the entry. */
+	add_update_entrydn_operational_attributes(ep);
 }
 
 /*

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

@@ -1257,7 +1257,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension )
         ++sr->sr_lookthroughcount;    /* checked above */
 
         /* get the entry */
-        e = id2entry_ext( be, id, NULL, &err, ID2ENTRY_ADD_ENTRYDN );
+        e = id2entry( be, id, NULL, &err );
         if ( e == NULL )
         {
             if ( err != 0 && err != DB_NOTFOUND )

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

@@ -217,8 +217,6 @@ int id2entry_add( backend *be, struct backentry *e, back_txn *txn );
 int id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt );
 int id2entry_delete( backend *be, struct backentry *e, back_txn *txn );
 struct backentry * id2entry( backend *be, ID id, back_txn *txn, int *err );
-struct backentry * id2entry_ext( backend *be, ID id, back_txn *txn, int *err, int flags  );
-
 
 /*
  * idl.c