rpms/389-ds-base
6
0
Fork 0
Code Issues Pull Requests Releases Activity
c9
389-ds-base/SOURCES/0025-Issue-6427-fix-various-memory-leaks.patch

220 lines
8.4 KiB
Diff
Raw Permalink Blame History

From 1b48c4174857db5c7c1dc1aa6ad3d0a5155c72c5 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Thu, 21 Nov 2024 16:56:45 -0500
Subject: [PATCH] Issue 6427 - fix various memory leaks
Description:
This was all on main branch
- Display attributes were leaked on searches
- Work thread indexes were leaked at shutdown
- Accept thread was leaked at shutdown
- When we reused a search pblock, we did not free the "operation" and "search
attributes" before overwriting them with freshly allocated structures.
Fixes: https://github.com/389ds/389-ds-base/issues/6427
Reviewed by: progier (Thanks!)
---
ldap/servers/slapd/connection.c | 30 ++++++++++++++-----------
ldap/servers/slapd/daemon.c | 7 ++++--
ldap/servers/slapd/plugin_internal_op.c | 18 +++++++++++++++
ldap/servers/slapd/proto-slap.h | 1 +
ldap/servers/slapd/result.c | 5 ++---
5 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index 07d629475..bb4fcd77f 100644
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -40,20 +40,22 @@ static void log_ber_too_big_error(const Connection *conn,
static PRStack *op_stack; /* stack of Slapi_Operation * objects so we don't have to malloc/free every time */
static PRInt32 op_stack_size; /* size of op_stack */
-
struct Slapi_op_stack
{
PRStackElem stackelem; /* must be first in struct for PRStack to work */
Slapi_Operation *op;
};
-static void add_work_q(work_q_item *, struct Slapi_op_stack *);
-static work_q_item *get_work_q(struct Slapi_op_stack **);
+/* worker threads */
+static int32_t max_threads = 0;
+static int32_t *threads_indexes = NULL;
/*
* We maintain a global work queue of items that have not yet
* been handed off to an operation thread.
*/
+static void add_work_q(work_q_item *, struct Slapi_op_stack *);
+static work_q_item *get_work_q(struct Slapi_op_stack **);
struct Slapi_work_q
{
PRStackElem stackelem; /* must be first in struct for PRStack to work */
@@ -430,9 +432,7 @@ void
init_op_threads()
{
pthread_condattr_t condAttr;
- int32_t max_threads = config_get_threadnumber();
int32_t rc;
- int32_t *threads_indexes;
/* Initialize the locks and cv */
if ((rc = pthread_mutex_init(&work_q_lock, NULL)) != 0) {
@@ -463,13 +463,13 @@ init_op_threads()
op_stack = PR_CreateStack("connection_operation");
alloc_per_thread_snmp_vars(max_threads);
init_thread_private_snmp_vars();
-
+ max_threads = config_get_threadnumber();
threads_indexes = (int32_t *) slapi_ch_calloc(max_threads, sizeof(int32_t));
for (size_t i = 0; i < max_threads; i++) {
threads_indexes[i] = i + 1; /* idx 0 is reserved for global snmp_vars */
}
-
+
/* start the operation threads */
for (size_t i = 0; i < max_threads; i++) {
PR_SetConcurrency(4);
@@ -486,10 +486,14 @@ init_op_threads()
g_incr_active_threadcnt();
}
}
- /* Here we should free thread_indexes, but because of the dynamic of the new
- * threads (connection_threadmain) we are not sure when it can be freed.
- * Let's accept that unique initialization leak (typically 32 to 64 bytes)
- */
+ /* We will free threads_indexes at the very end of slapd_daemon() */
+}
+
+/* Called at shutdown to silence ASAN and friends */
+void
+free_worker_thread_indexes()
+{
+ slapi_ch_free((void **)&threads_indexes);
}
static void
@@ -1227,7 +1231,7 @@ connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, int *
/* Process HAProxy header */
if (conn->c_haproxyheader_read == 0) {
conn->c_haproxyheader_read = 1;
- /*
+ /*
* We only check for HAProxy header if nsslapd-haproxy-trusted-ip is configured.
* If it is we proceed with the connection only if it's comming from trusted
* proxy server with correct and complete header.
@@ -1253,7 +1257,7 @@ connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, int *
/* Now, reset RC and set it to 0 only if a match is found */
haproxy_rc = -1;
- /*
+ /*
* We need to allow a configuration where DS instance and HAProxy are on the same machine.
* In this case, we need to check if
* the HAProxy client IP (which will be a loopback address) matches one of the the trusted IP addresses,
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
index 1a20796b9..c09e54c7f 100644
--- a/ldap/servers/slapd/daemon.c
+++ b/ldap/servers/slapd/daemon.c
@@ -1323,8 +1323,11 @@ slapd_daemon(daemon_ports_t *ports)
slapi_log_err(SLAPI_LOG_ERR, "slapd_daemon", "Failed to remove pid file %s\n", get_pid_file());
}
}
-}
+ /* final cleanup for ASAN and other analyzers */
+ PR_JoinThread(accept_thread_p);
+ free_worker_thread_indexes();
+}
void
ct_thread_cleanup(void)
@@ -1654,7 +1657,7 @@ handle_pr_read_ready(Connection_Table *ct, int list_num, PRIntn num_poll __attri
continue;
}
- /* Try to get connection mutex, if not available just skip the connection and
+ /* Try to get connection mutex, if not available just skip the connection and
* process other connections events. May generates cpu load for listening thread
* if connection mutex is held for a long time
*/
diff --git a/ldap/servers/slapd/plugin_internal_op.c b/ldap/servers/slapd/plugin_internal_op.c
index 0d553d1ba..0164a70be 100644
--- a/ldap/servers/slapd/plugin_internal_op.c
+++ b/ldap/servers/slapd/plugin_internal_op.c
@@ -249,6 +249,12 @@ slapi_search_internal_set_pb(Slapi_PBlock *pb, const char *base, int scope, cons
return;
}
+ /* Free op just in case this pb is being reused */
+ slapi_pblock_get(pb, SLAPI_OPERATION, &op);
+ operation_free(&op, NULL);
+ slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &tmp_attrs);
+ slapi_ch_array_free(tmp_attrs);
+
op = internal_operation_new(SLAPI_OPERATION_SEARCH, operation_flags);
slapi_pblock_set(pb, SLAPI_OPERATION, op);
slapi_pblock_set(pb, SLAPI_ORIGINAL_TARGET_DN, (void *)base);
@@ -276,6 +282,12 @@ slapi_search_internal_set_pb_ext(Slapi_PBlock *pb, Slapi_DN *sdn, int scope, con
return;
}
+ /* Free op/attrs just in case this pb is being reused */
+ slapi_pblock_get(pb, SLAPI_OPERATION, &op);
+ operation_free(&op, NULL);
+ slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &tmp_attrs);
+ slapi_ch_array_free(tmp_attrs);
+
op = internal_operation_new(SLAPI_OPERATION_SEARCH, operation_flags);
slapi_pblock_set(pb, SLAPI_OPERATION, op);
slapi_pblock_set(pb, SLAPI_ORIGINAL_TARGET_DN,
@@ -305,6 +317,12 @@ slapi_seq_internal_set_pb(Slapi_PBlock *pb, char *base, int type, char *attrname
return;
}
+ /* Free op/attrs just in case this pb is being reused */
+ slapi_pblock_get(pb, SLAPI_OPERATION, &op);
+ operation_free(&op, NULL);
+ slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &tmp_attrs);
+ slapi_ch_array_free(tmp_attrs);
+
op = internal_operation_new(SLAPI_OPERATION_SEARCH, operation_flags);
slapi_pblock_set(pb, SLAPI_OPERATION, op);
slapi_pblock_set(pb, SLAPI_ORIGINAL_TARGET_DN, (void *)base);
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index 214f2e609..2031cfb7d 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -1543,6 +1543,7 @@ int connection_is_free(Connection *conn, int user_lock);
int connection_is_active_nolock(Connection *conn);
ber_slen_t openldap_read_function(Sockbuf_IO_Desc *sbiod, void *buf, ber_len_t len);
int32_t connection_has_psearch(Connection *c);
+void free_worker_thread_indexes(void);
/*
* saslbind.c
diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c
index 97af5a2b8..9772e8213 100644
--- a/ldap/servers/slapd/result.c
+++ b/ldap/servers/slapd/result.c
@@ -1392,9 +1392,8 @@ exit:
if (NULL != typelist) {
slapi_vattr_attrs_free(&typelist, typelist_flags);
}
- if (NULL != default_attrs) {
- slapi_ch_free((void **)&default_attrs);
- }
+ slapi_ch_array_free(default_attrs);
+
return rc;
}
--
2.48.1
Reference in New Issue View Git Blame Copy Permalink