From ceef0b6ae9edbb60bc6324c3dc045f3a4e5fd725 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz <tbordaz@redhat.com>
Date: Fri, 8 Nov 2019 18:16:06 +0100
Subject: [PATCH] Ticket 50709: Several memory leaks reported by Valgrind for
389-ds 1.3.9.1-10
Description of the problem:
When evaluating an ACI with 'ip' subject, it adds a PRNetAddr to the subject
property list. When the list is free (acl__done_aclpb) the property is not freed.
Description of the fix:
Add the property to the pblock (SLAPI_CONN_CLIENTNETADDR_ACLIP) so that it
the property is freed with acl pblock.
https://pagure.io/389-ds-base/issue/50709
Reviewed by: Mark Reynolds, William Brown, Ludwig Krispenz
---
ldap/servers/plugins/acl/acllas.c | 54 ++++++++++++++++++++-----------
ldap/servers/slapd/connection.c | 2 ++
ldap/servers/slapd/pblock.c | 16 +++++++++
ldap/servers/slapd/slap.h | 1 +
ldap/servers/slapd/slapi-plugin.h | 1 +
5 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/ldap/servers/plugins/acl/acllas.c b/ldap/servers/plugins/acl/acllas.c
index 3950fd405..a5602e198 100644
--- a/ldap/servers/plugins/acl/acllas.c
+++ b/ldap/servers/plugins/acl/acllas.c
@@ -251,6 +251,7 @@ DS_LASIpGetter(NSErr_t *errp, PList_t subject, PList_t resource, PList_t auth_in
{
struct acl_pblock *aclpb = NULL;
PRNetAddr *client_praddr = NULL;
+ PRNetAddr *pb_client_praddr = NULL;
char ip_str[256];
int rv = LAS_EVAL_TRUE;
@@ -262,25 +263,39 @@ DS_LASIpGetter(NSErr_t *errp, PList_t subject, PList_t resource, PList_t auth_in
return LAS_EVAL_FAIL;
}
- client_praddr = (PRNetAddr *)slapi_ch_malloc(sizeof(PRNetAddr));
- if (client_praddr == NULL) {
- slapi_log_err(SLAPI_LOG_ERR, plugin_name, "DS_LASIpGetter - Failed to allocate client_praddr\n");
- return (LAS_EVAL_FAIL);
- }
+ slapi_pblock_get(aclpb->aclpb_pblock, SLAPI_CONN_CLIENTNETADDR_ACLIP, &pb_client_praddr);
+ if (pb_client_praddr == NULL) {
- if (slapi_pblock_get(aclpb->aclpb_pblock, SLAPI_CONN_CLIENTNETADDR, client_praddr) != 0) {
- slapi_log_err(SLAPI_LOG_ERR, plugin_name, "DS_LASIpGetter - Could not get client IP.\n");
- slapi_ch_free((void **)&client_praddr);
- return (LAS_EVAL_FAIL);
- }
+ client_praddr = (PRNetAddr *) slapi_ch_malloc(sizeof (PRNetAddr));
+ if (client_praddr == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, plugin_name, "DS_LASIpGetter - Failed to allocate client_praddr\n");
+ return (LAS_EVAL_FAIL);
+ }
- rv = PListInitProp(subject, 0, ACL_ATTR_IP, (void *)client_praddr, NULL);
- if (rv < 0) {
- slapi_log_err(SLAPI_LOG_ACL, plugin_name, "DS_LASIpGetter - "
- "Couldn't set the client addr property(%d)\n",
- rv);
- slapi_ch_free((void **)&client_praddr);
- return LAS_EVAL_FAIL;
+ if (slapi_pblock_get(aclpb->aclpb_pblock, SLAPI_CONN_CLIENTNETADDR, client_praddr) != 0) {
+ slapi_log_err(SLAPI_LOG_ERR, plugin_name, "DS_LASIpGetter - Could not get client IP.\n");
+ slapi_ch_free((void **) &client_praddr);
+ return (LAS_EVAL_FAIL);
+ }
+
+ rv = PListInitProp(subject, 0, ACL_ATTR_IP, (void *) client_praddr, NULL);
+ if (rv < 0) {
+ slapi_log_err(SLAPI_LOG_ACL, plugin_name, "DS_LASIpGetter - "
+ "Couldn't set the client addr property(%d)\n",
+ rv);
+ slapi_ch_free((void **) &client_praddr);
+ return LAS_EVAL_FAIL;
+ }
+
+ } else {
+ client_praddr = pb_client_praddr;
+ rv = PListInitProp(subject, 0, ACL_ATTR_IP, (void *) client_praddr, NULL);
+ if (rv < 0) {
+ slapi_log_err(SLAPI_LOG_ACL, plugin_name, "DS_LASIpGetter - "
+ "Couldn't set the client addr property(%d)\n",
+ rv);
+ return LAS_EVAL_FAIL;
+ }
}
if (PR_NetAddrToString(client_praddr, ip_str, sizeof(ip_str)) == PR_SUCCESS) {
slapi_log_err(SLAPI_LOG_ACL, plugin_name, "DS_LASIpGetter - "
@@ -290,7 +305,10 @@ DS_LASIpGetter(NSErr_t *errp, PList_t subject, PList_t resource, PList_t auth_in
slapi_log_err(SLAPI_LOG_ACL, plugin_name, "DS_LASIpGetter - "
"Returning client ip address 'unknown'\n");
}
-
+ if (client_praddr != pb_client_praddr) {
+ /* Set it in pblock only if it is newly allocated */
+ slapi_pblock_set(aclpb->aclpb_pblock, SLAPI_CONN_CLIENTNETADDR_ACLIP, client_praddr);
+ }
return LAS_EVAL_TRUE;
}
diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index 9abd546f9..b9b280e6d 100644
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -205,6 +205,7 @@ connection_cleanup(Connection *conn)
conn->c_isreplication_session = 0;
slapi_ch_free((void **)&conn->cin_addr);
slapi_ch_free((void **)&conn->cin_destaddr);
+ slapi_ch_free((void **)&conn->cin_addr_aclip);
slapi_ch_free_string(&conn->c_ipaddr);
if (conn->c_domain != NULL) {
ber_bvecfree(conn->c_domain);
@@ -397,6 +398,7 @@ connection_reset(Connection *conn, int ns, PRNetAddr *from, int fromLen __attrib
str_destip = str_unknown;
}
}
+ slapi_ch_free((void **)&conn->cin_addr_aclip);
if (!in_referral_mode) {
diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c
index bc18a7b18..d2ad6147a 100644
--- a/ldap/servers/slapd/pblock.c
+++ b/ldap/servers/slapd/pblock.c
@@ -482,6 +482,14 @@ slapi_pblock_get(Slapi_PBlock *pblock, int arg, void *value)
}
PR_ExitMonitor(pblock->pb_conn->c_mutex);
break;
+ case SLAPI_CONN_CLIENTNETADDR_ACLIP:
+ if (pblock->pb_conn == NULL) {
+ break;
+ }
+ pthread_mutex_lock(&(pblock->pb_conn->c_mutex));
+ (*(PRNetAddr **) value) = pblock->pb_conn->cin_addr_aclip;
+ pthread_mutex_unlock(&(pblock->pb_conn->c_mutex));
+ break;
case SLAPI_CONN_SERVERNETADDR:
if (pblock->pb_conn == NULL) {
memset(value, 0, sizeof(PRNetAddr));
@@ -2571,6 +2579,14 @@ slapi_pblock_set(Slapi_PBlock *pblock, int arg, void *value)
pblock->pb_conn->c_authtype = slapi_ch_strdup((char *)value);
PR_ExitMonitor(pblock->pb_conn->c_mutex);
break;
+ case SLAPI_CONN_CLIENTNETADDR_ACLIP:
+ if (pblock->pb_conn == NULL) {
+ break;
+ }
+ pthread_mutex_lock(&(pblock->pb_conn->c_mutex));
+ slapi_ch_free((void **)&pblock->pb_conn->cin_addr_aclip);
+ pblock->pb_conn->cin_addr_aclip = (PRNetAddr *)value;
+ pthread_mutex_unlock(&(pblock->pb_conn->c_mutex));
case SLAPI_CONN_IS_REPLICATION_SESSION:
if (pblock->pb_conn == NULL) {
slapi_log_err(SLAPI_LOG_ERR,
diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h
index a8908d94c..4c53d43dc 100644
--- a/ldap/servers/slapd/slap.h
+++ b/ldap/servers/slapd/slap.h
@@ -1617,6 +1617,7 @@ typedef struct conn
char *c_external_dn; /* client DN of this SSL session */
char *c_external_authtype; /* used for c_external_dn */
PRNetAddr *cin_addr; /* address of client on this conn */
+ PRNetAddr *cin_addr_aclip; /* address of client allocated by acl with 'ip' subject */
PRNetAddr *cin_destaddr; /* address client connected to */
struct berval **c_domain; /* DNS names of client */
Operation *c_ops; /* list of pending operations */
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 0bc3a6fab..679bdbb5c 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6971,6 +6971,7 @@ slapi_timer_result slapi_timespec_expire_check(struct timespec *expire);
#define SLAPI_CONN_DN 143
#define SLAPI_CONN_CLIENTNETADDR 850
#define SLAPI_CONN_SERVERNETADDR 851
+#define SLAPI_CONN_CLIENTNETADDR_ACLIP 853
#define SLAPI_CONN_IS_REPLICATION_SESSION 149
#define SLAPI_CONN_IS_SSL_SESSION 747
#define SLAPI_CONN_CERT 743
--
2.24.1