Hi folks, We are using samba 4.3 stack. We ran into some kind of memory leak issues recently. To detect those leaks, we have run samba under valgrind. We were able to detect and fix one of the leaks specific to us(not in samba). Whereas, Valgirnd also reported couple of areas as possible memory leak. Both leading to talloc_zero in add_a_service(). … ==32729== 88,320 bytes in 115 blocks are possibly lost in loss record 486 of 487 ==32729== at 0x4C27A2E: malloc (vg_replace_malloc.c:270) ==32729== by 0x853D91E: __talloc_with_prefix (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x853DAA5: __talloc (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x853DE5C: _talloc_named_const (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x8540941: _talloc_zero (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x6FBFEA2: add_a_service (loadparm.c:1327) ==32729== by 0x6FC06C7: lp_add_ipc (loadparm.c:1472) ==32729== by 0x6FC6314: lp_load_ex (loadparm.c:3747) ==32729== by 0x6FC658B: lp_load (loadparm.c:3806) ==32729== by 0x534AAD2: reload_services (server_reload.c:146) ==32729== by 0x53F04A6: check_reload (process.c:2448) ==32729== by 0x53F1025: housekeeping_fn (process.c:2807) ==32729== ==32729== 98,304 bytes in 128 blocks are possibly lost in loss record 487 of 487 ==32729== at 0x4C27A2E: malloc (vg_replace_malloc.c:270) ==32729== by 0x853D91E: __talloc_with_prefix (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x853DAA5: __talloc (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x853DE5C: _talloc_named_const (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x8540941: _talloc_zero (in /usr/lib/libtalloc.so.2.1.1) ==32729== by 0x6FBFEA2: add_a_service (loadparm.c:1327) ==32729== by 0x6FC346D: lp_do_section (loadparm.c:2617) ==32729== by 0x507B118: parse_section (tini.c:199) ==32729== by 0x507B30A: tini_parse (tini.c:284) ==32729== by 0x50734F0: pm_process (params.c:99) ==32729== by 0x6FC605E: lp_load_ex (loadparm.c:3685) ==32729== by 0x6FC658B: lp_load (loadparm.c:3806) … Looking at the code, we have found that NULL context has been used for global ServicePtr structure and for individual service records. This ServicePtrs structure has been freed on gfree_loadparm() and free_service_byindex() is supposed free the actual serviceptr records. talloc_free_children(ServicePtrs[idx]) is used in free_service_byindex(). According to its documentation, talloc_free_children() will only cleanup its child contexts and not the actual one. Instead of using NULL we can use "ServicePtrs” as context for allocating individual records. Valgrind is happy after these changes. No leak reports are seen now.
Created attachment 11804 [details] Fix for loadparm memory leak issue.
Created attachment 11813 [details] git-am fix for 4.4.x, 4.3.x, 4.2.x Cherry-pick from master. Applies back to 4.2.x.
Comment on attachment 11813 [details] git-am fix for 4.4.x, 4.3.x, 4.2.x LGTM
Karolin, please add the patches to the relevant branches. Thanks!
Pushed to autobuild-v4-[4|3|2]-test.
(In reply to Karolin Seeger from comment #5) Pushed to all branches. Closing out bug report. Thanks!