dna memleak reported by valgrind

[389-ds-bot]

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

• Created at 2012-07-13 22:07:55 by rmeggins (@richm)
• Closed as Fixed
• Assigned to mreynolds (@mreynolds389)
• Associated bugzillas
  • https://bugzilla.redhat.com/show_bug.cgi?id=842437

---

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)

[389-ds-bot]

Comment from nkinder (@nkinder) at 2012-07-24 02:55:11

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

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2012-08-02 02:42:21

Sending fix out for review...

[389-ds-bot]

Comment from rmeggins (@richm) at 2012-08-02 02:52:37

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.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2012-08-02 03:28:43

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.

[389-ds-bot]

Comment from rmeggins (@richm) at 2012-08-02 03:47:08

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.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2012-08-02 04:11:19

Ok, here is patch number 3...

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2012-08-02 04:11:35

3 0001-Ticket-407-memory-leak-in-dna-plugin.2.patch

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2012-08-02 19:40:30

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

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2012-08-07 01:36:45

Fixes crash 0001-Ticket-407-dna-memory-leak.patch

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2012-08-07 01:41:01

[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

[389-ds-bot]

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

Added initial screened field value.

[389-ds-bot]

Comment from nkinder (@nkinder) at 2017-02-11 22:51:57

Metadata Update from @nkinder:

• Issue assigned to mreynolds389
• Issue set to the milestone: 1.2.11.8