pxcloud/guacamole-server
5
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

GUACAMOLE-1732: Prevent deadlock and enhance cleanup in RDP client.

Browse Source
This commit is contained in:
Alex Leitner 2024-03-12 23:35:13 +00:00
parent 26cf71f2ee
commit 19df34b7b2
No known key found for this signature in database
GPG Key ID: C8AF47D3CD30770E
5 changed files with 48 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/protocols/rdp/client.c
View File

@ -35,10 +35,12 @@
#include "common-ssh/user.h"
#endif
#include <guacamole/argv.h>
#include <guacamole/audio.h>
#include <guacamole/client.h>
#include <guacamole/mem.h>
#include <guacamole/recording.h>
#include <guacamole/rwlock.h>
#include <dirent.h>
#include <errno.h>
@ -117,7 +119,7 @@ static int guac_rdp_join_pending_handler(guac_client* client) {
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
guac_socket* broadcast_socket = client->pending_socket;
pthread_rwlock_rdlock(&(rdp_client->lock));
guac_rwlock_acquire_read_lock(&(rdp_client->lock));
/* Synchronize any audio stream for each pending user */
if (rdp_client->audio)
@ -133,7 +135,7 @@ static int guac_rdp_join_pending_handler(guac_client* client) {
guac_socket_flush(broadcast_socket);
}
pthread_rwlock_unlock(&(rdp_client->lock));
guac_rwlock_release_lock(&(rdp_client->lock));
return 0;
@ -220,7 +222,7 @@ int guac_client_init(guac_client* client, int argc, char** argv) {
PTHREAD_MUTEX_RECURSIVE);
/* Init required locks */
pthread_rwlock_init(&(rdp_client->lock), NULL);
guac_rwlock_init(&(rdp_client->lock));
pthread_mutex_init(&(rdp_client->message_lock), &(rdp_client->attributes));
/* Set handlers */
@ -241,6 +243,14 @@ int guac_rdp_client_free_handler(guac_client* client) {
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
/*
* Signals any threads that are blocked awaiting user input for authentication
* (e.g., username or password) to terminate their wait. By broadcasting a
* condition signal, the authentication process is interrupted, allowing for
* premature termination and cleanup during client disconnection.
*/
guac_argv_stop();
/* Wait for client thread */
pthread_join(rdp_client->client_thread, NULL);
@ -297,7 +307,7 @@ 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));
guac_rwlock_destroy(&(rdp_client->lock));
pthread_mutex_destroy(&(rdp_client->message_lock));
/* Free client data */
@ -306,4 +316,3 @@ int guac_rdp_client_free_handler(guac_client* client) {
return 0;
}

14
src/protocols/rdp/input.c
View File

