memory leak in range searches

[389-ds-bot]

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/47517

• Created at 2013-09-19 13:31:52 by lkrispen (@elkris)
• Closed as Fixed
• Assigned to mreynolds (@mreynolds389)
• Associated bugzillas
  • https://bugzilla.redhat.com/show_bug.cgi?id=1016717

---

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)

[389-ds-bot]

Comment from lkrispen (@elkris) at 2013-09-19 13:35:43

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

[389-ds-bot]

Comment from rmeggins (@richm) at 2013-10-05 04:17:55

Where is saved_key freed? The call to

    DBT_FREE_PAYLOAD(cur_key);

was removed

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2013-10-05 04:32:55

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.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2013-10-07 19:41:48

Replying to [comment:5 richm]:

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.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2013-10-07 19:45:22

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.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2013-10-07 19:58:17

revision
0001-Ticket-47517-memory-leak-in-range-searches-and-other.patch

[389-ds-bot]

Comment from rmeggins (@richm) at 2013-10-07 21:07:57

Replying to [comment:7 mreynolds389]:

Replying to [comment:5 richm]:

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;

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2013-10-07 21:22:22

Replying to [comment:9 richm]:

Replying to [comment:7 mreynolds389]:

Replying to [comment:5 richm]:

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.

[389-ds-bot]

Comment from rmeggins (@richm) at 2013-10-07 22:11:22

ack

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2013-10-07 22:23:38

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 mreynolds389@redhat.com
Date: Mon Oct 7 09:57:47 2013 -0400

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2013-10-08 20:30:52

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

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2013-10-08 20:57:10

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1016717

[389-ds-bot]

Comment from rmeggins (@richm) at 2013-10-11 20:27:32

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

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2017-02-11 23:04:14

Metadata Update from @mreynolds389:

• Issue assigned to mreynolds389
• Issue set to the milestone: 1.3.3 - 10/13 (October)