GUACAMOLE-1053: guacd segfaults when user actively presses keys at RDP disconnect time

src/protocols/rdp/client.c

@@ -155,6 +155,9 @@ int guac_client_init(guac_client* client, int argc, char** argv) {
     pthread_mutexattr_settype(&(rdp_client->attributes),
             PTHREAD_MUTEX_RECURSIVE);
 
+    /* Initalize the lock */
+    pthread_rwlock_init(&(rdp_client->lock), NULL);
+
     /* Set handlers */
     client->join_handler = guac_rdp_user_join_handler;
     client->free_handler = guac_rdp_client_free_handler;
@@ -216,6 +219,8 @@ int guac_rdp_client_free_handler(guac_client* client) {
     if (rdp_client->audio_input != NULL)
         guac_rdp_audio_buffer_free(rdp_client->audio_input);
 
+    pthread_rwlock_destroy(&(rdp_client->lock));
+
     /* Free client data */
     free(rdp_client);
 

src/protocols/rdp/input.c

@@ -38,10 +38,12 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) {
     guac_client* client = user->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
+    pthread_rwlock_rdlock(&(rdp_client->lock));
+
     /* Skip if not yet connected */
     freerdp* rdp_inst = rdp_client->rdp_inst;
     if (rdp_inst == NULL)
-        return 0;
+        goto complete;
 
     /* Store current mouse location/state */
     guac_common_cursor_update(rdp_client->display->cursor, user, x, y, mask);
@@ -114,6 +116,9 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) {
         rdp_client->mouse_button_mask = mask;
     }
 
+complete:
+    pthread_rwlock_unlock(&(rdp_client->lock));
+
     return 0;
 }
 
@@ -121,6 +126,9 @@ int guac_rdp_user_key_handler(guac_user* user, int keysym, int pressed) {
 
     guac_client* client = user->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+    int retval = 0;
+
+    pthread_rwlock_rdlock(&(rdp_client->lock));
 
     /* Report key state within recording */
     if (rdp_client->recording != NULL)
@@ -129,11 +137,16 @@ int guac_rdp_user_key_handler(guac_user* user, int keysym, int pressed) {
 
     /* Skip if keyboard not yet ready */
     if (rdp_client->keyboard == NULL)
-        return 0;
+        goto complete;
 
     /* Update keysym state */
-    return guac_rdp_keyboard_update_keysym(rdp_client->keyboard,
-            keysym, pressed);
+    retval = guac_rdp_keyboard_update_keysym(rdp_client->keyboard,
+                keysym, pressed);
+
+complete:
+    pthread_rwlock_unlock(&(rdp_client->lock));
+
+    return retval;
 
 }
 

src/protocols/rdp/rdp.c

@@ -345,7 +345,9 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
     /* Init random number generator */
     srandom(time(NULL));
-    
+
+    pthread_rwlock_wrlock(&(rdp_client->lock));
+
     /* Set up screen recording, if requested */
     if (settings->recording_path != NULL) {
         rdp_client->recording = guac_common_recording_create(client,
@@ -380,7 +382,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
                 "FreeRDP initialization failed before connecting. Please "
                 "check for errors earlier in the logs and/or enable "
                 "debug-level logging for guacd.");
-        return 1;
+        goto fail;
     }
 
     ((rdp_freerdp_context*) rdp_inst->context)->client = client;
@@ -396,7 +398,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
     if (!freerdp_connect(rdp_inst)) {
         guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_NOT_FOUND,
                 "Error connecting to RDP server");
-        return 1;
+        goto fail;
     }
 
     /* Connection complete */
@@ -407,6 +409,8 @@ static int guac_rdp_handle_connection(guac_client* client) {
     /* Signal that reconnect has been completed */
     guac_rdp_disp_reconnect_complete(rdp_client->disp);
 
+    pthread_rwlock_unlock(&(rdp_client->lock));
+
     /* Handle messages from RDP server while client is running */
     while (client->state == GUAC_CLIENT_RUNNING
             && !guac_rdp_disp_reconnect_needed(rdp_client->disp)) {
@@ -489,6 +493,8 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
     }
 
+    pthread_rwlock_wrlock(&(rdp_client->lock));
+
     /* Clean up print job, if active */
     if (rdp_client->active_job != NULL) {
         guac_rdp_print_job_kill(rdp_client->active_job);
@@ -510,18 +516,27 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
     /* Free SVC list */
     guac_common_list_free(rdp_client->available_svc);
+    rdp_client->available_svc = NULL;
 
     /* Free RDP keyboard state */
     guac_rdp_keyboard_free(rdp_client->keyboard);
+    rdp_client->keyboard = NULL;
 
     /* Free display */
     guac_common_display_free(rdp_client->display);
+    rdp_client->display = NULL;
+
+    pthread_rwlock_unlock(&(rdp_client->lock));
 
     /* Client is now disconnected */
     guac_client_log(client, GUAC_LOG_INFO, "Internal RDP client disconnected");
 
     return 0;
 
+fail:
+    pthread_rwlock_unlock(&(rdp_client->lock));
+    return 1;
+
 }
 
 void* guac_rdp_client_thread(void* data) {

src/protocols/rdp/rdp.h

@@ -158,6 +158,14 @@ typedef struct guac_rdp_client {
      */
     pthread_mutexattr_t attributes;
 
+    /**
+     * Lock which is used to synchronizes access to RDP data structures
+     * between user input and client threads. It prevents input handlers
+     * from running when RDP data structures are allocated or freed
+     * by the client thread.
+     */
+    pthread_rwlock_t lock;
+
 } guac_rdp_client;
 
 /**