@ -30,6 +30,7 @@
#include <freerdp/input.h>
#include <guacamole/client.h>
#include <guacamole/recording.h>
#include <guacamole/rwlock.h>
#include <guacamole/user.h>
#include <stdlib.h>
@ -39,7 +40,7 @@ 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));
guac_rwlock_acquire_read_lock(&(rdp_client->lock));
/* Skip if not yet connected */
freerdp* rdp_inst = rdp_client->rdp_inst;
@ -125,7 +126,7 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) {
}
complete:
pthread_rwlock_unlock(&(rdp_client->lock));
guac_rwlock_release_lock(&(rdp_client->lock));
return 0;
}
@ -136,7 +137,7 @@ int guac_rdp_user_touch_handler(guac_user* user, int id, int x, int y,
guac_client* client = user->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
pthread_rwlock_rdlock(&(rdp_client->lock));
guac_rwlock_acquire_read_lock(&(rdp_client->lock));
/* Skip if not yet connected */
freerdp* rdp_inst = rdp_client->rdp_inst;
@ -152,7 +153,7 @@ int guac_rdp_user_touch_handler(guac_user* user, int id, int x, int y,
guac_rdp_rdpei_touch_update(rdp_client->rdpei, id, x, y, force);
complete:
pthread_rwlock_unlock(&(rdp_client->lock));
guac_rwlock_release_lock(&(rdp_client->lock));
return 0;
}
@ -163,7 +164,7 @@ int guac_rdp_user_key_handler(guac_user* user, int keysym, int pressed) {
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
int retval = 0;
pthread_rwlock_rdlock(&(rdp_client->lock));
guac_rwlock_acquire_read_lock(&(rdp_client->lock));
/* Report key state within recording */
if (rdp_client->recording != NULL)
@ -179,7 +180,7 @@ int guac_rdp_user_key_handler(guac_user* user, int keysym, int pressed) {
keysym, pressed, GUAC_RDP_KEY_SOURCE_CLIENT);
complete:
pthread_rwlock_unlock(&(rdp_client->lock));
guac_rwlock_release_lock(&(rdp_client->lock));
return retval;
@ -202,4 +203,3 @@ int guac_rdp_user_size_handler(guac_user* user, int width, int height) {
return 0;
}

5
src/protocols/rdp/keyboard.c
View File

@ -26,6 +26,7 @@
#include <freerdp/input.h>
#include <guacamole/client.h>
#include <guacamole/mem.h>
#include <guacamole/rwlock.h>
#include <stdlib.h>
@ -718,7 +719,7 @@ BOOL guac_rdp_keyboard_set_indicators(rdpContext* context, UINT16 flags) {
guac_client* client = ((rdp_freerdp_context*) context)->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
pthread_rwlock_rdlock(&(rdp_client->lock));
guac_rwlock_acquire_read_lock(&(rdp_client->lock));
/* Skip if keyboard not yet ready */
guac_rdp_keyboard* keyboard = rdp_client->keyboard;
@ -730,7 +731,7 @@ BOOL guac_rdp_keyboard_set_indicators(rdpContext* context, UINT16 flags) {
keyboard->lock_flags = flags;
complete:
pthread_rwlock_unlock(&(rdp_client->lock));
guac_rwlock_release_lock(&(rdp_client->lock));
return TRUE;
}

27
src/protocols/rdp/rdp.c
View File

@ -108,8 +108,14 @@ BOOL rdp_freerdp_pre_connect(freerdp* instance) {
/* Load "AUDIO_INPUT" plugin for audio input*/
if (settings->enable_audio_input) {
/* Upgrade the lock to write temporarily for setting the newly allocated audio buffer */
guac_rwlock_acquire_write_lock(&(rdp_client->lock));
rdp_client->audio_input = guac_rdp_audio_buffer_alloc(client);
guac_rdp_audio_load_plugin(instance->context);
/* Downgrade the lock to allow for concurrent read access */
guac_rwlock_release_lock(&(rdp_client->lock));
guac_rwlock_acquire_read_lock(&(rdp_client->lock));
}
/* Load "cliprdr" service if not disabled */
@ -470,7 +476,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
/* Init random number generator */
srandom(time(NULL));
pthread_rwlock_wrlock(&(rdp_client->lock));
guac_rwlock_acquire_write_lock(&(rdp_client->lock));
/* Create display */
rdp_client->display = guac_common_display_alloc(client,
@ -516,12 +522,23 @@ static int guac_rdp_handle_connection(guac_client* client) {
/* Set default pointer */
guac_common_cursor_set_pointer(rdp_client->display->cursor);
/*
* Downgrade the lock to allow for concurrent read access.
* Access to read locks needs to be made available for other processes such
* as the join_pending_handler to use while we await credentials from the user.
*/
guac_rwlock_release_lock(&(rdp_client->lock));
guac_rwlock_acquire_read_lock(&(rdp_client->lock));
/* Connect to RDP server */
if (!freerdp_connect(rdp_inst)) {
guac_rdp_client_abort(client, rdp_inst);
goto fail;
}
/* Upgrade to write lock again for further exclusive operations */
guac_rwlock_acquire_write_lock(&(rdp_client->lock));
/* Connection complete */
rdp_client->rdp_inst = rdp_inst;
@ -530,7 +547,7 @@ 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));
guac_rwlock_release_lock(&(rdp_client->lock));
/* Handle messages from RDP server while client is running */
while (client->state == GUAC_CLIENT_RUNNING
@ -617,7 +634,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
}
pthread_rwlock_wrlock(&(rdp_client->lock));
guac_rwlock_acquire_write_lock(&(rdp_client->lock));
/* Clean up print job, if active */
if (rdp_client->active_job != NULL) {
@ -652,7 +669,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
guac_common_display_free(rdp_client->display);
rdp_client->display = NULL;
pthread_rwlock_unlock(&(rdp_client->lock));
guac_rwlock_release_lock(&(rdp_client->lock));
/* Client is now disconnected */
guac_client_log(client, GUAC_LOG_INFO, "Internal RDP client disconnected");
@ -660,7 +677,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
return 0;
fail:
pthread_rwlock_unlock(&(rdp_client->lock));
guac_rwlock_release_lock(&(rdp_client->lock));
return 1;
}

4
src/protocols/rdp/rdp.h
View File

@ -44,6 +44,7 @@
#include <freerdp/freerdp.h>
#include <guacamole/audio.h>
#include <guacamole/client.h>
#include <guacamole/rwlock.h>
#include <guacamole/recording.h>
#include <winpr/wtypes.h>
@ -170,7 +171,7 @@ typedef struct guac_rdp_client {
* from running when RDP data structures are allocated or freed
* by the client thread.
*/
pthread_rwlock_t lock;
guac_rwlock lock;
/**
* Lock which synchronizes the sending of each RDP message, ensuring
@ -219,4 +220,3 @@ typedef struct rdp_freerdp_context {
void* guac_rdp_client_thread(void* data);
#endif