From ceef0b6ae9edbb60bc6324c3dc045f3a4e5fd725 Mon Sep 17 00:00:00 2001 From: Thierry Bordaz 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