#47517 memory leak in range searches
Closed: Fixed None Opened 6 years ago by lkrispen.

In a range search like
ldapsearch ... -b "cn=changelog" "(&(changenumber>=74)(changenumber<=84))"

valgrind reports the following memory leak:

==26736== 12 bytes in 3 blocks are definitely lost in loss record 128 of 1,619
==26736== at 0x4A0881C: malloc (vg_replace_malloc.c:270)
==26736== by 0x319DE4F8CB: slapi_ch_malloc (ch_malloc.c:155)
==26736== by 0x390F752176: __os_umalloc (in /usr/lib64/libdb-5.2.so)
==26736== by 0x390F71493D: __db_retcopy (in /usr/lib64/libdb-5.2.so)
==26736== by 0x390F6EC9D3: __dbc_iget (in /usr/lib64/libdb-5.2.so)
==26736== by 0x390F6FBC0B: __dbc_get_pp (in /usr/lib64/libdb-5.2.so)
==26736== by 0x93D7FFF: idl_new_range_fetch (idl_new.c:466)
==26736== by 0x93EB2A4: index_range_read_ext (index.c:1459)
==26736== by 0x93D02DC: range_candidates (filterindex.c:619)
==26736== by 0x93D0889: list_candidates (filterindex.c:773)
==26736== by 0x93CEF11: filter_candidates_ext (filterindex.c:167)
==26736== by 0x93D0B86: list_candidates (filterindex.c:836)


The leak is caused by this call:
ret = cursor->c_get(cursor, &cur_key, &data, DB_SET|DB_MULTIPLE);

The data component in cur_key is allocated and will be freed by DBT_FREE_PAYLOAD, but there are several calls to cursor->c_get(cursor, &cur_key,...) and only the last allocation is freed

Where is saved_key freed? The call to
{{{
DBT_FREE_PAYLOAD(cur_key);
}}}
was removed

ldap/servers/slapd/ldaputil.c
Good catch! But according to the man page, outvalue needs to get freed under any condition? Your patch frees it only when the hostname is duplicated...
{{{
int ldap_get_option(LDAP ld, int option, void outvalue);
LDAP_OPT_HOST_NAME
Sets/gets a space-separated list of hosts to be contacted by the library when trying to establish a con‐
nection. This is now deprecated in favor of LDAP_OPT_URI. outvalue must be a char *, and the caller is
responsible of freeing the resulting string by calling ldap_memfree(3), while invalue must be a const char
; the library duplicates the corresponding string.
}}}

Replying to [comment:5 rmeggins]:

Where is saved_key freed? The call to
{{{
DBT_FREE_PAYLOAD(cur_key);
}}}
was removed

Its gets freed on line 704, but it looks like there is one more place where we need to check if cur_key != the saved key.

Replying to [comment:6 nhosoi]:

ldap/servers/slapd/ldaputil.c
Good catch! But according to the man page, outvalue needs to get freed under any condition? Your patch frees it only when the hostname is duplicated...
{{{
int ldap_get_option(LDAP ld, int option, void outvalue);
LDAP_OPT_HOST_NAME
Sets/gets a space-separated list of hosts to be contacted by the library when trying to establish a con‐
nection. This is now deprecated in favor of LDAP_OPT_URI. outvalue must be a char *, and the caller is
responsible of freeing the resulting string by calling ldap_memfree(3), while invalue must be a const char
; the library duplicates the corresponding string.
}}}

I free the original value, and the copy gets freed later on line 1126.

Replying to [comment:7 mreynolds]:

Replying to [comment:5 rmeggins]:

Where is saved_key freed? The call to
{{{
DBT_FREE_PAYLOAD(cur_key);
}}}
was removed

Its gets freed on line 704, but it looks like there is one more place where we need to check if cur_key != the saved key.

This is line 704 after the latest patch:
{{{
LDAPDebug2Args(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns nids=%lu\n",
}}}

This is line 704 before patching:
{{{
ret = ret2;
}}}

Replying to [comment:9 rmeggins]:

Replying to [comment:7 mreynolds]:

Replying to [comment:5 rmeggins]:

Where is saved_key freed? The call to
{{{
DBT_FREE_PAYLOAD(cur_key);
}}}
was removed

Its gets freed on line 704, but it looks like there is one more place where we need to check if cur_key != the saved key.

This is line 704 after the latest patch:
{{{
LDAPDebug2Args(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns nids=%lu\n",
}}}

This is line 704 before patching:
{{{
ret = ret2;
}}}

That was with the old patch. It is line 709 with the new one:

error:
DBT_FREE_PAYLOAD(cur_key);
/ Close the cursor /
if (NULL != cursor) {

When we get to the end, cur_key is always equal to saved_key.

git merge ticket47517
Updating 64e0b37..b737882
Fast-forward
ldap/servers/slapd/back-ldbm/idl_new.c | 16 +++++++++++++++-
ldap/servers/slapd/back-ldbm/index.c | 2 --
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 1 +
ldap/servers/slapd/ldaputil.c | 3 ++-
4 files changed, 18 insertions(+), 4 deletions(-)

git push origin master
Counting objects: 19, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 1.15 KiB, done.
Total 10 (delta 8), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
64e0b37..b737882 master -> master

commit b737882
Author: Mark Reynolds mreynolds@redhat.com
Date: Mon Oct 7 09:57:47 2013 -0400

Checked into 1.3.1

4b105d0..98dd62e  389-ds-base-1.3.1 -> 389-ds-base-1.3.1

Partially checked into 1.3.0

82a50df..a0d2d52  389-ds-base-1.3.0 -> 389-ds-base-1.3.0
commit a0d2d529ad83b90367fba52e7a13e03803a9b3d1

Partially checked into 1.2.11

26c669d..200b4e4  389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 200b4e44230840a5dd970e0f6b404053545cb381

86000e4..2dd2489 389-ds-base-1.3.2 -> 389-ds-base-1.3.2
commit 2dd2489
Author: Mark Reynolds mreynolds@redhat.com
Date: Mon Oct 7 09:57:47 2013 -0400

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.3.3 - 10/13 (October)

2 years ago

Login to comment on this ticket.

Metadata