Skip to content

Commit

Permalink
Issue 6555 - Potential crash when deleting a replicated backend (389d…
Browse files Browse the repository at this point in the history
…s#6559)

Problem: connection cleanup may try to access a freed replica
Solution: Ensure that the replica is still existing before using its pointer

Issue: 389ds#6555

Reviewed by: @tbordaz, @droideck (Thanks!)
  • Loading branch information
progier389 authored Jan 30, 2025
1 parent e3b8986 commit f5e5b59
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
3 changes: 3 additions & 0 deletions ldap/servers/plugins/replication/repl5.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "repl5_ruv.h"
#include "plstr.h"
#include <pthread.h>
#include <stdbool.h>

#define START_UPDATE_DELAY 2 /* 2 second */
#define REPLICA_TYPE_WINDOWS 1
Expand Down Expand Up @@ -710,6 +711,8 @@ PRBool replica_is_updatedn(Replica *r, const Slapi_DN *sdn);
void replica_set_updatedn(Replica *r, const Slapi_ValueSet *vs, int mod_op);
void replica_set_groupdn(Replica *r, const Slapi_ValueSet *vs, int mod_op);
char *replica_get_generation(const Replica *r);
bool replica_check_validity(Replica *replica);


/* currently supported flags */
#define REPLICA_LOG_CHANGES 0x1 /* enable change logging */
Expand Down
36 changes: 36 additions & 0 deletions ldap/servers/plugins/replication/repl5_replica_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,18 @@ struct repl_enum_data
void *arg;
};

struct repl_enum_validity
{
Replica *replica;
bool is_valid;
};


/* Forward declarations */
static PRIntn replica_destroy_hash_entry(PLHashEntry *he, PRIntn index, void *arg);
static PRIntn replica_enumerate(PLHashEntry *he, PRIntn index, void *hash_data);
static PRIntn replica_validity_cb(PLHashEntry *he, PRIntn index, void *arg);



int
Expand Down Expand Up @@ -192,6 +201,21 @@ replica_enumerate_replicas(FNEnumReplica fn, void *arg)
slapi_rwlock_unlock(s_lock);
}

/* Check that the replica still exists */
bool
replica_check_validity(Replica *replica)
{
struct repl_enum_validity data = { replica, false };

if (replica == NULL) {
return false;
}
slapi_rwlock_rdlock(s_lock);
PL_HashTableEnumerateEntries(s_hash, replica_validity_cb, &data);
slapi_rwlock_unlock(s_lock);
return data.is_valid;
}

/* Helper functions */

/* this function called for each hash node during hash destruction */
Expand Down Expand Up @@ -224,3 +248,15 @@ replica_enumerate(PLHashEntry *he, PRIntn index __attribute__((unused)), void *h

return HT_ENUMERATE_NEXT;
}

static PRIntn
replica_validity_cb(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg)
{
struct repl_enum_validity *data = arg;
Replica *r = (Replica *)he->value;
if (r == data->replica) {
data->is_valid = true;
return HT_ENUMERATE_STOP;
}
return HT_ENUMERATE_NEXT;
}
2 changes: 1 addition & 1 deletion ldap/servers/plugins/replication/repl_connext.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ consumer_connection_extension_destructor(void *ext, void *object __attribute__((
* a replica. If so, release it here.
*/
consumer_connection_extension *connext = (consumer_connection_extension *)ext;
if (NULL != connext->replica_acquired) {
if (replica_check_validity(connext->replica_acquired)) {
Replica *r = connext->replica_acquired;
/* If a total update was in progress, abort it */
if (REPL_PROTOCOL_50_TOTALUPDATE == connext->repl_protocol_version) {
Expand Down

0 comments on commit f5e5b59

Please sign in to comment.