Possible memory leak in loadparm.

[Hemanth Thummala]

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.

Here I have attached the patch for the same. I am not sure how much extent this memory leak/buildup can grow. I feel using non-null context to allocate individual serviceptr records is right thing to do.

Please review this patch and let me know if this looks good.

Thanks,
Hemanth.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Use-non-NULL-talloc-context-to-resolve-memory-buildu.patch
Type: application/octet-stream
Size: 963 bytes
Desc: 0001-Use-non-NULL-talloc-context-to-resolve-memory-buildu.patch
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160202/a5ae1d26/0001-Use-non-NULL-talloc-context-to-resolve-memory-buildu.obj>