Bug 11471 - Memory leak in libsmbclient library
Summary: Memory leak in libsmbclient library
Status: NEEDINFO
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.1.12
Hardware: All All
: P5 major (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-28 12:50 UTC by hargagan
Modified: 2016-01-08 00:33 UTC (History)
4 users (show)

See Also:


Attachments
prable fix for the memory leaks in kerberos credential cache and in dns resolution during spnego authentication using kerberos. (1.28 KB, patch)
2015-08-28 12:50 UTC, hargagan
no flags Details
prable fix for the memory leaks in kerberos credential cache and in dns resolution during spnego authentication using kerberos. (1.49 KB, patch)
2015-08-28 13:32 UTC, hargagan
no flags Details
probable fix for the memory leaks in kerberos credential cache and in dns resolution during spnego authentication using kerberos. (1.43 KB, patch)
2015-09-03 05:07 UTC, hargagan
no flags Details
Proposing patch for supporting multi-threaded smbclient library (3.34 KB, patch)
2015-09-11 08:27 UTC, hargagan
no flags Details
git-am fix for 4.3.next, 4.3.next (1.07 KB, patch)
2015-09-11 20:21 UTC, Jeremy Allison
vl: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description hargagan 2015-08-28 12:50:33 UTC
Created attachment 11375 [details]
prable fix for the memory leaks in kerberos credential cache and in dns resolution during spnego authentication using kerberos.

Noticing these two memory leaks in libsmbclient library :

==9932== 7,672 (3,264 direct, 4,408 indirect) bytes in 102 blocks are definitely lost in loss record 1,274 of 1,365
==9932==    at 0x4C2975E: malloc (vg_replace_malloc.c:296)
==9932==    by 0xF6BE07A: krb5_copy_principal (principal.c:885)
==9932==    by 0xA0D8B0A: kerberos_kinit_password_ext (kerberos.c:235)
==9932==    by 0xA0D8DF1: kerberos_kinit_password (kerberos.c:580)
==9932==    by 0x94F4D92: cli_session_setup_spnego_send (cliconnect.c:1839)
==9932==    by 0x94F6EF0: cli_session_setup_send (cliconnect.c:2097)
==9932==    by 0x94F70DC: cli_full_connection_started (cliconnect.c:3320)
==9932==    by 0xAB8813B: smbXcli_negprot_smb1_done (smbXcli_base.c:4296)
==9932==    by 0xAB8308D: smb1cli_conn_dispatch_incoming (smbXcli_base.c:2068)
==9932==    by 0xAB87756: smbXcli_conn_received (smbXcli_base.c:1628)
==9932==    by 0xD7C4A36: read_smb_done (read_smb.c:98)
==9932==    by 0xB5DD9F4: epoll_event_loop_once (in /usr/lib64/libtevent.so.0.9.21)

==9932== 42,196 bytes in 1,507 blocks are definitely lost in loss record 1,347 of 1,365
==9932==    at 0x4C2975E: malloc (vg_replace_malloc.c:296)
==9932==    by 0x11AC55C1: __libc_res_nsend (in /lib64/libresolv-2.11.3.so)
==9932==    by 0x11AC2B48: __libc_res_nquery (in /lib64/libresolv-2.11.3.so)
==9932==    by 0x11AC3179: __libc_res_nquerydomain (in /lib64/libresolv-2.11.3.so)
==9932==    by 0x11AC3384: __libc_res_nsearch (in /lib64/libresolv-2.11.3.so)
==9932==    by 0x11AC38F4: __res_nsearch (in /lib64/libresolv-2.11.3.so)
==9932==    by 0x12951432: dns_lookup_int (resolve.c:565)
==9932==    by 0xF6B45EB: srv_find_realm (krbhst.c:88)
==9932==    by 0xF6B544F: srv_get_hosts (krbhst.c:441)
==9932==    by 0xF6B5E70: kdc_get_next (krbhst.c:667)
==9932==    by 0xF6C60A1: krb5_sendto (send_to_kdc.c:381)
==9932==    by 0xF6C673E: krb5_sendto_context (send_to_kdc.c:625)
Comment 1 hargagan 2015-08-28 13:32:24 UTC
Created attachment 11377 [details]
prable fix for the memory leaks in kerberos credential cache and in dns resolution during spnego authentication using kerberos.
Comment 2 Jeremy Allison 2015-09-02 16:56:05 UTC
The credential cache fix looks wrong to me. Doing a krb5_cc_initialize (which is what calls this) shouldn't ever have a non-NULL m->primary_principal field.

Looks like a missing krb5_cc_destroy instead to me.

Jeremy.
Comment 3 Jeremy Allison 2015-09-02 17:44:47 UTC
The resolve one looks correct !
Comment 4 hargagan 2015-09-03 05:07:10 UTC
Created attachment 11397 [details]
probable fix for the memory leaks in kerberos credential cache and in dns resolution during spnego authentication using kerberos.

Seeing issues after putting krb5_cc_destroy(). After putting the new fix of checking the m->primary_principal and then freeing before doing krb5_copy_principal again. Hence clearing the memory. The resolve dns fix remains same.
Comment 5 hargagan 2015-09-03 05:28:37 UTC
Thanks for looking into the proposed patch. Please review the new version.
Comment 6 Jeremy Allison 2015-09-03 19:25:45 UTC
(In reply to hargagan from comment #4)

What issues where you seeing with adding the krb5_cc_destroy() ?

The core of this issue is that mcc_initialize() which is called by krb5_cc_initialize() should destroy all contents of the krb5_mcache * pointer.

From the man page:

krb5_cc_initialize - Initialize a credential cache.

Destroy any existing contents of cache and initialize it for the default principal principal.

Right now it doesn't change creds or any other value, just overwrites default principal.

What I'd like you to do is add in this version of the patch:

+    if (m->primary_principal != NULL) {
+           abort()
+    }

The run under gdb and get me a backtrace of when it hits the abort. I need to see what code path this is being used under.
Comment 7 hargagan 2015-09-09 08:30:39 UTC
(In reply to Jeremy Allison from comment #6)

Apologies for the delay. Here is the backtrace after putting the abort() in the code path :

#0  0x00007ffff64a6885 in raise () from /lib64/libc.so.6
#1  0x00007ffff64a7e61 in abort () from /lib64/libc.so.6
#2  0x00007fffed7694fc in mcc_initialize (context=0x7fffd0654ae0, id=<optimized out>, primary_principal=0x7fffd064e7b0) at ../source4/heimdal/lib/krb5/mcache.c:168
#3  0x00007ffff2d26b6b in kerberos_kinit_password_ext (principal=0x7fffd0651a50 "4a15-vm2@win2012.performance.com", password=0x7fffd0650920 "novell", time_offset=0, expire_time=0x0, 
    renew_till_time=0x0, cache_name=0x7fffd0652f80 "MEMORY:cliconnect", request_pac=false, add_netbios_addr=false, renewable_time=0, ntstatus=0x0) at ../source3/libads/kerberos.c:235
#4  0x00007ffff2d26e52 in kerberos_kinit_password (principal=0x4882 <error: Cannot access memory at address 0x4882>, password=0x4968 <error: Cannot access memory at address 0x4968>, 
    time_offset=6, cache_name=<optimized out>) at ../source3/libads/kerberos.c:580
#5  0x00007ffff38dedf3 in cli_session_setup_spnego_send (mem_ctx=<optimized out>, ev=0x7fffd0653ab0, cli=0x7fffd0653900, user=0x7fffd0651a50 "4a15-vm2@win2012.performance.com", 
    pass=0x7fffd0650920 "password123", user_domain=0x7fffd0650990 "WIN2012PERF", dest_realm=0x0) at ../source3/libsmb/cliconnect.c:1839
