#47517 memory leak in range searches
Closed: wontfix None Opened 10 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)

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/854

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata