389-ds-base

#407 dna memleak reported by valgrind
Closed: wontfix Opened by rmeggins.

Closed: wontfix
Reopen Issue

Running the multi_plugin test in valgrind:
==27981== 24 bytes in 1 blocks are definitely lost in loss record 248 of 1,456
==27981== at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==27981== by 0x4C57EE9: slapi_ch_calloc (ch_malloc.c:243)
==27981== by 0x4CA11AC: slapi_mods_new (modutil.c:93)
==27981== by 0xB0FAE6B: dna_pre_op (dna.c:3254)
==27981== by 0xB0FB08C: dna_mod_pre_op (dna.c:3334)
==27981== by 0x4CB2228: plugin_call_func (plugin.c:1453)
==27981== by 0x4CB20DB: plugin_call_list (plugin.c:1415)
==27981== by 0x4CB066F: plugin_call_plugins (plugin.c:398)
==27981== by 0xB577B8B: ldbm_back_modify (ldbm_modify.c:442)
==27981== by 0x4C9E6B6: op_shared_modify (modify.c:945)
==27981== by 0x4C9D8E2: modify_internal_pb (modify.c:616)
==27981== by 0x4C9D45E: slapi_modify_internal_pb (modify.c:471)
==27981== by 0x4CBF4F3: pw_mod_allowchange_aci (pw.c:1324)
==27981== by 0x4291ED: pw_init (pw_mgmt.c:304)
==27981== by 0x42150F: main (main.c:1217)

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

Sending fix out for review...

It's not that simple. The problem is that you cannot just call slapi_mods_free(&smods) in every case.
{{{
slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods);
smods = slapi_mods_new();
slapi_mods_init_passin(smods, mods);
}}}
once this happens, smods owns mods. This means when you call slapi_mods_free(&smods) it will free the mods as well, since it owns that memory.

I have new patch (attached) but I can't verify it without a testcase. I would think its ok, as I just removed the allocation all together, and used a local variable.

Could still leak memory:
{{{
if (LDAP_CHANGETYPE_ADD == modtype) {
ret = _dna_pre_op_add(pb, test_e);
} else {
- ret = _dna_pre_op_modify(pb, test_e, smods);
+ ret = _dna_pre_op_modify(pb, test_e, &smods);
}
if (ret) {
goto bail;
}}}
If ret is set and it does the goto bail, if smods has been changed, those changes will be leaked. Also, it's not a good idea to use the structure directly - better to stick with the slapi API.

Ok, here is patch number 3...

Thanks for the review Rich!

git merge ticket407
Updating 657efc6..9229db2
Fast-forward
ldap/servers/plugins/dna/dna.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

git push origin master
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 854 bytes, done.
Total 7 (delta 5), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
657efc6..9229db2 master -> master

[mareynol@localhost ds]$ git merge ticket407
Updating 52e1507..60316fc
Fast-forward
ldap/servers/plugins/dna/dna.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

[mareynol@localhost ds]$ git push origin master
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 682 bytes, done.
Total 7 (delta 5), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
52e1507..60316fc master -> master

Added initial screened field value.

Metadata Update from @nkinder:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.2.11.8

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/407

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)
Metadata
None
None
None
Normal
ack
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.