#6  0x00007ffff38e0f51 in cli_session_setup_send (mem_ctx=<optimized out>, ev=0x7fffd0653ab0, cli=0x7fffd0653900, user=0x7fffd0651a50 "4a15-vm2@win2012.performance.com", 
    pass=0x7fffd0650920 "password123", passlen=11, ntpass=0x7fffd0650920 "password123", ntpasslen=11, workgroup=0x7fffd0650990 "WIN2012PERF") at ../source3/libsmb/cliconnect.c:2097
#7  0x00007ffff38e143b in cli_session_setup (cli=0x7fffd0653900, user=0x7fffd0651a50 "4a15-vm2@win2012.performance.com", pass=0x7fffd0650920 "password123", passlen=11, 
    ntpass=0x7fffd0650920 "password123", ntpasslen=11, workgroup=0x7fffd0650990 "WIN2012PERF") at ../source3/libsmb/cliconnect.c:2223
#8  0x00007ffff4394f0d in SMBC_server_internal (ctx=0x7fffd0650a30, context=0x7fffd0650c40, connect_if_not_found=true, server=0x7fffd0652380 "srv2015", port=0, share=0x7fffd06523f0 "DATA", 
    pp_workgroup=0x7fff74f489c8, pp_username=0x7fff74f489d8, pp_password=0x7fff74f489d0, in_cache=0x7fff74f48917) at ../source3/libsmb/libsmb_server.c:501
