ldap/servers/slapd/back-ldbm/ldbm_add.c | 74 ++++++++++++++-------------- ldap/servers/slapd/back-ldbm/ldbm_compare.c | 6 +- ldap/servers/slapd/back-ldbm/ldbm_delete.c | 20 +++---- ldap/servers/slapd/back-ldbm/ldbm_modify.c | 24 ++++----- ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 26 ++++----- ldap/servers/slapd/back-ldbm/ldbm_search.c | 9 +-- 6 files changed, 82 insertions(+), 77 deletions(-)
New commits: commit e229bb45b418371d83af302f3d24f881ca32bd41 Author: Noriko Hosoi nhosoi@totoro.usersys.redhat.com Date: Fri Nov 2 14:36:38 2012 -0700
Coverity fixes
13107,13108: Explicit null deferenced
Bug description: "Explicit null dereferenced" error was introduced by commit 7f81635990fa340e2db5c1b14a8d1ba10fa53887 Trac Ticket #391 - Slapd crashes when deleting backends while operations are still in progress
Fix description: Added codes to check if the inst variable is NULL or not. If NULL, skip accessing the instance variable and return an error.
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index cf2676e..1fd2240 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -78,7 +78,7 @@ ldbm_back_add( Slapi_PBlock *pb ) { backend *be; struct ldbminfo *li; - ldbm_instance *inst; + ldbm_instance *inst = NULL; const char *dn = NULL; Slapi_Entry *e = NULL; struct backentry *tombstoneentry = NULL; @@ -102,7 +102,7 @@ ldbm_back_add( Slapi_PBlock *pb ) modify_context ruv_c = {0}; int parent_found = 0; int ruv_c_init = 0; - int rc; + int rc = 0; int addingentry_id_assigned= 0; int addingentry_in_cache= 0; int tombstone_in_cache= 0; @@ -132,12 +132,12 @@ ldbm_back_add( Slapi_PBlock *pb ) is_ruv = operation_is_flag_set(operation, OP_FLAG_REPL_RUV);
inst = (ldbm_instance *) be->be_instance_info; - if (inst->inst_ref_count) { + if (inst && inst->inst_ref_count) { slapi_counter_increment(inst->inst_ref_count); } else { LDAPDebug1Arg(LDAP_DEBUG_ANY, - "ldbm_add: instance %s does not exist.\n", - inst->inst_name); + "ldbm_add: instance "%s" does not exist.\n", + inst ? inst->inst_name : "null instance"); goto error_return; }
@@ -1060,13 +1060,15 @@ error_return: { if ( addingentry_in_cache ) { - CACHE_REMOVE(&inst->inst_cache, addingentry); + if (inst) { + CACHE_REMOVE(&inst->inst_cache, addingentry); + } addingentry_in_cache = 0; } backentry_clear_entry(addingentry); /* e is released in the frontend */ backentry_free( &addingentry ); /* release the backend wrapper, here */ } - if(tombstone_in_cache) + if(tombstone_in_cache && inst) { CACHE_RETURN(&inst->inst_cache, &tombstoneentry); } @@ -1075,8 +1077,9 @@ error_return: dblayer_remember_disk_filled(li); ldbm_nasty("Add",80,rc); disk_full = 1; + } else if (0 == rc) { + rc = SLAPI_FAIL_GENERAL; } - diskfull_return: if (disk_full) { rc= return_on_disk_full(li); @@ -1121,33 +1124,34 @@ diskfull_return: } common_return: - if (addingentry_in_cache && addingentry) - { - if (entryrdn_get_switch()) { /* subtree-rename: on */ - /* since adding the entry to the entry cache was successful, - * let's add the dn to dncache, if not yet done. */ - struct backdn *bdn = dncache_find_id(&inst->inst_dncache, - addingentry->ep_id); - if (bdn) { /* already in the dncache */ - CACHE_RETURN(&inst->inst_dncache, &bdn); - } else { /* not in the dncache yet */ - Slapi_DN *addingsdn = - slapi_sdn_dup(slapi_entry_get_sdn(addingentry->ep_entry)); - if (addingsdn) { - bdn = backdn_init(addingsdn, addingentry->ep_id, 0); - if (bdn) { - CACHE_ADD( &inst->inst_dncache, bdn, NULL ); - CACHE_RETURN(&inst->inst_dncache, &bdn); - slapi_log_error(SLAPI_LOG_CACHE, "ldbm_back_add", - "set %s to dn cache\n", dn); - } - } - } - } - CACHE_RETURN( &inst->inst_cache, &addingentry ); - } - if (inst->inst_ref_count) { - slapi_counter_decrement(inst->inst_ref_count); + if (inst) { + if (addingentry_in_cache && addingentry) { + if (entryrdn_get_switch()) { /* subtree-rename: on */ + /* since adding the entry to the entry cache was successful, + * let's add the dn to dncache, if not yet done. */ + struct backdn *bdn = dncache_find_id(&inst->inst_dncache, + addingentry->ep_id); + if (bdn) { /* already in the dncache */ + CACHE_RETURN(&inst->inst_dncache, &bdn); + } else { /* not in the dncache yet */ + Slapi_DN *addingsdn = + slapi_sdn_dup(slapi_entry_get_sdn(addingentry->ep_entry)); + if (addingsdn) { + bdn = backdn_init(addingsdn, addingentry->ep_id, 0); + if (bdn) { + CACHE_ADD( &inst->inst_dncache, bdn, NULL ); + CACHE_RETURN(&inst->inst_dncache, &bdn); + slapi_log_error(SLAPI_LOG_CACHE, "ldbm_back_add", + "set %s to dn cache\n", dn); + } + } + } + } + CACHE_RETURN( &inst->inst_cache, &addingentry ); + } + if (inst->inst_ref_count) { + slapi_counter_decrement(inst->inst_ref_count); + } } /* bepost op needs to know this result */ slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_compare.c b/ldap/servers/slapd/back-ldbm/ldbm_compare.c index e201ca6..12b6aa5 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_compare.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_compare.c @@ -74,12 +74,12 @@ ldbm_back_compare( Slapi_PBlock *pb ) }
inst = (ldbm_instance *) be->be_instance_info; - if (inst->inst_ref_count) { + if (inst && inst->inst_ref_count) { slapi_counter_increment(inst->inst_ref_count); } else { LDAPDebug1Arg(LDAP_DEBUG_ANY, - "ldbm_compare: instance %s does not exist.\n", - inst->inst_name); + "ldbm_compare: instance "%s" does not exist.\n", + inst ? inst->inst_name : "null instance"); return -1; } /* get the namespace dn */ diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 5661bfa..683d324 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -146,12 +146,12 @@ ldbm_back_delete( Slapi_PBlock *pb ) delete_tombstone_entry = operation_is_flag_set(operation, OP_FLAG_TOMBSTONE_ENTRY); inst = (ldbm_instance *) be->be_instance_info; - if (inst->inst_ref_count) { + if (inst && inst->inst_ref_count) { slapi_counter_increment(inst->inst_ref_count); } else { LDAPDebug1Arg(LDAP_DEBUG_ANY, - "ldbm_delete: instance %s does not exist.\n", - inst->inst_name); + "ldbm_delete: instance "%s" does not exist.\n", + inst ? inst->inst_name : "null instance"); goto error_return; }
@@ -1116,7 +1116,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) goto common_return;
error_return: - if (tombstone_in_cache) + if (inst && tombstone_in_cache) { CACHE_REMOVE( &inst->inst_cache, tombstone ); CACHE_RETURN( &inst->inst_cache, &tombstone ); @@ -1186,7 +1186,7 @@ common_return: for the post op plugins */ slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry ); } - if (tombstone_in_cache) + if (inst && tombstone_in_cache) { CACHE_RETURN( &inst->inst_cache, &tombstone ); tombstone = NULL; @@ -1208,14 +1208,14 @@ common_return: }
/* Need to return to cache after post op plugins are called */ - if (retval) { /* error case */ - if (e) { + if (inst) { + if (retval && e) { /* error case */ cache_unlock_entry( &inst->inst_cache, e ); CACHE_RETURN( &inst->inst_cache, &e ); } - } - if (inst->inst_ref_count) { - slapi_counter_decrement(inst->inst_ref_count); + if (inst->inst_ref_count) { + slapi_counter_decrement(inst->inst_ref_count); + } } if (ruv_c_init) { diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index 9cb872a..0db1f87 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -368,12 +368,12 @@ ldbm_back_modify( Slapi_PBlock *pb ) { goto error_return; } - if (inst->inst_ref_count) { + if (inst && inst->inst_ref_count) { slapi_counter_increment(inst->inst_ref_count); } else { LDAPDebug1Arg(LDAP_DEBUG_ANY, - "ldbm_modify: instance %s does not exist.\n", - inst->inst_name); + "ldbm_modify: instance "%s" does not exist.\n", + inst ? inst->inst_name : "null instance"); goto error_return; }
@@ -795,7 +795,7 @@ error_return: }
/* if ec is in cache, remove it, then add back e if we still have it */ - if (ec_in_cache) { + if (inst && ec_in_cache) { CACHE_REMOVE( &inst->inst_cache, ec ); /* if ec was in cache, e was not - add back e */ if (e) { @@ -807,7 +807,7 @@ error_return: common_return: slapi_mods_done(&smods); - if (ec_in_cache) + if (inst && ec_in_cache) { cache_unlock_entry( &inst->inst_cache, ec); CACHE_RETURN( &inst->inst_cache, &ec ); @@ -817,12 +817,14 @@ common_return: backentry_free(&ec); } - if (e!=NULL) { - cache_unlock_entry( &inst->inst_cache, e); - CACHE_RETURN( &inst->inst_cache, &e); - } - if (inst->inst_ref_count) { - slapi_counter_decrement(inst->inst_ref_count); + if (inst) { + if (e) { + cache_unlock_entry( &inst->inst_cache, e); + CACHE_RETURN( &inst->inst_cache, &e); + } + if (inst->inst_ref_count) { + slapi_counter_decrement(inst->inst_ref_count); + } }
/* result code could be used in the bepost plugin functions. */ diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index ef73904..98b30dc 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -63,7 +63,7 @@ int ldbm_back_modrdn( Slapi_PBlock *pb ) { backend *be; - ldbm_instance *inst; + ldbm_instance *inst = NULL; struct ldbminfo *li; struct backentry *e= NULL; struct backentry *ec= NULL; @@ -191,12 +191,12 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) return( -1 ); }
- if (inst->inst_ref_count) { + if (inst && inst->inst_ref_count) { slapi_counter_increment(inst->inst_ref_count); } else { LDAPDebug1Arg(LDAP_DEBUG_ANY, - "ldbm_modrdn: instance %s does not exist.\n", - inst->inst_name); + "ldbm_modrdn: instance "%s" does not exist.\n", + inst ? inst->inst_name : "null instance"); return( -1 ); }
@@ -1177,7 +1177,7 @@ error_return: /* make sure caller doesn't attempt to free this */ slapi_pblock_set( pb, SLAPI_ENTRY_POST_OP, postentry ); } - if (e && entryrdn_get_switch()) + if (e && entryrdn_get_switch() && inst) { struct backdn *bdn = dncache_find_id(&inst->inst_dncache, e->ep_id); CACHE_REMOVE(&inst->inst_dncache, bdn); @@ -1186,7 +1186,7 @@ error_return: if(children) { int i = 0; - if (child_entries && *child_entries) + if (child_entries && *child_entries && inst) { if (entryrdn_get_switch()) /* subtree-rename: on */ { @@ -1208,7 +1208,7 @@ error_return: } } } - if (entryrdn_get_switch() && child_dns && *child_dns) + if (entryrdn_get_switch() && child_dns && *child_dns && inst) { for (i = 0; child_dns[i] != NULL; i++) { CACHE_REMOVE( &inst->inst_dncache, child_dns[i] ); @@ -1282,10 +1282,10 @@ common_return: if (ec) { /* remove the new entry from the cache if the op failed - otherwise, leave it in */ - if (ec_in_cache && retval) { - CACHE_REMOVE( &inst->inst_cache, ec ); - } - if (ec_in_cache) { + if (ec_in_cache && inst) { + if (retval) { + CACHE_REMOVE( &inst->inst_cache, ec ); + } CACHE_RETURN( &inst->inst_cache, &ec ); } else { backentry_free( &ec ); @@ -1296,12 +1296,12 @@ common_return:
/* put e back in the cache if the modrdn failed */ if (e) { - if (!e_in_cache && retval) { + if (!e_in_cache && retval && inst) { CACHE_ADD(&inst->inst_cache, e, NULL); e_in_cache = 1; } } - if (inst->inst_ref_count) { + if (inst && inst->inst_ref_count) { slapi_counter_decrement(inst->inst_ref_count); }
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index 652424e..5f085f9 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -363,19 +363,18 @@ ldbm_back_search( Slapi_PBlock *pb ) slapi_pblock_set( pb, SLAPI_TXN, txn.back_txn_txn ); }
- inst = (ldbm_instance *) be->be_instance_info; - if (NULL == basesdn) { slapi_send_ldap_result( pb, LDAP_INVALID_DN_SYNTAX, NULL, "Null target DN", 0, NULL ); return( -1 ); } - if (inst->inst_ref_count) { + inst = (ldbm_instance *) be->be_instance_info; + if (inst && inst->inst_ref_count) { slapi_counter_increment(inst->inst_ref_count); } else { LDAPDebug1Arg(LDAP_DEBUG_ANY, - "ldbm_search: instance %s does not exist.\n", - inst->inst_name); + "ldbm_search: instance "%s" does not exist.\n", + inst ? inst->inst_name : "null instance"); return( -1 ); } base = slapi_sdn_get_dn(basesdn);
389-commits@lists.fedoraproject.org