From a79e780c83ed9d31115d18b642bee4ef889602cd Mon Sep 17 00:00:00 2001

From: Christophe Fergeau <cfergeau@redhat.com>

Date: Fri, 11 Oct 2013 19:56:25 +0200

Subject: [PATCH] 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)

=

Related: rhbz#1097338

(cherry picked from commit 76724d7cb6089b0b91b1cb19ca06f4f6ac145db7)

---

 gtk/spice-channel.c | 21 +++++++++++----------

 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c

index f101c3a..1fa42c0 100644

--- a/gtk/spice-channel.c

+++ b/gtk/spice-channel.c

@@ -1330,7 +1330,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;

@@ -1385,8 +1385,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)",

@@ -1435,8 +1433,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;

     }

@@ -1452,8 +1448,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;

     }

@@ -1639,15 +1633,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 */