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):
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:
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
git patch file (master)
Fix description: Ran valgrind with the MMR+SASL servers and
fixed leaks found in the test.
conn_connect could have overridden conn->ld without releasing
it. This patch releases it if necessary.
If DN normalization fails in slapi_sdn_get_dn, this patch
releases the locally strdup'ed string.
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.
modrdn_newsuperior_address.sdn was not release when the modrdn
operaton is done. This patch adds the release code.
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.
- (*mbvp)->bv_val = slapi_ch_strdup(slapi_sdn_get_dn(sdn));
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.
Reviewed by Rich (Thank you!!)
Pushed to master.
$ git merge trac51
.../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)
b8a874a..d4eedec master -> master
Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=788754
Added initial screened field value.
Metadata Update from @nhosoi:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.2.10
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:
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)
to comment on this ticket.