From f5e5b5939d0f65a9abf1f301bdfb1ff707a26d9e Mon Sep 17 00:00:00 2001 From: progier389 Date: Thu, 30 Jan 2025 14:44:39 +0100 Subject: [PATCH] Issue 6555 - Potential crash when deleting a replicated backend (#6559) Problem: connection cleanup may try to access a freed replica Solution: Ensure that the replica is still existing before using its pointer Issue: #6555 Reviewed by: @tbordaz, @droideck (Thanks!) --- ldap/servers/plugins/replication/repl5.h | 3 ++ .../plugins/replication/repl5_replica_hash.c | 36 +++++++++++++++++++ .../plugins/replication/repl_connext.c | 2 +- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h index 6e5552f54e..027d8634d4 100644 --- a/ldap/servers/plugins/replication/repl5.h +++ b/ldap/servers/plugins/replication/repl5.h @@ -29,6 +29,7 @@ #include "repl5_ruv.h" #include "plstr.h" #include +#include #define START_UPDATE_DELAY 2 /* 2 second */ #define REPLICA_TYPE_WINDOWS 1 @@ -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 */ diff --git a/ldap/servers/plugins/replication/repl5_replica_hash.c b/ldap/servers/plugins/replication/repl5_replica_hash.c index bb7b054c69..275e5f7a8c 100644 --- a/ldap/servers/plugins/replication/repl5_replica_hash.c +++ b/ldap/servers/plugins/replication/repl5_replica_hash.c @@ -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 @@ -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 */ @@ -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; +} diff --git a/ldap/servers/plugins/replication/repl_connext.c b/ldap/servers/plugins/replication/repl_connext.c index 19cb396657..7b3041e1f6 100644 --- a/ldap/servers/plugins/replication/repl_connext.c +++ b/ldap/servers/plugins/replication/repl_connext.c @@ -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) {