#9  0x00007ffff43957f6 in SMBC_server (ctx=0x4882, context=0x4968, connect_if_not_found=6, server=0xffffffffffffffff <error: Cannot access memory at address 0xffffffffffffffff>, 
    port=<optimized out>, share=0x0, pp_workgroup=0x7fff74f489c8, pp_username=0x7fff74f489d8, pp_password=0x7fff74f489d0) at ../source3/libsmb/libsmb_server.c:697
#10 0x00007ffff4392579 in SMBC_open_ctx (context=0x7fffd0650c40, 
    fname=0x7fffd0650db0 "smb://srv2015/DATA/adusers/4a15-vm2/Test%201-ced294e68d4942558db52f9001243df9.xls", flags=514, 
    mode=<optimized out>) at ../source3/libsmb/libsmb_file.c:94
#11 0x00007ffff45b3073 in FilePut() () from /tmp/test/lib64/libcifsmod.so
#12 0x000000000040c813 in thread_handler ()
#13 0x00007ffff625e806 in start_thread () from /lib64/libpthread.so.0
#14 0x00007ffff655265d in clone () from /lib64/libc.so.6
#15 0x0000000000000000 in ?? ()
Comment 8 hargagan 2015-09-10 05:30:51 UTC
When I use 'krb5_cc_destroy()' in place of 'krb5_cc_close()', the working session setup operation starts failing with these errors :


**************************************************************************

[2015/09/09 23:22:48.583810,  3, pid=4164, effective(0, 0), real(0, 0)] ../source3/libsmb/cliconnect.c:1687(cli_session_setup_get_principal)
  cli_session_setup_spnego: using target hostname not SPNEGO principal
[2015/09/09 23:22:48.583945,  5, pid=4164, effective(0, 0), real(0, 0)] ../lib/krb5_wrap/krb5_samba.c:2199(smb_krb5_get_default_realm_from_ccache)
  kerberos_get_default_realm_from_ccache: Trying to read krb5 cache: MEMORY:cliconnect
[2015/09/09 23:22:48.583996,  0, pid=4164, effective(0, 0), real(0, 0)] ../lib/krb5_wrap/krb5_samba.c:2207(smb_krb5_get_default_realm_from_ccache)
  kerberos_get_default_realm_from_ccache: failed to get default principal
[2015/09/09 23:22:48.584044,  3, pid=4164, effective(0, 0), real(0, 0)] ../lib/krb5_wrap/krb5_samba.c:2312(kerberos_get_principal_from_service_hostname)
  kerberos_get_principal_from_service_hostname: cannot get realm from, desthost oes2015 or default ccache. Using default smb.conf realm
