From d734bac59006074f6d50e819c46e9392ccc00681 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 17 Mar 2022 17:32:37 +0000 Subject: [PATCH 1/2] GUACAMOLE-1115: Do not hold general RDP message lock while waiting for print operations. Holding the message lock will block handling of things like mouse and keyboard events, as the message lock must be acquired before sending the corresponding messages to the RDP server. If mouse and keyboard events are blocked, then handling of further Guacamole instructions like "ack" is also blocked. If the print job is blocked until an "ack" is received, this results in deadlock. --- src/protocols/rdp/print-job.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/protocols/rdp/print-job.c b/src/protocols/rdp/print-job.c index 7d711c00..81612efc 100644 --- a/src/protocols/rdp/print-job.c +++ b/src/protocols/rdp/print-job.c @@ -18,6 +18,7 @@ */ #include "print-job.h" +#include "rdp.h" #include #include @@ -603,6 +604,9 @@ static void guac_rdp_print_job_read_filename(guac_rdp_print_job* job, int guac_rdp_print_job_write(guac_rdp_print_job* job, void* buffer, int length) { + guac_client* client = job->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + /* Create print job, if not yet created */ if (job->bytes_received == 0) { @@ -618,19 +622,40 @@ int guac_rdp_print_job_write(guac_rdp_print_job* job, /* Update counter of bytes received */ job->bytes_received += length; - /* Write data to filter process */ - return write(job->input_fd, buffer, length); + /* Write data to filter process, unblocking any threads waiting on the + * generic RDP message lock as this may be a lengthy operation that depends + * on other threads sending outstanding messages (resulting in deadlock if + * those messages are blocked) */ + int unlock_status = pthread_mutex_unlock(&(rdp_client->message_lock)); + int write_status = write(job->input_fd, buffer, length); + + /* Restore RDP message lock state */ + if (!unlock_status) + pthread_mutex_lock(&(rdp_client->message_lock)); + + return write_status; } void guac_rdp_print_job_free(guac_rdp_print_job* job) { + guac_client* client = job->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + /* No more input will be provided */ close(job->input_fd); - /* Wait for job to terminate */ + /* Wait for job to terminate, unblocking any threads waiting on the generic + * RDP message lock as this may be a lengthy operation that depends on + * other threads sending outstanding messages (resulting in deadlock if + * those messages are blocked) */ + int unlock_status = pthread_mutex_unlock(&(rdp_client->message_lock)); pthread_join(job->output_thread, NULL); + /* Restore RDP message lock state */ + if (!unlock_status) + pthread_mutex_lock(&(rdp_client->message_lock)); + /* Destroy lock */ pthread_mutex_destroy(&(job->state_lock)); From ce88fa4d4a45f2d640148231e06413eb8223fd4d Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 17 Mar 2022 18:27:05 +0000 Subject: [PATCH 2/2] GUACAMOLE-1115: Forcibly kill any outstanding PDF filter job when cleaning up resources. --- src/protocols/rdp/client.c | 8 ++++++++ src/protocols/rdp/print-job.c | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/src/protocols/rdp/client.c b/src/protocols/rdp/client.c index 5dbb6d6d..aed5ea68 100644 --- a/src/protocols/rdp/client.c +++ b/src/protocols/rdp/client.c @@ -199,6 +199,14 @@ int guac_rdp_client_free_handler(guac_client* client) { if (rdp_client->filesystem != NULL) guac_rdp_fs_free(rdp_client->filesystem); + /* End active print job, if any */ + guac_rdp_print_job* job = (guac_rdp_print_job*) rdp_client->active_job; + if (job != NULL) { + guac_rdp_print_job_kill(job); + guac_rdp_print_job_free(job); + rdp_client->active_job = NULL; + } + #ifdef ENABLE_COMMON_SSH /* Free SFTP filesystem, if loaded */ if (rdp_client->sftp_filesystem) diff --git a/src/protocols/rdp/print-job.c b/src/protocols/rdp/print-job.c index 81612efc..7564fc88 100644 --- a/src/protocols/rdp/print-job.c +++ b/src/protocols/rdp/print-job.c @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -666,6 +667,9 @@ void guac_rdp_print_job_free(guac_rdp_print_job* job) { void guac_rdp_print_job_kill(guac_rdp_print_job* job) { + /* Forcibly kill filter process, if running */ + kill(job->filter_pid, SIGKILL); + /* Stop all handling of I/O */ close(job->input_fd); close(job->output_fd);