Bug 548535 - memory leak in attrcrypt
Summary: memory leak in attrcrypt
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Database - General
Version: 1.3.0
Hardware: All
OS: Linux
medium
low
Target Milestone: ---
Assignee: Noriko Hosoi
QA Contact: Viktor Ashirov
URL:
Whiteboard:
Depends On:
Blocks: 434914 389_1.2.6
TreeView+ depends on / blocked
 
Reported: 2009-12-17 18:04 UTC by Noriko Hosoi
Modified: 2015-12-07 17:03 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-07 17:03:54 UTC


Attachments (Terms of Use)
git patch file (4.37 KB, patch)
2010-02-06 02:03 UTC, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (revised based upon the comment from Rich) (4.36 KB, patch)
2010-02-08 16:55 UTC, Noriko Hosoi
no flags Details | Diff
mmraccept valgrind logs (820.00 KB, application/x-tar)
2010-06-03 17:06 UTC, Jenny Severance
no flags Details

Description Noriko Hosoi 2009-12-17 18:04:21 UTC
Description of problem:
Found in TET mmrepl/accept + valgrind
accept.vg
==31192== 1,392 (48 direct, 1,344 indirect) bytes in 2 blocks are definitely lost in loss record 32 of 80
==31192==    at 0x4A0776F: realloc (vg_replace_malloc.c:429)
==31192==    by 0x4C50531: slapi_ch_realloc (ch_malloc.c:199)
==31192==    by 0xB241EA2: attrcrypt_acs_list_add (ldbm_attrcrypt.c:449)
==31192==    by 0xB242047: attrcrypt_init (ldbm_attrcrypt.c:483)
==31192==    by 0xB2184FD: dblayer_instance_start (dblayer.c:1840)
==31192==    by 0xB234E82: bulk_import_start (import-threads.c:1789)
==31192==    by 0xB235693: ldbm_back_wire_import (import-threads.c:2001)
==31192==    by 0x4C4F627: process_bulk_import_op (bulk_import.c:176)
==31192==    by 0x4C4F381: slapi_start_bulk_import (bulk_import.c:76)
==31192==    by 0xB4C6B2D: multimaster_extop_StartNSDS50ReplicationRequest (repl_extop.c:835)
==31192==    by 0x4C99634: plugin_call_exop_plugins (plugin.c:424)
==31192==    by 0x41B28E: do_extended (extendop.c:349)
==31192==    by 0x413417: connection_dispatch_operation (connection.c:617)
==31192==    by 0x4149A1: connection_threadmain (connection.c:2267)
==31192==    by 0x32B7429262: (within /lib64/libnspr4.so)
==31192==    by 0x37A6406869: start_thread (in /lib64/libpthread-2.10.1.so)
==31192==    by 0x37A58DE3BC: clone (in /lib64/libc-2.10.1.so)

Note: attrcrypt_cleanup never be called.
$ egrep attrcrypt_cleanup *.[ch]
ldbm_attrcrypt.c:attrcrypt_cleanup(attrcrypt_cipher_state *acs)
ldbm_attrcrypt.c:    LDAPDebug(LDAP_DEBUG_TRACE,"-> attrcrypt_cleanup\n", 0, 0, 0);
ldbm_attrcrypt.c:    LDAPDebug(LDAP_DEBUG_TRACE,"<- attrcrypt_cleanup\n", 0, 0, 0);

Comment 2 Noriko Hosoi 2010-02-06 02:03:22 UTC
Created attachment 389229 [details]
git patch file

Files:
	ldap/servers/slapd/back-ldbm/dblayer.c
	ldap/servers/slapd/back-ldbm/ldbm_attrcrypt.c
	ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

Description:
The attrcrypt module maintains the inst_attrcrypt_state_private
field in the instance structure (ldbm_instance) to store the private
keys.  The area and the space for the private keys are allocated in
attrcrypt_init which is called from dblayer_instance_start.
A backend instance could be closed and restarted multiple times
(for instance, in the bulk_import, which is used by the replica
initialization), but the area had no chance to be freed.
This patch is adding the clean-up code.

Comment 3 Rich Megginson 2010-02-08 15:18:04 UTC
Comment on attachment 389229 [details]
git patch file

The formatting in attrcrypt_cleanup_private() of the LDAPDebug macros is not correct - looks like you need tabs there.

Otherwise, looks good.

Comment 4 Noriko Hosoi 2010-02-08 16:55:01 UTC
Created attachment 389573 [details]
git patch file (revised based upon the comment from Rich)

(In reply to comment #3)
> (From update of attachment 389229 [details])
> The formatting in attrcrypt_cleanup_private() of the LDAPDebug macros is not
> correct - looks like you need tabs there.
> 
> Otherwise, looks good.    

Rich, thank you for reviewing the patch!

Pushed to master.

$ git merge work
Updating 5c859f5..571f580
Fast forward
 ldap/servers/slapd/back-ldbm/dblayer.c         |    8 ++++++-
 ldap/servers/slapd/back-ldbm/ldbm_attrcrypt.c  |   28 +++++++++++++++++++++++-
 ldap/servers/slapd/back-ldbm/proto-back-ldbm.h |    1 +
 3 files changed, 35 insertions(+), 2 deletions(-)
$ git push
Counting objects: 17, done.
Delta compression using 4 threads.
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 1.47 KiB, done.
Total 9 (delta 7), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   5c859f5..571f580  master -> master

Comment 5 Jenny Severance 2010-06-03 17:06:19 UTC
Created attachment 419447 [details]
mmraccept valgrind logs

Comment 6 Jenny Severance 2010-06-03 17:18:02 UTC
please review the attached valgrind logs for memory leak fix.  Thanks!

Comment 7 Noriko Hosoi 2010-06-03 17:39:23 UTC
Thanks, Jenny.  The particular leak does not exist in the valgrind logs.

$ tar xvf ../valgrind_mmraccept.tar 
accept.vg.12390.12396
accept.vg.12697.12703
accept.vg.12994.13000
[...]
accept.vg.7935.7941
accept.vg.8372.8378
accept.vg.8811.8817
$ egrep attrcrypt_acs_list_add *
$

Comment 8 Jenny Severance 2010-06-03 17:59:51 UTC
verified - RHEL 4 - Thanks Noriko!

version:
redhat-ds-base-8.2.0-2010060204.el4dsrv


Note You need to log in before you can comment on or make changes to this bug.