diff options
author | Christophe Fergeau <cfergeau@redhat.com> | 2013-10-11 19:56:25 +0200 |
---|---|---|
committer | Christophe Fergeau <cfergeau@redhat.com> | 2014-03-13 17:17:15 +0100 |
commit | 76724d7cb6089b0b91b1cb19ca06f4f6ac145db7 (patch) | |
tree | 718057fafb273d075fef07435b99c244e15def42 /gtk | |
parent | b7c9343ba20abd93429a37b475920cde97bf01e8 (diff) |
sasl: Rework memory handling in spice_channel_perform_auth_sasl()
While looking at the SASL code, I noticed some memory leaks in error paths.
This commit adds a cleanup: block to free some of the memory dynamically
allocated in that function, and remove the corresponding g_free() from
the regular code flow. This should ensure that both the regular path
and the error paths free the same memory.
This fixes at least this 'mechlist' leak which I got during regular SASL
PLAIN authentication:
==3452== 6 bytes in 1 blocks are definitely lost in loss record 140 of 11,706
==3452== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.s
==3452== by 0x35BAC4EE6E: g_malloc (gmem.c:104)
==3452== by 0x5BF7CAA: spice_channel_perform_auth_sasl (spice-channel.c:1440)
==3452== by 0x5BF9033: spice_channel_recv_link_msg (spice-channel.c:1727)
==3452== by 0x5BFAECD: spice_channel_coroutine (spice-channel.c:2348)
==3452== by 0x5C35D6D: coroutine_trampoline (coroutine_ucontext.c:63)
==3452== by 0x5C35A1B: continuation_trampoline (continuation.c:51)
==3452== by 0x31342479BF: ??? (in /usr/lib64/libc-2.18.so)
==3452== by 0x75F2940591224CFF: ???
==3452== by 0xE756E5F: ???
==3452== by 0xE7589BF: ???
==3452== by 0xFFEFFF78F: ???
==3452== by 0x5BFCD92: g_io_wait_helper (gio-coroutine.c:43)
=
Diffstat (limited to 'gtk')
-rw-r--r-- | gtk/spice-channel.c | 21 |
1 files changed, 11 insertions, 10 deletions
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c index ace8b94..83c7006 100644 --- a/gtk/spice-channel.c +++ b/gtk/spice-channel.c @@ -1348,7 +1348,7 @@ static gboolean spice_channel_perform_auth_sasl(SpiceChannel *channel) }; sasl_interact_t *interact = NULL; guint32 len; - char *mechlist; + char *mechlist = NULL; const char *mechname; gboolean ret = FALSE; GSocketAddress *addr = NULL; @@ -1403,8 +1403,6 @@ static gboolean spice_channel_perform_auth_sasl(SpiceChannel *channel) saslcb, SASL_SUCCESS_DATA, &saslconn); - g_free(localAddr); - g_free(remoteAddr); if (err != SASL_OK) { g_critical("Failed to create SASL client context: %d (%s)", @@ -1453,8 +1451,6 @@ static gboolean spice_channel_perform_auth_sasl(SpiceChannel *channel) spice_channel_read(channel, mechlist, len); mechlist[len] = '\0'; if (c->has_error) { - g_free(mechlist); - mechlist = NULL; goto error; } @@ -1470,8 +1466,6 @@ restart: if (err != SASL_OK && err != SASL_CONTINUE && err != SASL_INTERACT) { g_critical("Failed to start SASL negotiation: %d (%s)", err, sasl_errdetail(saslconn)); - g_free(mechlist); - mechlist = NULL; goto error; } @@ -1657,15 +1651,22 @@ complete: * is defined to be sent unencrypted, and setting saslconn turns * on the SSF layer encryption processing */ c->sasl_conn = saslconn; - return ret; + goto cleanup; error: - g_clear_object(&addr); if (saslconn) sasl_dispose(&saslconn); emit_main_context(channel, SPICE_CHANNEL_EVENT, SPICE_CHANNEL_ERROR_AUTH); c->has_error = TRUE; /* force disconnect */ - return FALSE; + ret = FALSE; + +cleanup: + g_free(localAddr); + g_free(remoteAddr); + g_free(mechlist); + g_free(serverin); + g_clear_object(&addr); + return ret; } #endif /* HAVE_SASL */ |