[2015/09/09 23:22:48.584079,  3, pid=4164, effective(0, 0), real(0, 0)] ../source3/libsmb/cliconnect.c:1702(cli_session_setup_get_principal)
  cli_session_setup_spnego: guessed server principal=cifs/srv2015@
[2015/09/09 23:22:48.584109,  2, pid=4164, effective(0, 0), real(0, 0)] ../source3/libsmb/cliconnect.c:1312(cli_session_setup_kerberos_send)
  Doing kerberos session setup
[2015/09/09 23:22:48.584193,  3, pid=4164, effective(0, 0), real(0, 0)] ../lib/krb5_wrap/krb5_samba.c:499(ads_krb5_mk_req)
  ads_krb5_mk_req: krb5_cc_get_principal failed (No such file or directory)
[2015/09/09 23:22:48.584238,  1, pid=4164, effective(0, 0), real(0, 0)] ../source3/libsmb/cliconnect.c:1331(cli_session_setup_kerberos_send)
  cli_session_setup_kerberos: spnego_gen_krb5_negTokenInit failed: No such file or directory

*****************************************************************************

Please let me know if you need more information.
Comment 9 Jeremy Allison 2015-09-10 21:23:38 UTC
Hang on a minute. I need to see the program you're using to drive libsmbclient.

The only way the code path you've shown could happen, is if krb5_cc_resolve had been called *twice*, which would up the refcount inside the MEMORY:cliconnect memory cc_cache.

Are you calling this twice from a threaded program .. ?

The stack trace:

#11 0x00007ffff45b3073 in FilePut() () from /tmp/test/lib64/libcifsmod.so
#12 0x000000000040c813 in thread_handler ()
#13 0x00007ffff625e806 in start_thread () from /lib64/libpthread.so.0
#14 0x00007ffff655265d in clone () from /lib64/libc.so.6
#15 0x0000000000000000 in ?? ()

is very suspicious. You know libsmbclient isn't thread-safe right ? (See bug:

https://bugzilla.samba.org/show_bug.cgi?id=11413

for details).
Comment 10 hargagan 2015-09-11 08:27:52 UTC
Created attachment 11430 [details]
Proposing patch for supporting multi-threaded smbclient library

Hi Jeremy, 

Sorry, but I never got to know of the bug https://bugzilla.samba.org/show_bug.cgi?id=11413, and also didn't get a chance to raise a bug for this issue. So I couldn't submit the changes for multi-thread support. Attaching the patch which has the changes that I did to get around the multi-threading issue with smbclient library. Please let me know if you want me to update this patch on the original bug for the multi-threading issue (11413).

Thanks.
Comment 11 Jeremy Allison 2015-09-11 15:46:13 UTC
(In reply to hargagan from comment #10)

So I'm guessing the answer to my question is "yes, I am trying to use it in a multi-threaded program" :-).

Well that explains the problem in the krb5 code. Currently libsmbclient isn't MT-safe. At *all*. To see some of the problems, run under valgrind --tool=drd or valgrind --tool=helgrind.

The patch you provided is helpful, but doesn't go near fixing the multiple issues we have here.

So I'm going to close out this bug once I've gotten the dns resolution bugfix pushed to 4.3.x, 4.2.x and then let's work together on the other bug to fix up libsmbclient to be MT-safe !

Jeremy.
Comment 12 Jeremy Allison 2015-09-11 20:21:21 UTC
Created attachment 11432 [details]
git-am fix for 4.3.next, 4.3.next

Cherry-picked from master.
Comment 13 Andrew Bartlett 2016-01-08 00:33:41 UTC
The matching upstream Heimdal fix is d931fd0a22799adc5a27c4f6ec7168ea68a0f560 and was added in 2011.  Sadly we never updated to that version.