search

389ds / 389-ds-base

The enterprise-class Open Source LDAP server for Linux
https://www.port389.org/
Other
210 stars 90 forks source link

memory leaks in 389-ds-base-1.2.8.2-1.el5? #51

Closed 389-ds-bot closed 4 years ago

389-ds-bot commented 4 years ago

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

https://bugzilla.redhat.com/show_bug.cgi?id=697701

Description of problem: valgrind output for a GSSAPI request with a MMR enabled
server shows memory leaks?

Version-Release number of selected component (if applicable):
389-ds-base-1.2.8.2-1.el5

How reproducible: run server under valgrind (per instructions in bug 694195).

Steps to Reproduce:
1. Setup pair of LDAP servers, MMR between them (plain auth via plain LDAP for
that). SELinux is disabled, avoiding memory leak from that:

# getenforce
Disabled

2. Launch server under valgrind.
3. Hit server with single read-only GSSAPI request.

Actual results: see attachment for valgrind output.

Expected results: no reported memory leaks from valgrind? (Or is the output
normal?)
389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2012-02-04 03:19:47

git patch file (master) 0001-Trac-Ticket-51-memory-leaks-in-389-ds-base-1.2.8.2-1.patch

389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2012-02-04 03:20:54

Fix description: Ran valgrind with the MMR+SASL servers and fixed leaks found in the test. [plugin/replication/repl5_connection.c] conn_connect could have overridden conn->ld without releasing it. This patch releases it if necessary. [slapd/dn.c] If DN normalization fails in slapi_sdn_get_dn, this patch releases the locally strdup'ed string. [slapd/modify.c, modutil.c] DN syntax attribute value is found in mods, it was normalized and replaced in slapi_mods_insert_at. It leaked the pre- noralized value. Instead, this patch normalizes mods in do_modify and frees it when the modify is done. [slapd/operation.c] modrdn_newsuperior_address.sdn was not release when the modrdn operaton is done. This patch adds the release code.

389-ds-bot commented 4 years ago

Comment from rmeggins (@richm) at 2012-02-04 03:52:20

slapi_mods_insert_at is called indirectly from slapi_mods_add_ldapmod() and slapi_mods_add_smod() in a number of places in the server, some of which do not directly call a modify operation. Are you sure it is safe to remove the DN normalization from slapi_mods_insert_at()?

Rather, it should not have been there... :( The problem is bv_val is reset there without releasing the previous value. But slapi_mods_insert_at has no idea the original value is on the stack or on the heap... So, it's not a good idea to touch it there.

The way do_modify normalizes the entire mods is the same was being done in slapi_modify_internal (actually, modify_internal_pb). So, now the external op and the internal one are taking the same steps.

389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2012-02-07 03:58:41

Reviewed by Rich (Thank you!!)

Pushed to master.

$ git merge trac51 Updating b8a874a..d4eedec Fast-forward .../servers/plugins/replication/repl5_connection.c | 8 ++++++-- ldap/servers/slapd/dn.c | 3 ++- ldap/servers/slapd/modify.c | 17 ++++++++++++++--- ldap/servers/slapd/modutil.c | 13 ------------- ldap/servers/slapd/operation.c | 1 + ldap/servers/slapd/search.c | 2 +- 6 files changed, 24 insertions(+), 20 deletions(-)

$ git push Counting objects: 25, done. Delta compression using up to 4 threads. Compressing objects: 100% (13/13), done. Writing objects: 100% (13/13), 1.69 KiB, done. Total 13 (delta 11), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git b8a874a..d4eedec master -> master

389-ds-bot commented 4 years ago

Comment from rmeggins (@richm) at 2012-02-09 05:08:36

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

389-ds-bot commented 4 years ago

Comment from nkinder (@nkinder) at 2012-08-28 04:14:26

Added initial screened field value.

389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2017-02-11 22:50:42

Metadata Update from @nhosoi: