#176 Memory leaks in gssproxy 0.7.0
Closed: Fixed Opened by abbra.

Reported as https://pagure.io/freeipa/issue/6775 originally, gssproxy 0.7.0 seems to leak a lot of memory:

gssproxy-debuginfo-0.7.0-2.fc25.x86_64
gssproxy-0.7.0-2.fc25.x86_64

==10030== 
==10030== 112,017,408 bytes in 355 blocks are definitely lost in loss record 748 of 748
==10030==    at 0x4C2FC47: realloc (vg_replace_malloc.c:785)
==10030==    by 0x6E12163: ??? (in /usr/lib64/libkrb5support.so.0.1)
==10030==    by 0x6E122D8: k5_buf_add_len (in /usr/lib64/libkrb5support.so.0.1)
==10030==    by 0x65EDD98: gss_export_cred (in /usr/lib64/libgssapi_krb5.so.2.2)
==10030==    by 0x11930E: gp_export_gssx_cred (gp_export.c:362)
==10030==    by 0x112ED3: gp_export_sync_creds (gp_creds.c:1136)
==10030==    by 0x11C8C5: gp_init_sec_context (gp_rpc_init_sec_context.c:173)
==10030==    by 0x1178A2: gp_rpc_execute (gp_rpc_process.c:321)
==10030==    by 0x1178A2: gp_rpc_process_call (gp_rpc_process.c:378)
==10030==    by 0x110A1A: gp_handle_query (gp_workers.c:424)
==10030==    by 0x110A1A: gp_worker_main (gp_workers.c:378)
==10030==    by 0x682F6C9: start_thread (in /usr/lib64/libpthread-2.24.so)
==10030==    by 0x6B4DF7E: clone (in /usr/lib64/libc-2.24.so)
==10030== 
==10030== LEAK SUMMARY:
==10030==    definitely lost: 235,809,111 bytes in 589,235 blocks
==10030==    indirectly lost: 20,632,139 bytes in 835,807 blocks
==10030==      possibly lost: 6,998,512 bytes in 340 blocks
==10030==    still reachable: 105,255 bytes in 2,933 blocks
==10030==         suppressed: 0 bytes in 0 blocks
==10030== 
==10030== For counts of detected and suppressed errors, rerun with: -v
==10030== ERROR SUMMARY: 44 errors from 44 contexts (suppressed: 0 from 0)

valgrind-debug


Substantial improvement posted in 69a73d85eb3e70fdc7501794d5fd11a73a1d20fa but not totally resolved.

Commit eae00113 relates to this ticket

With https://koji.fedoraproject.org/koji/taskinfo?taskID=20869818 we're down to two leaks in the daemon - one in verto, and one that might be our fault or might be rpc related:

==17730== 27 bytes in 1 blocks are definitely lost in loss record 5 of 25
==17730==    at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==17730==    by 0x6F5AB49: strdup (strdup.c:42)
==17730==    by 0x5BC56CD: do_load_file (verto.c:260)
==17730==    by 0x5BC5267: do_load_dir (verto.c:332)
==17730==    by 0x5BC5C07: load_module (verto.c:426)
==17730==    by 0x5BC6093: verto_default (verto.c:517)
==17730==    by 0x407699: init_event_loop (gp_init.c:110)
==17730==    by 0x418CC7: main (gssproxy.c:241)
==17730==
==17730== 99 bytes in 11 blocks are definitely lost in loss record 13 of 25
==17730==    at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==17730==    by 0x411E4A: gp_memdup (gp_conv.c:15)
==17730==    by 0x411EC4: gp_conv_octet_string (gp_conv.c:33)
==17730==    by 0x4120C0: gp_conv_oid_to_gssx (gp_conv.c:103)
==17730==    by 0x414451: gp_export_ctx_id_to_gssx (gp_export.c:649)
==17730==    by 0x41740B: gp_init_sec_context (gp_rpc_init_sec_context.c:148)
==17730==    by 0x411AE7: gp_rpc_execute (gp_rpc_process.c:321)
==17730==    by 0x411CFA: gp_rpc_process_call (gp_rpc_process.c:378)
==17730==    by 0x4099C5: gp_handle_query (gp_workers.c:430)
==17730==    by 0x409792: gp_worker_main (gp_workers.c:384)
==17730==    by 0x6CAA3A8: start_thread (pthread_create.c:465)
==17730==    by 0x6FDC36E: clone (clone.S:95)

The latter looks definitely our fault from the trace.

@simo: The issue with the latter is that its (the equivalent) of ctx->mech (its actually iscr->context_handle->mech); ctx has free called on it by the RPC code or here, but I'm not experienced enough with the RPC code to know how to free ctx->mech. From what I can tell though, xdr_gssx_ctx seems to handle ctx->mech fine, with that being a xdr_gssx_OID; xdr_gssx_OID is in turn an xdr_octet_string, which is a xdr_bytes. Unless xdr_bytes is not handling free, then I'm not sure why mech is leaking.

libverto issue: https://github.com/npmccallum/libverto/issues/1

@simo, @rharwood -- I found the issue. The above code was for input arguments to init, not for handling responses. In res, note that context_handle is treated as a raw pointer with no sub-memory, here. However, this doesn't fully help the issue as under GSS_S_CONTINUE_NEEDED, mech (set here) gets overwritten here. I'll get a patch with that fixed submitted soon.

Commit 470cf4d7 relates to this ticket

Commit eae00113 relates to this ticket

I think we've fixed all that I'm aware of at this time. (Also, we're on 0.8.0 series.)

Metadata Update from @rharwood:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

Metadata