From 2a4dd06315cf3428719634af96bc8f8bac268034 Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: Tue, 30 Mar 2021 09:46:04 -0400 Subject: [PATCH] import spice-vdagent-0.20.0-3.el8 --- ...l-for-agent_owns_clipboard-and-clien.patch | 107 + ...ntd-Automatically-release-agent_data.patch | 72 + ...on-Pass-err-to-g_credentials_get_uni.patch | 34 + ...check-for-vdagent_connection_get_pee.patch | 44 + .../0009-vdagentd-Avoid-calling-chmod.patch | 49 + ...-file-transfer-IDs-allocation-and-us.patch | 84 + ...ncontrolled-active_xfers-allocations.patch | 61 + ...2-Avoids-unlimited-agent-connections.patch | 50 + .../0013-Avoids-user-session-hijacking.patch | 144 ++ SOURCES/0014-Better-check-for-sessions.patch | 164 ++ ...it-number-of-agents-per-session-to-1.patch | 57 + ...ve_xfers-when-the-client-disconnects.patch | 27 + ...allow-to-use-an-already-used-file-xf.patch | 33 + .../0018-Add-a-test-for-session_info.patch | 130 + ...9-wayland-fix-monitor-mapping-issues.patch | 2292 +++++++++++++++++ SPECS/spice-vdagent.spec | 26 +- 16 files changed, 3372 insertions(+), 2 deletions(-) create mode 100644 SOURCES/0005-vdagentd-Use-bool-for-agent_owns_clipboard-and-clien.patch create mode 100644 SOURCES/0006-vdagentd-Automatically-release-agent_data.patch create mode 100644 SOURCES/0007-vdagent-connection-Pass-err-to-g_credentials_get_uni.patch create mode 100644 SOURCES/0008-vdagentd-Better-check-for-vdagent_connection_get_pee.patch create mode 100644 SOURCES/0009-vdagentd-Avoid-calling-chmod.patch create mode 100644 SOURCES/0010-Avoids-unchecked-file-transfer-IDs-allocation-and-us.patch create mode 100644 SOURCES/0011-Avoids-uncontrolled-active_xfers-allocations.patch create mode 100644 SOURCES/0012-Avoids-unlimited-agent-connections.patch create mode 100644 SOURCES/0013-Avoids-user-session-hijacking.patch create mode 100644 SOURCES/0014-Better-check-for-sessions.patch create mode 100644 SOURCES/0015-vdagentd-Limit-number-of-agents-per-session-to-1.patch create mode 100644 SOURCES/0016-cleanup-active_xfers-when-the-client-disconnects.patch create mode 100644 SOURCES/0017-vdagentd-do-not-allow-to-use-an-already-used-file-xf.patch create mode 100644 SOURCES/0018-Add-a-test-for-session_info.patch create mode 100644 SOURCES/0019-wayland-fix-monitor-mapping-issues.patch diff --git a/SOURCES/0005-vdagentd-Use-bool-for-agent_owns_clipboard-and-clien.patch b/SOURCES/0005-vdagentd-Use-bool-for-agent_owns_clipboard-and-clien.patch new file mode 100644 index 0000000..7a5166a --- /dev/null +++ b/SOURCES/0005-vdagentd-Use-bool-for-agent_owns_clipboard-and-clien.patch @@ -0,0 +1,107 @@ +From 4bb11e214304209e37afe4bf324a7ce9e56f351c Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Mon, 21 Sep 2020 06:53:45 +0100 +Subject: [PATCH vd_agent_linux 05/17] vdagentd: Use bool for + agent_owns_clipboard and client_connected +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +More clear (instaed of 0/1) and save some bytes. + +Signed-off-by: Frediano Ziglio +Acked-by: Jakub Janků +--- + src/vdagentd/vdagentd.c | 19 ++++++++++--------- + 1 file changed, 10 insertions(+), 9 deletions(-) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index 753c9bf..051de74 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -23,6 +23,7 @@ + + #include + #include ++#include + #include + #include + #include +@@ -77,9 +78,9 @@ static int capabilities_size = 0; + static const char *active_session = NULL; + static unsigned int session_count = 0; + static UdscsConnection *active_session_conn = NULL; +-static int agent_owns_clipboard[256] = { 0, }; ++static bool agent_owns_clipboard[256] = { false, }; + static int retval = 0; +-static int client_connected = 0; ++static bool client_connected = false; + static int max_clipboard = -1; + static uint32_t clipboard_serial[256]; + +@@ -155,7 +156,7 @@ static void do_client_disconnect(void) + if (client_connected) { + udscs_server_write_all(server, VDAGENTD_CLIENT_DISCONNECTED, 0, 0, + NULL, 0); +- client_connected = 0; ++ client_connected = false; + } + } + +@@ -239,7 +240,7 @@ static void do_client_capabilities(VirtioPort *vport, + do_client_disconnect(); + if (debug) + syslog(LOG_DEBUG, "New client connected"); +- client_connected = 1; ++ client_connected = true; + memset(clipboard_serial, 0, sizeof(clipboard_serial)); + send_capabilities(vport, 0); + } +@@ -286,7 +287,7 @@ static void do_client_clipboard(VirtioPort *vport, + } + + msg_type = VDAGENTD_CLIPBOARD_GRAB; +- agent_owns_clipboard[selection] = 0; ++ agent_owns_clipboard[selection] = false; + break; + case VD_AGENT_CLIPBOARD_REQUEST: { + VDAgentClipboardRequest *req = (VDAgentClipboardRequest *)data; +@@ -624,7 +625,7 @@ static void virtio_port_read_complete( + + static void virtio_port_error_cb(VDAgentConnection *conn, GError *err) + { +- gboolean old_client_connected = client_connected; ++ bool old_client_connected = client_connected; + syslog(LOG_CRIT, "AIIEEE lost spice client connection, reconnecting (err: %s)", + err ? err->message : ""); + g_clear_error(&err); +@@ -717,7 +718,7 @@ static void do_agent_clipboard(UdscsConnection *conn, + switch (header->type) { + case VDAGENTD_CLIPBOARD_GRAB: + msg_type = VD_AGENT_CLIPBOARD_GRAB; +- agent_owns_clipboard[selection] = 1; ++ agent_owns_clipboard[selection] = true; + break; + case VDAGENTD_CLIPBOARD_REQUEST: + msg_type = VD_AGENT_CLIPBOARD_REQUEST; +@@ -737,7 +738,7 @@ static void do_agent_clipboard(UdscsConnection *conn, + case VDAGENTD_CLIPBOARD_RELEASE: + msg_type = VD_AGENT_CLIPBOARD_RELEASE; + size = 0; +- agent_owns_clipboard[selection] = 0; ++ agent_owns_clipboard[selection] = false; + break; + default: + syslog(LOG_WARNING, "unexpected clipboard message type"); +@@ -851,7 +852,7 @@ static void release_clipboards(void) + vdagent_virtio_port_write(virtio_port, VDP_CLIENT_PORT, + VD_AGENT_CLIPBOARD_RELEASE, 0, &sel, 1); + } +- agent_owns_clipboard[sel] = 0; ++ agent_owns_clipboard[sel] = false; + } + } + +-- +2.26.2 + diff --git a/SOURCES/0006-vdagentd-Automatically-release-agent_data.patch b/SOURCES/0006-vdagentd-Automatically-release-agent_data.patch new file mode 100644 index 0000000..60cc939 --- /dev/null +++ b/SOURCES/0006-vdagentd-Automatically-release-agent_data.patch @@ -0,0 +1,72 @@ +From 21fbcf665a7ad6b761e9342d116657d5c2353592 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Mon, 21 Sep 2020 07:00:39 +0100 +Subject: [PATCH vd_agent_linux 06/17] vdagentd: Automatically release + "agent_data" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +It's not guaranteed that agent_disconnect will release the connection +so avoid to have a dandling pointer. + +Signed-off-by: Frediano Ziglio +Acked-by: Jakub Janků +--- + src/vdagentd/vdagentd.c | 17 ++++++++++------- + 1 file changed, 10 insertions(+), 7 deletions(-) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index 051de74..94b8681 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -86,6 +86,13 @@ static uint32_t clipboard_serial[256]; + + static GMainLoop *loop; + ++static void agent_data_destroy(struct agent_data *agent_data) ++{ ++ g_free(agent_data->session); ++ g_free(agent_data->screen_info); ++ g_free(agent_data); ++} ++ + static void vdagentd_quit(gint exit_code) + { + retval = exit_code; +@@ -930,7 +937,7 @@ static void agent_connect(UdscsConnection *conn) + syslog(LOG_ERR, "Could not get peer PID, disconnecting new client: %s", + err->message); + g_error_free(err); +- g_free(agent_data); ++ agent_data_destroy(agent_data); + udscs_server_destroy_connection(server, conn); + return; + } +@@ -938,7 +945,8 @@ static void agent_connect(UdscsConnection *conn) + agent_data->session = session_info_session_for_pid(session_info, pid); + } + +- g_object_set_data(G_OBJECT(conn), "agent_data", agent_data); ++ g_object_set_data_full(G_OBJECT(conn), "agent_data", agent_data, ++ (GDestroyNotify) agent_data_destroy); + udscs_write(conn, VDAGENTD_VERSION, 0, 0, + (uint8_t *)VERSION, strlen(VERSION) + 1); + update_active_session_connection(conn); +@@ -951,13 +959,8 @@ static void agent_connect(UdscsConnection *conn) + + static void agent_disconnect(VDAgentConnection *conn, GError *err) + { +- struct agent_data *agent_data = g_object_get_data(G_OBJECT(conn), "agent_data"); +- + g_hash_table_foreach_remove(active_xfers, remove_active_xfers, conn); + +- g_clear_pointer(&agent_data->session, g_free); +- g_free(agent_data->screen_info); +- g_free(agent_data); + if (err) { + syslog(LOG_ERR, "%s", err->message); + g_error_free(err); +-- +2.26.2 + diff --git a/SOURCES/0007-vdagent-connection-Pass-err-to-g_credentials_get_uni.patch b/SOURCES/0007-vdagent-connection-Pass-err-to-g_credentials_get_uni.patch new file mode 100644 index 0000000..af45c33 --- /dev/null +++ b/SOURCES/0007-vdagent-connection-Pass-err-to-g_credentials_get_uni.patch @@ -0,0 +1,34 @@ +From ac89076d412f67aa7122a9fb786af32eb5f87d5d Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Tue, 22 Sep 2020 11:45:56 +0100 +Subject: [PATCH vd_agent_linux 07/17] vdagent-connection: Pass "err" to + g_credentials_get_unix_pid +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Allows to return detailed information if g_credentials_get_unix_pid +fails. + +Signed-off-by: Frediano Ziglio +Acked-by: Julien Ropé +--- + src/vdagent-connection.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/vdagent-connection.c b/src/vdagent-connection.c +index 8c023e2..b1d4db6 100644 +--- a/src/vdagent-connection.c ++++ b/src/vdagent-connection.c +@@ -155,7 +155,7 @@ gint vdagent_connection_get_peer_pid(VDAgentConnection *self, + sock = g_socket_connection_get_socket(G_SOCKET_CONNECTION(priv->io_stream)); + cred = g_socket_get_credentials(sock, err); + if (cred) { +- pid = g_credentials_get_unix_pid(cred, NULL); ++ pid = g_credentials_get_unix_pid(cred, err); + g_object_unref(cred); + } + +-- +2.26.2 + diff --git a/SOURCES/0008-vdagentd-Better-check-for-vdagent_connection_get_pee.patch b/SOURCES/0008-vdagentd-Better-check-for-vdagent_connection_get_pee.patch new file mode 100644 index 0000000..a23bb37 --- /dev/null +++ b/SOURCES/0008-vdagentd-Better-check-for-vdagent_connection_get_pee.patch @@ -0,0 +1,44 @@ +From 64289e71a1b2a9dc6868eb810cf93ca8c0644693 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Mon, 21 Sep 2020 16:42:26 +0100 +Subject: [PATCH vd_agent_linux 08/17] vdagentd: Better check for + vdagent_connection_get_peer_pid results +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The function can return -1 and leave "err" to NULL in some cases, +do not check only for "err". + +Signed-off-by: Frediano Ziglio +Acked-by: Julien Ropé +--- + src/vdagentd/vdagentd.c | 12 ++++++++---- + 1 file changed, 8 insertions(+), 4 deletions(-) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index 94b8681..12cbbd0 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -933,10 +933,14 @@ static void agent_connect(UdscsConnection *conn) + + if (session_info) { + pid = vdagent_connection_get_peer_pid(VDAGENT_CONNECTION(conn), &err); +- if (err) { +- syslog(LOG_ERR, "Could not get peer PID, disconnecting new client: %s", +- err->message); +- g_error_free(err); ++ if (err || pid <= 0) { ++ static const char msg[] = "Could not get peer PID, disconnecting new client"; ++ if (err) { ++ syslog(LOG_ERR, "%s: %s", msg, err->message); ++ g_error_free(err); ++ } else { ++ syslog(LOG_ERR, "%s", msg); ++ } + agent_data_destroy(agent_data); + udscs_server_destroy_connection(server, conn); + return; +-- +2.26.2 + diff --git a/SOURCES/0009-vdagentd-Avoid-calling-chmod.patch b/SOURCES/0009-vdagentd-Avoid-calling-chmod.patch new file mode 100644 index 0000000..adf00fd --- /dev/null +++ b/SOURCES/0009-vdagentd-Avoid-calling-chmod.patch @@ -0,0 +1,49 @@ +From dd46157d3faa95a12fc6f04cd2515f200e3ca465 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Thu, 24 Sep 2020 12:13:24 +0100 +Subject: [PATCH vd_agent_linux 09/17] vdagentd: Avoid calling chmod + +Create the socket with the right permissions using umask. +This also prevents possible symlink exploitation in case socket +path is not secure. + +Signed-off-by: Frediano Ziglio +Acked-by: Uri Lublin +--- + src/vdagentd/vdagentd.c | 12 ++---------- + 1 file changed, 2 insertions(+), 10 deletions(-) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index 12cbbd0..eddfcf6 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -1211,7 +1211,9 @@ int main(int argc, char *argv[]) + /* systemd socket activation not enabled, create our own */ + #endif /* WITH_SYSTEMD_SOCKET_ACTIVATION */ + { ++ mode_t mode = umask(0111); + udscs_server_listen_to_address(server, vdagentd_socket, &err); ++ umask(mode); + } + + if (err) { +@@ -1222,16 +1224,6 @@ int main(int argc, char *argv[]) + return 1; + } + +- /* no need to set permissions on a socket that was provided by systemd */ +- if (own_socket) { +- if (chmod(vdagentd_socket, 0666)) { +- syslog(LOG_CRIT, "Fatal could not change permissions on %s: %m", +- vdagentd_socket); +- udscs_destroy_server(server); +- return 1; +- } +- } +- + #ifdef WITH_STATIC_UINPUT + uinput = vdagentd_uinput_create(uinput_device, 1024, 768, NULL, 0, + debug > 1, uinput_fake); +-- +2.26.2 + diff --git a/SOURCES/0010-Avoids-unchecked-file-transfer-IDs-allocation-and-us.patch b/SOURCES/0010-Avoids-unchecked-file-transfer-IDs-allocation-and-us.patch new file mode 100644 index 0000000..16e1bfd --- /dev/null +++ b/SOURCES/0010-Avoids-unchecked-file-transfer-IDs-allocation-and-us.patch @@ -0,0 +1,84 @@ +From 956608c1344f185e39294004b64906a7e1b5c14c Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Sat, 19 Sep 2020 15:13:42 +0100 +Subject: [PATCH vd_agent_linux 10/17] Avoids unchecked file transfer IDs + allocation and usage + +Avoid agents allocating file transfers. +The "active_xfers" entries are now inserted when client start sending +files. +Also different agents cannot mess with other agent transfers as a +transfer is bound to a single agent. + +This issue was reported by SUSE security team. + +Signed-off-by: Frediano Ziglio +Acked-by: Uri Lublin +--- + src/vdagentd/vdagentd.c | 28 ++++++++++++++++++++++------ + 1 file changed, 22 insertions(+), 6 deletions(-) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index eddfcf6..8961a99 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -381,9 +381,11 @@ static void do_client_file_xfer(VirtioPort *vport, + s->id, VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED, NULL, 0); + return; + } +- udscs_write(active_session_conn, VDAGENTD_FILE_XFER_START, 0, 0, +- data, message_header->size); +- return; ++ msg_type = VDAGENTD_FILE_XFER_START; ++ id = s->id; ++ // associate the id with the active connection ++ g_hash_table_insert(active_xfers, GUINT_TO_POINTER(id), active_session_conn); ++ break; + } + case VD_AGENT_FILE_XFER_STATUS: { + VDAgentFileXferStatusMessage *s = (VDAgentFileXferStatusMessage *)data; +@@ -408,6 +410,12 @@ static void do_client_file_xfer(VirtioPort *vport, + return; + } + udscs_write(conn, msg_type, 0, 0, data, message_header->size); ++ ++ // client told that transfer is ended, agents too stop the transfer ++ // and release resources ++ if (message_header->type == VD_AGENT_FILE_XFER_STATUS) { ++ g_hash_table_remove(active_xfers, GUINT_TO_POINTER(id)); ++ } + } + + static void forward_data_to_session_agent(uint32_t type, uint8_t *data, size_t size) +@@ -1015,6 +1023,15 @@ static void do_agent_file_xfer_status(UdscsConnection *conn, + const gchar *log_msg = NULL; + guint data_size = 0; + ++ UdscsConnection *task_conn = g_hash_table_lookup(active_xfers, task_id); ++ if (task_conn == NULL || task_conn != conn) { ++ // Protect against misbehaving agent. ++ // Ignore the message, but do not disconnect the agent, to protect against ++ // a misbehaving client that tries to disconnect a good agent ++ // e.g. by sending a new task and immediately cancelling it. ++ return; ++ } ++ + /* header->arg1 = file xfer task id, header->arg2 = file xfer status */ + switch (header->arg2) { + case VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE: +@@ -1029,10 +1046,9 @@ static void do_agent_file_xfer_status(UdscsConnection *conn, + send_file_xfer_status(virtio_port, log_msg, header->arg1, header->arg2, + data, data_size); + +- if (header->arg2 == VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA) +- g_hash_table_insert(active_xfers, task_id, conn); +- else ++ if (header->arg2 != VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA) { + g_hash_table_remove(active_xfers, task_id); ++ } + } + + static void agent_read_complete(UdscsConnection *conn, +-- +2.26.2 + diff --git a/SOURCES/0011-Avoids-uncontrolled-active_xfers-allocations.patch b/SOURCES/0011-Avoids-uncontrolled-active_xfers-allocations.patch new file mode 100644 index 0000000..d278e62 --- /dev/null +++ b/SOURCES/0011-Avoids-uncontrolled-active_xfers-allocations.patch @@ -0,0 +1,61 @@ +From b173eba1698138f92b08d4deeaac4d2979a67bbf Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Fri, 2 Oct 2020 12:27:59 +0100 +Subject: [PATCH vd_agent_linux 11/17] Avoids uncontrolled "active_xfers" + allocations + +Limit the number of active file transfers possibly causing DoSes +consuming memory in "active_xfers". + +This issue was reported by SUSE security team. + +Signed-off-by: Frediano Ziglio +Acked-by: Uri Lublin +--- + src/vdagentd/vdagentd.c | 23 +++++++++++++++++++++++ + 1 file changed, 23 insertions(+) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index 8961a99..b31941d 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -47,6 +47,14 @@ + + #define DEFAULT_UINPUT_DEVICE "/dev/uinput" + ++// Maximum number of transfers active at any time. ++// Avoid DoS from client. ++// As each transfer could likely end up taking a file descriptor ++// it is good to have a limit less than the number of file descriptors ++// in the process (by default 1024). The daemon do not open file ++// descriptors for the transfers but the agents do. ++#define MAX_ACTIVE_TRANSFERS 128 ++ + struct agent_data { + char *session; + int width; +@@ -380,6 +388,21 @@ static void do_client_file_xfer(VirtioPort *vport, + "Cancelling client file-xfer request %u", + s->id, VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED, NULL, 0); + return; ++ } else if (g_hash_table_size(active_xfers) >= MAX_ACTIVE_TRANSFERS) { ++ VDAgentFileXferStatusError error = { ++ GUINT32_TO_LE(VD_AGENT_FILE_XFER_STATUS_ERROR_GLIB_IO), ++ GUINT32_TO_LE(G_IO_ERROR_TOO_MANY_OPEN_FILES), ++ }; ++ size_t detail_size = sizeof(error); ++ if (!VD_AGENT_HAS_CAPABILITY(capabilities, capabilities_size, ++ VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS)) { ++ detail_size = 0; ++ } ++ send_file_xfer_status(vport, ++ "Too many transfers ongoing. " ++ "Cancelling client file-xfer request %u", ++ s->id, VD_AGENT_FILE_XFER_STATUS_ERROR, (void*) &error, detail_size); ++ return; + } + msg_type = VDAGENTD_FILE_XFER_START; + id = s->id; +-- +2.26.2 + diff --git a/SOURCES/0012-Avoids-unlimited-agent-connections.patch b/SOURCES/0012-Avoids-unlimited-agent-connections.patch new file mode 100644 index 0000000..7bd215f --- /dev/null +++ b/SOURCES/0012-Avoids-unlimited-agent-connections.patch @@ -0,0 +1,50 @@ +From 6e5b9924b172be4f33c7fc264a8ff1d6109b79fe Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Sun, 20 Sep 2020 08:05:37 +0100 +Subject: [PATCH vd_agent_linux 12/17] Avoids unlimited agent connections + +Limit the number of agents that can be connected. +Avoids reaching the maximum number of files in a process. +Beside one file descriptor per agent the daemon open just some +other fixed number of files. + +This issue was reported by SUSE security team. + +Signed-off-by: Frediano Ziglio +--- + src/udscs.c | 12 ++++++++++++ + 1 file changed, 12 insertions(+) + +diff --git a/src/udscs.c b/src/udscs.c +index 7c99eed..3df67b3 100644 +--- a/src/udscs.c ++++ b/src/udscs.c +@@ -30,6 +30,12 @@ + #include "vdagentd-proto-strings.h" + #include "vdagent-connection.h" + ++// Maximum number of connected agents. ++// Avoid DoS from agents. ++// As each connection end up taking a file descriptor is good to have a limit ++// less than the number of file descriptors in the process (by default 1024). ++#define MAX_CONNECTED_AGENTS 128 ++ + struct _UdscsConnection { + VDAgentConnection parent_instance; + int debug; +@@ -254,6 +260,12 @@ static gboolean udscs_server_accept_cb(GSocketService *service, + struct udscs_server *server = user_data; + UdscsConnection *new_conn; + ++ /* prevents DoS having too many agents attached */ ++ if (g_list_length(server->connections) >= MAX_CONNECTED_AGENTS) { ++ syslog(LOG_ERR, "Too many agents connected"); ++ return TRUE; ++ } ++ + new_conn = g_object_new(UDSCS_TYPE_CONNECTION, NULL); + new_conn->debug = server->debug; + new_conn->read_callback = server->read_callback; +-- +2.26.2 + diff --git a/SOURCES/0013-Avoids-user-session-hijacking.patch b/SOURCES/0013-Avoids-user-session-hijacking.patch new file mode 100644 index 0000000..e78771d --- /dev/null +++ b/SOURCES/0013-Avoids-user-session-hijacking.patch @@ -0,0 +1,144 @@ +From 5d9881309d0aeeebbc177d8af6dc26aa2ba56cfc Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Sun, 20 Sep 2020 08:06:16 +0100 +Subject: [PATCH vd_agent_linux 13/17] Avoids user session hijacking + +Avoids user hijacking sessions by reusing PID. +In theory an attacker could: +- open a connection to the daemon; +- fork and exit the process but keep the file descriptor open + (inheriting or duplicating it in forked process); +- force OS to recycle the initial PID, by creating many short lived + processes. +Daemon would detect the old PID as having the new session. +Check the user to avoid such replacements. + +This issue was reported by SUSE security team. + +Signed-off-by: Frediano Ziglio +Acked-by: Uri Lublin +--- + src/vdagent-connection.c | 13 +++++++------ + src/vdagent-connection.h | 13 +++++++++---- + src/vdagentd/vdagentd.c | 31 +++++++++++++++++++++++++++---- + 3 files changed, 43 insertions(+), 14 deletions(-) + +diff --git a/src/vdagent-connection.c b/src/vdagent-connection.c +index b1d4db6..fb331be 100644 +--- a/src/vdagent-connection.c ++++ b/src/vdagent-connection.c +@@ -142,24 +142,25 @@ void vdagent_connection_destroy(gpointer p) + g_object_unref(self); + } + +-gint vdagent_connection_get_peer_pid(VDAgentConnection *self, +- GError **err) ++PidUid vdagent_connection_get_peer_pid_uid(VDAgentConnection *self, ++ GError **err) + { + VDAgentConnectionPrivate *priv = vdagent_connection_get_instance_private(self); + GSocket *sock; + GCredentials *cred; +- gint pid = -1; ++ PidUid pid_uid = { -1, -1 }; + +- g_return_val_if_fail(G_IS_SOCKET_CONNECTION(priv->io_stream), pid); ++ g_return_val_if_fail(G_IS_SOCKET_CONNECTION(priv->io_stream), pid_uid); + + sock = g_socket_connection_get_socket(G_SOCKET_CONNECTION(priv->io_stream)); + cred = g_socket_get_credentials(sock, err); + if (cred) { +- pid = g_credentials_get_unix_pid(cred, err); ++ pid_uid.pid = g_credentials_get_unix_pid(cred, err); ++ pid_uid.uid = g_credentials_get_unix_user(cred, err); + g_object_unref(cred); + } + +- return pid; ++ return pid_uid; + } + + /* Performs single write operation, +diff --git a/src/vdagent-connection.h b/src/vdagent-connection.h +index 9d5a212..c515a79 100644 +--- a/src/vdagent-connection.h ++++ b/src/vdagent-connection.h +@@ -92,10 +92,15 @@ void vdagent_connection_write(VDAgentConnection *self, + /* Synchronously write all queued messages to the output stream. */ + void vdagent_connection_flush(VDAgentConnection *self); + +-/* Returns the PID of the foreign process connected to the socket +- * or -1 with @err set. */ +-gint vdagent_connection_get_peer_pid(VDAgentConnection *self, +- GError **err); ++typedef struct PidUid { ++ pid_t pid; ++ uid_t uid; ++} PidUid; ++ ++/* Returns the PID and UID of the foreign process connected to the socket ++ * or fill @err set. */ ++PidUid vdagent_connection_get_peer_pid_uid(VDAgentConnection *self, ++ GError **err); + + G_END_DECLS + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index b31941d..e98fbe5 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -955,16 +955,28 @@ static gboolean remove_active_xfers(gpointer key, gpointer value, gpointer conn) + return 0; + } + ++/* Check a given process has a given UID */ ++static bool check_uid_of_pid(pid_t pid, uid_t uid) ++{ ++ char fn[128]; ++ struct stat st; ++ ++ snprintf(fn, sizeof(fn), "/proc/%u/status", (unsigned) pid); ++ if (stat(fn, &st) != 0 || st.st_uid != uid) { ++ return false; ++ } ++ return true; ++} ++ + static void agent_connect(UdscsConnection *conn) + { + struct agent_data *agent_data; + agent_data = g_new0(struct agent_data, 1); + GError *err = NULL; +- gint pid; + + if (session_info) { +- pid = vdagent_connection_get_peer_pid(VDAGENT_CONNECTION(conn), &err); +- if (err || pid <= 0) { ++ PidUid pid_uid = vdagent_connection_get_peer_pid_uid(VDAGENT_CONNECTION(conn), &err); ++ if (err || pid_uid.pid <= 0) { + static const char msg[] = "Could not get peer PID, disconnecting new client"; + if (err) { + syslog(LOG_ERR, "%s: %s", msg, err->message); +@@ -977,7 +989,18 @@ static void agent_connect(UdscsConnection *conn) + return; + } + +- agent_data->session = session_info_session_for_pid(session_info, pid); ++ agent_data->session = session_info_session_for_pid(session_info, pid_uid.pid); ++ ++ /* Check that the UID of the PID did not change, this should be done after ++ * computing the session to avoid race conditions. ++ * This can happen as vdagent_connection_get_peer_pid_uid get information ++ * from the time of creating the socket, but the process in the meantime ++ * have been replaced */ ++ if (!check_uid_of_pid(pid_uid.pid, pid_uid.uid)) { ++ agent_data_destroy(agent_data); ++ udscs_server_destroy_connection(server, conn); ++ return; ++ } + } + + g_object_set_data_full(G_OBJECT(conn), "agent_data", agent_data, +-- +2.26.2 + diff --git a/SOURCES/0014-Better-check-for-sessions.patch b/SOURCES/0014-Better-check-for-sessions.patch new file mode 100644 index 0000000..11c98f3 --- /dev/null +++ b/SOURCES/0014-Better-check-for-sessions.patch @@ -0,0 +1,164 @@ +From dc4d1b8e3e315933c82b23b2806a9cf973e78e78 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Mon, 21 Sep 2020 07:06:09 +0100 +Subject: [PATCH vd_agent_linux 14/17] Better check for sessions + +Do not allow other users to hijack a session checking that +the process is launched by the owner of the session. + +Signed-off-by: Frediano Ziglio +Acked-by: Uri Lublin +--- + src/vdagentd/console-kit.c | 67 +++++++++++++++++++++++++++++++ + src/vdagentd/dummy-session-info.c | 5 +++ + src/vdagentd/session-info.h | 3 ++ + src/vdagentd/systemd-login.c | 9 +++++ + src/vdagentd/vdagentd.c | 10 ++++- + 5 files changed, 93 insertions(+), 1 deletion(-) + +diff --git a/src/vdagentd/console-kit.c b/src/vdagentd/console-kit.c +index fc630f1..09a8402 100644 +--- a/src/vdagentd/console-kit.c ++++ b/src/vdagentd/console-kit.c +@@ -568,3 +568,70 @@ exit: + } + return ret; + } ++ ++uid_t session_info_uid_for_session(struct session_info *info, const char *session) ++{ ++ DBusError error; ++ DBusMessage *message = NULL; ++ DBusMessage *reply = NULL; ++ uint32_t uid; ++ uid_t ret = -1; ++ const char *err_msg; ++ ++ g_return_val_if_fail(info != NULL, ret); ++ g_return_val_if_fail(info->connection != NULL, ret); ++ g_return_val_if_fail(info->active_session != NULL, ret); ++ ++ dbus_error_init(&error); ++ ++ err_msg = "(console-kit) Unable to create dbus message for GetUnixUser"; ++ message = dbus_message_new_method_call(INTERFACE_CONSOLE_KIT, ++ session, ++ INTERFACE_CONSOLE_KIT_SESSION, ++ "GetUnixUser"); ++ if (message == NULL) { ++ goto exit; ++ } ++ ++ err_msg = "(console-kit) GetUnixUser failed"; ++ reply = dbus_connection_send_with_reply_and_block(info->connection, ++ message, ++ -1, ++ &error); ++ if (reply == NULL || dbus_error_is_set(&error)) { ++ goto exit; ++ } ++ ++ dbus_error_init(&error); ++ err_msg = "(console-kit) fail to get session-type from reply"; ++ if (!dbus_message_get_args(reply, ++ &error, ++ DBUS_TYPE_UINT32, &uid, ++ DBUS_TYPE_INVALID)) { ++ goto exit; ++ } ++ ++ if (info->verbose) { ++ syslog(LOG_DEBUG, "(console-kit) unix user is '%u'", (unsigned) uid); ++ } ++ ++ err_msg = NULL; ++ ret = uid; ++ ++exit: ++ if (err_msg) { ++ if (dbus_error_is_set(&error)) { ++ syslog(LOG_ERR, "%s: %s", err_msg, error.message); ++ dbus_error_free(&error); ++ } else { ++ syslog(LOG_ERR, "%s", err_msg); ++ } ++ } ++ if (reply != NULL) { ++ dbus_message_unref(reply); ++ } ++ if (message != NULL) { ++ dbus_message_unref(message); ++ } ++ return ret; ++} +diff --git a/src/vdagentd/dummy-session-info.c b/src/vdagentd/dummy-session-info.c +index 7fd1eea..137c01a 100644 +--- a/src/vdagentd/dummy-session-info.c ++++ b/src/vdagentd/dummy-session-info.c +@@ -55,3 +55,8 @@ gboolean session_info_session_is_locked(G_GNUC_UNUSED struct session_info *si) + { + return FALSE; + } ++ ++uid_t session_info_uid_for_session(struct session_info *si, const char *session) ++{ ++ return -1; ++} +diff --git a/src/vdagentd/session-info.h b/src/vdagentd/session-info.h +index c8edb86..96aa8d3 100644 +--- a/src/vdagentd/session-info.h ++++ b/src/vdagentd/session-info.h +@@ -40,4 +40,7 @@ char *session_info_session_for_pid(struct session_info *ck, uint32_t pid); + gboolean session_info_session_is_locked(struct session_info *si); + gboolean session_info_is_user(struct session_info *si); + ++/* get owner of a given session */ ++uid_t session_info_uid_for_session(struct session_info *si, const char *session); ++ + #endif +diff --git a/src/vdagentd/systemd-login.c b/src/vdagentd/systemd-login.c +index 0b8f3c1..0e1ff3f 100644 +--- a/src/vdagentd/systemd-login.c ++++ b/src/vdagentd/systemd-login.c +@@ -392,3 +392,12 @@ gboolean session_info_is_user(struct session_info *si) + + return ret; + } ++ ++uid_t session_info_uid_for_session(struct session_info *si, const char *session) ++{ ++ uid_t ret = -1; ++ if (sd_session_get_uid(session, &ret) < 0) { ++ return -1; ++ } ++ return ret; ++} +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index e98fbe5..bb39340 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -991,12 +991,20 @@ static void agent_connect(UdscsConnection *conn) + + agent_data->session = session_info_session_for_pid(session_info, pid_uid.pid); + ++ uid_t session_uid = session_info_uid_for_session(session_info, agent_data->session); ++ + /* Check that the UID of the PID did not change, this should be done after + * computing the session to avoid race conditions. + * This can happen as vdagent_connection_get_peer_pid_uid get information + * from the time of creating the socket, but the process in the meantime + * have been replaced */ +- if (!check_uid_of_pid(pid_uid.pid, pid_uid.uid)) { ++ if (!check_uid_of_pid(pid_uid.pid, pid_uid.uid) || ++ /* Check that the user launching the Agent is the same as session one ++ * or root user. ++ * This prevents session hijacks from other users. */ ++ (pid_uid.uid != 0 && pid_uid.uid != session_uid)) { ++ syslog(LOG_ERR, "UID mismatch: UID=%u PID=%u suid=%u", pid_uid.uid, ++ pid_uid.pid, session_uid); + agent_data_destroy(agent_data); + udscs_server_destroy_connection(server, conn); + return; +-- +2.26.2 + diff --git a/SOURCES/0015-vdagentd-Limit-number-of-agents-per-session-to-1.patch b/SOURCES/0015-vdagentd-Limit-number-of-agents-per-session-to-1.patch new file mode 100644 index 0000000..a273246 --- /dev/null +++ b/SOURCES/0015-vdagentd-Limit-number-of-agents-per-session-to-1.patch @@ -0,0 +1,57 @@ +From 570b15e0ea71950ff14ddc2bf667e9e361720939 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Thu, 24 Sep 2020 12:13:44 +0100 +Subject: [PATCH vd_agent_linux 15/17] vdagentd: Limit number of agents per + session to 1 + +Signed-off-by: Frediano Ziglio +Acked-by: Uri Lublin +--- + src/vdagentd/vdagentd.c | 24 ++++++++++++++++++++++++ + 1 file changed, 24 insertions(+) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index bb39340..5ef547e 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -955,6 +955,20 @@ static gboolean remove_active_xfers(gpointer key, gpointer value, gpointer conn) + return 0; + } + ++/* Check if this connection matches the passed session */ ++static int connection_matches_session(UdscsConnection *conn, void *priv) ++{ ++ const char *session = priv; ++ const struct agent_data *agent_data = g_object_get_data(G_OBJECT(conn), "agent_data"); ++ ++ if (!agent_data || !agent_data->session || ++ strcmp(agent_data->session, session) != 0) { ++ return 0; ++ } ++ ++ return 1; ++} ++ + /* Check a given process has a given UID */ + static bool check_uid_of_pid(pid_t pid, uid_t uid) + { +@@ -1007,6 +1021,16 @@ static void agent_connect(UdscsConnection *conn) + udscs_server_destroy_connection(server, conn); + return; + } ++ ++ // Check there are no other connection for this session ++ // Note that "conn" is not counted as "agent_data" is still not attached to it ++ if (udscs_server_for_all_clients(server, connection_matches_session, ++ agent_data->session) > 0) { ++ syslog(LOG_ERR, "An agent is already connected for this session"); ++ agent_data_destroy(agent_data); ++ udscs_server_destroy_connection(server, conn); ++ return; ++ } + } + + g_object_set_data_full(G_OBJECT(conn), "agent_data", agent_data, +-- +2.26.2 + diff --git a/SOURCES/0016-cleanup-active_xfers-when-the-client-disconnects.patch b/SOURCES/0016-cleanup-active_xfers-when-the-client-disconnects.patch new file mode 100644 index 0000000..5dcc163 --- /dev/null +++ b/SOURCES/0016-cleanup-active_xfers-when-the-client-disconnects.patch @@ -0,0 +1,27 @@ +From 1153bb8906377e155dccf730762b83f00a8d47c2 Mon Sep 17 00:00:00 2001 +From: Uri Lublin +Date: Wed, 7 Oct 2020 19:34:57 +0300 +Subject: [PATCH vd_agent_linux 16/17] cleanup active_xfers when the client + disconnects + +Signed-off-by: Uri Lublin +Acked-by: Frediano Ziglio +--- + src/vdagentd/vdagentd.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index 5ef547e..9243cfb 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -168,6 +168,7 @@ static void send_capabilities(VirtioPort *vport, + + static void do_client_disconnect(void) + { ++ g_hash_table_remove_all(active_xfers); + if (client_connected) { + udscs_server_write_all(server, VDAGENTD_CLIENT_DISCONNECTED, 0, 0, + NULL, 0); +-- +2.26.2 + diff --git a/SOURCES/0017-vdagentd-do-not-allow-to-use-an-already-used-file-xf.patch b/SOURCES/0017-vdagentd-do-not-allow-to-use-an-already-used-file-xf.patch new file mode 100644 index 0000000..3d0b2e5 --- /dev/null +++ b/SOURCES/0017-vdagentd-do-not-allow-to-use-an-already-used-file-xf.patch @@ -0,0 +1,33 @@ +From 12708a0fc9f3da1d1cc492bb3a49844b02e76b40 Mon Sep 17 00:00:00 2001 +From: Uri Lublin +Date: Sun, 11 Oct 2020 20:59:17 +0300 +Subject: [PATCH vd_agent_linux 17/17] vdagentd: do not allow to use an already + used file-xfer id + +Signed-off-by: Uri Lublin +Acked-by: Frediano Ziglio +--- + src/vdagentd/vdagentd.c | 7 +++++++ + 1 file changed, 7 insertions(+) + +diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c +index 9243cfb..279e7c3 100644 +--- a/src/vdagentd/vdagentd.c ++++ b/src/vdagentd/vdagentd.c +@@ -404,6 +404,13 @@ static void do_client_file_xfer(VirtioPort *vport, + "Cancelling client file-xfer request %u", + s->id, VD_AGENT_FILE_XFER_STATUS_ERROR, (void*) &error, detail_size); + return; ++ } else if (g_hash_table_lookup(active_xfers, GUINT_TO_POINTER(s->id)) != NULL) { ++ // id is already used -- client is confused ++ send_file_xfer_status(vport, ++ "File transfer ID is already used. " ++ "Cancelling client file-xfer request %u", ++ s->id, VD_AGENT_FILE_XFER_STATUS_ERROR, NULL, 0); ++ return; + } + msg_type = VDAGENTD_FILE_XFER_START; + id = s->id; +-- +2.26.2 + diff --git a/SOURCES/0018-Add-a-test-for-session_info.patch b/SOURCES/0018-Add-a-test-for-session_info.patch new file mode 100644 index 0000000..6956628 --- /dev/null +++ b/SOURCES/0018-Add-a-test-for-session_info.patch @@ -0,0 +1,130 @@ +From a77b09e5d77e5cf1d4fdd38d018ecf164cd01df9 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Tue, 20 Oct 2020 16:38:37 +0100 +Subject: [PATCH 10/10] Add a test for session_info + +Test from Uri, integrated in test suite. + +Signed-off-by: Uri Lublin +Signed-off-by: Frediano Ziglio +--- + Makefile.am | 30 ++++++++++++++++++++ + tests/test-session-info.c | 58 +++++++++++++++++++++++++++++++++++++++ + 2 files changed, 88 insertions(+) + create mode 100644 tests/test-session-info.c + +diff --git a/Makefile.am b/Makefile.am +index 575ba52..f4c65b4 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -109,13 +109,43 @@ src_spice_vdagentd_SOURCES = \ + src/vdagentd/virtio-port.h \ + $(NULL) + ++tests_test_session_info_CFLAGS = \ ++ $(DBUS_CFLAGS) \ ++ $(LIBSYSTEMD_DAEMON_CFLAGS) \ ++ $(LIBSYSTEMD_LOGIN_CFLAGS) \ ++ $(SPICE_CFLAGS) \ ++ $(GIO2_CFLAGS) \ ++ -I$(srcdir)/src \ ++ -I$(srcdir)/src/vdagentd \ ++ -DUDSCS_NO_SERVER \ ++ $(NULL) ++ ++tests_test_session_info_LDADD = \ ++ $(DBUS_LIBS) \ ++ $(LIBSYSTEMD_DAEMON_LIBS) \ ++ $(LIBSYSTEMD_LOGIN_LIBS) \ ++ $(SPICE_LIBS) \ ++ $(GIO2_LIBS) \ ++ $(NULL) ++ ++tests_test_session_info_SOURCES = \ ++ $(common_sources) \ ++ src/vdagentd/session-info.h \ ++ tests/test-session-info.c \ ++ $(NULL) ++ ++check_PROGRAMS += tests/test-session-info ++ + if HAVE_CONSOLE_KIT + src_spice_vdagentd_SOURCES += src/vdagentd/console-kit.c ++tests_test_session_info_SOURCES += src/vdagentd/console-kit.c + else + if HAVE_LIBSYSTEMD_LOGIN + src_spice_vdagentd_SOURCES += src/vdagentd/systemd-login.c ++tests_test_session_info_SOURCES += src/vdagentd/systemd-login.c + else + src_spice_vdagentd_SOURCES += src/vdagentd/dummy-session-info.c ++tests_test_session_info_SOURCES += src/vdagentd/dummy-session-info.c + endif + endif + +diff --git a/tests/test-session-info.c b/tests/test-session-info.c +new file mode 100644 +index 0000000..dae3ec6 +--- /dev/null ++++ b/tests/test-session-info.c +@@ -0,0 +1,58 @@ ++/* test-session-info.c - test session info ++ ++ Copyright 2020 Red Hat, Inc. ++ ++ This program is free software: you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation, either version 3 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with this program. If not, see . ++*/ ++#include ++ ++#undef NDEBUG ++#include ++#include ++#include ++#include ++ ++#include "session-info.h" ++ ++int main(int argc, char *argv[]) ++{ ++ int pid, uid, ck_uid; ++ ++ pid = (int)getpid(); ++ ++ struct session_info *session_info = session_info_create(1); ++ if (session_info == NULL) { ++ return 1; ++ } ++ ++ char *session = session_info_session_for_pid(session_info, pid); ++ if (session == NULL) { ++ session_info_destroy(session_info); ++ return 2; ++ } ++ ck_uid = session_info_uid_for_session(session_info, session); ++ ++ free(session); ++ session_info_destroy(session_info); ++ ++ uid = getuid(); ++ printf("MAIN: uid is %d, ck_uid is %d\n", uid, ck_uid); ++ ++ if (uid != ck_uid) { ++ fprintf(stderr, "MAIN: uid (%d) does not match console-kit uid %d\n", uid, ck_uid); ++ return 3; ++ } ++ ++ return 0; ++} +-- +2.28.0 + diff --git a/SOURCES/0019-wayland-fix-monitor-mapping-issues.patch b/SOURCES/0019-wayland-fix-monitor-mapping-issues.patch new file mode 100644 index 0000000..af0179a --- /dev/null +++ b/SOURCES/0019-wayland-fix-monitor-mapping-issues.patch @@ -0,0 +1,2292 @@ +From 2f025381f8c11cab2f351a478fba80ce7ae70a67 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Tue, 25 Feb 2020 12:17:12 +0100 +Subject: [PATCH 1/4] Split X11-specific code from the core agent code +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This commit adds a set of functions (vdagent_display_*) that separates the +agent's code from the underlying backend used for handling monitor +events/settings. +Today, this does not change the behaviour of the agent, as those functions +will just call the existing X11 functions. But we will later add code to +them to support other means of dealing with monitors (i.e: Wayland +support). + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + Makefile.am | 2 + + src/vdagent/display.c | 129 ++++++++++++++++++++++++++++++++++++++++++ + src/vdagent/display.h | 37 ++++++++++++ + src/vdagent/vdagent.c | 37 ++++-------- + 4 files changed, 178 insertions(+), 27 deletions(-) + create mode 100644 src/vdagent/display.c + create mode 100644 src/vdagent/display.h + +diff --git a/Makefile.am b/Makefile.am +index 2abb5ec..431e414 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -43,6 +43,8 @@ src_spice_vdagent_SOURCES = \ + src/vdagent/clipboard.h \ + src/vdagent/device-info.c \ + src/vdagent/device-info.h \ ++ src/vdagent/display.c \ ++ src/vdagent/display.h \ + src/vdagent/file-xfers.c \ + src/vdagent/file-xfers.h \ + src/vdagent/x11-priv.h \ +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +new file mode 100644 +index 0000000..bb83762 +--- /dev/null ++++ b/src/vdagent/display.c +@@ -0,0 +1,129 @@ ++/* display.c vdagent display source code ++ ++ Copyright 2020 Red Hat, Inc. ++ ++ Red Hat Authors: ++ Julien Ropé ++ ++ This program is free software: you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation, either version 3 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with this program. If not, see . ++ */ ++ ++#include ++ ++#include ++#ifdef WITH_GTK ++#include ++#ifdef GDK_WINDOWING_X11 ++#include ++#endif ++#endif ++#include ++#include "x11.h" ++#include "x11-priv.h" ++ ++#include "display.h" ++ ++/** ++ * VDAgentDisplay and the vdagent_display_*() functions are used as wrappers for display-related ++ * operations. ++ * They allow vdagent code to call generic display functions that are independent from the underlying ++ * API (X11/GTK/etc). ++ * ++ * The display.c file contains the actual implementation and chooses what API will be called. ++ * The x11.c and x11-randr.c files contains the x11-specific functions. ++ */ ++struct VDAgentDisplay { ++ struct vdagent_x11 *x11; ++ GIOChannel *x11_channel; ++}; ++ ++struct vdagent_x11* vdagent_display_get_x11(VDAgentDisplay *display) ++{ ++ return display->x11; ++} ++ ++static gboolean x11_io_channel_cb(GIOChannel *source, GIOCondition condition, gpointer data) ++{ ++ VDAgentDisplay *display = data; ++ vdagent_x11_do_read(display->x11); ++ ++ return G_SOURCE_CONTINUE; ++} ++ ++VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int sync) ++{ ++ VDAgentDisplay *display; ++ ++ display = g_new0(VDAgentDisplay, 1); ++ display->x11 = vdagent_x11_create(vdagentd, debug, sync); ++ if (display->x11 == NULL) { ++ g_free(display); ++ return NULL; ++ } ++ ++ display->x11_channel = g_io_channel_unix_new(vdagent_x11_get_fd(display->x11)); ++ if (display->x11_channel == NULL) { ++ vdagent_x11_destroy(display->x11, TRUE); ++ g_free(display); ++ return NULL; ++ } ++ ++ g_io_add_watch(display->x11_channel, G_IO_IN, x11_io_channel_cb, display); ++ ++ return display; ++} ++ ++void vdagent_display_destroy(VDAgentDisplay *display, int vdagentd_disconnected) ++{ ++ if (!display) { ++ return; ++ } ++ ++ g_clear_pointer(&display->x11_channel, g_io_channel_unref); ++ vdagent_x11_destroy(display->x11, vdagentd_disconnected); ++} ++ ++/* Function used to determine the default location to save file-xfers, ++ xdg desktop dir or xdg download dir. We err on the safe side and use a ++ whitelist approach, so any unknown desktop will end up with saving ++ file-xfers to the xdg download dir, and opening the xdg download dir with ++ xdg-open when the file-xfer completes. */ ++int vdagent_display_has_icons_on_desktop(VDAgentDisplay *display) ++{ ++ return vdagent_x11_has_icons_on_desktop(display->x11); ++} ++ ++// handle the device info message from the server. This will allow us to ++// maintain a mapping from spice display id to xrandr output ++void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_t *data, ++ size_t size) ++{ ++ vdagent_x11_handle_graphics_device_info(display->x11, data, size); ++} ++ ++/* ++ * Set monitor configuration according to client request. ++ * ++ * On exit send current configuration to client, regardless of error. ++ * ++ * Errors: ++ * screen size too large for driver to handle. (we set the largest/smallest possible) ++ * no randr support in X server. ++ * invalid configuration request from client. ++ */ ++void vdagent_display_set_monitor_config(VDAgentDisplay *display, VDAgentMonitorsConfig *mon_config, ++ int fallback) ++{ ++ vdagent_x11_set_monitor_config(display->x11, mon_config, fallback); ++} +diff --git a/src/vdagent/display.h b/src/vdagent/display.h +new file mode 100644 +index 0000000..1f5365f +--- /dev/null ++++ b/src/vdagent/display.h +@@ -0,0 +1,37 @@ ++/* ++ * display.h- vdagent display handling header ++ ++ Copyright 2020 Red Hat, Inc. ++ ++ This program is free software: you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation, either version 3 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with this program. If not, see . ++ */ ++ ++#ifndef SRC_VDAGENT_DISPLAY_H_ ++#define SRC_VDAGENT_DISPLAY_H_ ++ ++typedef struct VDAgentDisplay VDAgentDisplay; ++ ++VDAgentDisplay *vdagent_display_create(UdscsConnection *vdagentd, int debug, int sync); ++void vdagent_display_destroy(VDAgentDisplay *display, int vdagentd_disconnected); ++ ++int vdagent_display_has_icons_on_desktop(VDAgentDisplay *display); ++void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_t *data, size_t size); ++void vdagent_display_set_monitor_config(VDAgentDisplay *display, ++ VDAgentMonitorsConfig *mon_config, ++ int fallback); ++ ++struct vdagent_x11 *vdagent_display_get_x11(VDAgentDisplay *display); ++ ++ ++#endif /* SRC_VDAGENT_DISPLAY_H_ */ +diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c +index 53b177b..af78328 100644 +--- a/src/vdagent/vdagent.c ++++ b/src/vdagent/vdagent.c +@@ -38,16 +38,15 @@ + #include "udscs.h" + #include "vdagentd-proto.h" + #include "audio.h" +-#include "x11.h" + #include "file-xfers.h" + #include "clipboard.h" ++#include "display.h" + + typedef struct VDAgent { + VDAgentClipboards *clipboards; +- struct vdagent_x11 *x11; ++ VDAgentDisplay *display; + struct vdagent_file_xfers *xfers; + UdscsConnection *conn; +- GIOChannel *x11_channel; + + GMainLoop *loop; + } VDAgent; +@@ -114,7 +113,7 @@ static const gchar *xfer_get_download_directory(VDAgent *agent) + return fx_dir; + } + +- return g_get_user_special_dir(vdagent_x11_has_icons_on_desktop(agent->x11) ? ++ return g_get_user_special_dir(vdagent_display_has_icons_on_desktop(agent->display) ? + G_USER_DIRECTORY_DESKTOP : + G_USER_DIRECTORY_DOWNLOAD); + } +@@ -144,7 +143,7 @@ static gboolean vdagent_init_file_xfer(VDAgent *agent) + } + + open_dir = fx_open_dir == -1 ? +- !vdagent_x11_has_icons_on_desktop(agent->x11) : ++ !vdagent_display_has_icons_on_desktop(agent->display) : + fx_open_dir; + + agent->xfers = vdagent_file_xfers_create(agent->conn, xfer_dir, +@@ -179,7 +178,7 @@ static void daemon_read_complete(UdscsConnection *conn, + + switch (header->type) { + case VDAGENTD_MONITORS_CONFIG: +- vdagent_x11_set_monitor_config(agent->x11, (VDAgentMonitorsConfig *)data, 0); ++ vdagent_display_set_monitor_config(agent->display, (VDAgentMonitorsConfig *)data, 0); + break; + case VDAGENTD_CLIPBOARD_REQUEST: + vdagent_clipboard_request(agent->clipboards, header->arg1, header->arg2); +@@ -249,7 +248,7 @@ static void daemon_read_complete(UdscsConnection *conn, + } + break; + case VDAGENTD_GRAPHICS_DEVICE_INFO: +- vdagent_x11_handle_graphics_device_info(agent->x11, data, header->size); ++ vdagent_display_handle_graphics_device_info(agent->display, data, header->size); + break; + case VDAGENTD_CLIENT_DISCONNECTED: + vdagent_clipboards_release_all(agent->clipboards); +@@ -335,15 +334,7 @@ static int daemonize(void) + return 0; + } + +-static gboolean x11_io_channel_cb(GIOChannel *source, +- GIOCondition condition, +- gpointer data) +-{ +- VDAgent *agent = data; +- vdagent_x11_do_read(agent->x11); + +- return G_SOURCE_CONTINUE; +-} + + gboolean vdagent_signal_handler(gpointer user_data) + { +@@ -369,13 +360,12 @@ static VDAgent *vdagent_new(void) + static void vdagent_destroy(VDAgent *agent) + { + vdagent_finalize_file_xfer(agent); +- vdagent_x11_destroy(agent->x11, agent->conn == NULL); ++ vdagent_display_destroy(agent->display, agent->conn == NULL); + g_clear_pointer(&agent->conn, vdagent_connection_destroy); + + while (g_source_remove_by_user_data(agent)) + continue; + +- g_clear_pointer(&agent->x11_channel, g_io_channel_unref); + g_clear_pointer(&agent->loop, g_main_loop_unref); + g_free(agent); + } +@@ -393,22 +383,15 @@ static gboolean vdagent_init_async_cb(gpointer user_data) + } + g_object_set_data(G_OBJECT(agent->conn), "agent", agent); + +- agent->x11 = vdagent_x11_create(agent->conn, debug, x11_sync); +- if (agent->x11 == NULL) +- goto err_init; +- agent->x11_channel = g_io_channel_unix_new(vdagent_x11_get_fd(agent->x11)); +- if (agent->x11_channel == NULL) ++ agent->display = vdagent_display_create(agent->conn, debug, x11_sync); ++ if (agent->display == NULL) + goto err_init; + +- g_io_add_watch(agent->x11_channel, +- G_IO_IN, +- x11_io_channel_cb, +- agent); + + if (!vdagent_init_file_xfer(agent)) + syslog(LOG_WARNING, "File transfer is disabled"); + +- agent->clipboards = vdagent_clipboards_new(agent->x11); ++ agent->clipboards = vdagent_clipboards_new(vdagent_display_get_x11(agent->display)); + vdagent_clipboards_set_conn(agent->clipboards, agent->conn); + + if (parent_socket != -1) { +-- +2.29.2 + + +From aa75e9d7920ffb2290b56fd018789611748f1ea8 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Tue, 25 Feb 2020 15:18:41 +0100 +Subject: [PATCH 2/4] Move handle_graphics_device_info code to generic display + functions +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Most of the code in vdagent_x11_handle_graphics_device_info() is dealing +with SPICE information, only a few of it is X11-specific. +Move that code away from x11-randr.c so that we can use it with Wayland. + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + src/vdagent/display.c | 33 ++++++++++++++++- + src/vdagent/x11-randr.c | 82 +++++++++++++---------------------------- + src/vdagent/x11.h | 3 +- + 3 files changed, 60 insertions(+), 58 deletions(-) + +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index bb83762..51b0a0d 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -109,7 +109,38 @@ int vdagent_display_has_icons_on_desktop(VDAgentDisplay *display) + void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_t *data, + size_t size) + { +- vdagent_x11_handle_graphics_device_info(display->x11, data, size); ++ VDAgentGraphicsDeviceInfo *graphics_device_info = (VDAgentGraphicsDeviceInfo *)data; ++ VDAgentDeviceDisplayInfo *device_display_info = graphics_device_info->display_info; ++ ++ void *buffer_end = data + size; ++ ++ syslog(LOG_INFO, "Received Graphics Device Info:"); ++ ++ for (size_t i = 0; i < graphics_device_info->count; ++i) { ++ if ((void*) device_display_info > buffer_end || ++ (void*) (&device_display_info->device_address + ++ device_display_info->device_address_len) > buffer_end) { ++ syslog(LOG_ERR, "Malformed graphics_display_info message, " ++ "extends beyond the end of the buffer"); ++ break; ++ } ++ ++ // make sure the string is terminated: ++ if (device_display_info->device_address_len > 0) { ++ device_display_info->device_address[device_display_info->device_address_len - 1] = '\0'; ++ } else { ++ syslog(LOG_WARNING, "Zero length device_address received for channel_id: %u, monitor_id: %u", ++ device_display_info->channel_id, device_display_info->monitor_id); ++ } ++ ++ vdagent_x11_handle_device_display_info(display->x11, device_display_info); ++ ++ device_display_info = (VDAgentDeviceDisplayInfo*) ((char*) device_display_info + ++ sizeof(VDAgentDeviceDisplayInfo) + device_display_info->device_address_len); ++ } ++ ++ // make sure daemon is up-to-date with (possibly updated) device IDs ++ vdagent_x11_send_daemon_guest_xorg_res(display->x11, 1); + } + + /* +diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c +index 3fb7a68..9148563 100644 +--- a/src/vdagent/x11-randr.c ++++ b/src/vdagent/x11-randr.c +@@ -778,64 +778,34 @@ static void dump_monitors_config(struct vdagent_x11 *x11, + + // handle the device info message from the server. This will allow us to + // maintain a mapping from spice display id to xrandr output +-void vdagent_x11_handle_graphics_device_info(struct vdagent_x11 *x11, uint8_t *data, size_t size) ++void vdagent_x11_handle_device_display_info(struct vdagent_x11 *x11, ++ VDAgentDeviceDisplayInfo *device_display_info) + { +- VDAgentGraphicsDeviceInfo *graphics_device_info = (VDAgentGraphicsDeviceInfo *)data; +- VDAgentDeviceDisplayInfo *device_display_info = graphics_device_info->display_info; +- +- void *buffer_end = data + size; +- +- syslog(LOG_INFO, "Received Graphics Device Info:"); +- +- for (size_t i = 0; i < graphics_device_info->count; ++i) { +- if ((void*) device_display_info > buffer_end || +- (void*) (&device_display_info->device_address + +- device_display_info->device_address_len) > buffer_end) { +- syslog(LOG_ERR, "Malformed graphics_display_info message, " +- "extends beyond the end of the buffer"); +- break; +- } +- +- // make sure the string is terminated: +- if (device_display_info->device_address_len > 0) { +- device_display_info->device_address[device_display_info->device_address_len - 1] = '\0'; +- } else { +- syslog(LOG_WARNING, "Zero length device_address received for channel_id: %u, monitor_id: %u", +- device_display_info->channel_id, device_display_info->monitor_id); +- } +- +- RROutput x_output; +- if (lookup_xrandr_output_for_device_info(device_display_info, x11->display, +- x11->randr.res, &x_output)) { +- gint64 *value = g_new(gint64, 1); +- *value = x_output; +- +- syslog(LOG_INFO, "Adding graphics device info: channel_id: %u monitor_id: " +- "%u device_address: %s, device_display_id: %u xrandr output ID: %lu", +- device_display_info->channel_id, +- device_display_info->monitor_id, +- device_display_info->device_address, +- device_display_info->device_display_id, +- x_output); +- +- g_hash_table_insert(x11->guest_output_map, +- GUINT_TO_POINTER(device_display_info->channel_id + device_display_info->monitor_id), +- value); +- } else { +- syslog(LOG_INFO, "channel_id: %u monitor_id: %u device_address: %s, " +- "device_display_id: %u xrandr output ID NOT FOUND", +- device_display_info->channel_id, +- device_display_info->monitor_id, +- device_display_info->device_address, +- device_display_info->device_display_id); +- } +- +- device_display_info = (VDAgentDeviceDisplayInfo*) ((char*) device_display_info + +- sizeof(VDAgentDeviceDisplayInfo) + device_display_info->device_address_len); ++ RROutput x_output; ++ if (lookup_xrandr_output_for_device_info(device_display_info, x11->display, ++ x11->randr.res, &x_output)) { ++ gint64 *value = g_new(gint64, 1); ++ *value = x_output; ++ ++ syslog(LOG_INFO, "Adding graphics device info: channel_id: %u monitor_id: " ++ "%u device_address: %s, device_display_id: %u xrandr output ID: %lu", ++ device_display_info->channel_id, ++ device_display_info->monitor_id, ++ device_display_info->device_address, ++ device_display_info->device_display_id, ++ x_output); ++ ++ g_hash_table_insert(x11->guest_output_map, ++ GUINT_TO_POINTER(device_display_info->channel_id + device_display_info->monitor_id), ++ value); ++ } else { ++ syslog(LOG_INFO, "channel_id: %u monitor_id: %u device_address: %s, " ++ "device_display_id: %u xrandr output ID NOT FOUND", ++ device_display_info->channel_id, ++ device_display_info->monitor_id, ++ device_display_info->device_address, ++ device_display_info->device_display_id); + } +- +- // make sure daemon is up-to-date with (possibly updated) device IDs +- vdagent_x11_send_daemon_guest_xorg_res(x11, 1); + } + + static int get_output_index_for_display_id(struct vdagent_x11 *x11, int display_id) +diff --git a/src/vdagent/x11.h b/src/vdagent/x11.h +index c49a397..6b3bcc1 100644 +--- a/src/vdagent/x11.h ++++ b/src/vdagent/x11.h +@@ -51,6 +51,7 @@ void vdagent_x11_client_disconnected(struct vdagent_x11 *x11); + #endif + + int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11); +-void vdagent_x11_handle_graphics_device_info(struct vdagent_x11 *x11, uint8_t *data, size_t size); ++void vdagent_x11_handle_device_display_info(struct vdagent_x11 *x11, ++ VDAgentDeviceDisplayInfo *device_display_info); + + #endif +-- +2.29.2 + + +From 1601f4de9533dfe35536f68afc9ca899cf578ae3 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Mon, 6 Jul 2020 09:24:10 +0200 +Subject: [PATCH 3/4] Move more code from x11.c to display.c +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Moving the non-X11 part of the code related to +"vdagent_x11_has_icon_on_desktop()". +Turns out most of it is disabled anyway when we build with GTK. + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + src/vdagent/display.c | 60 ++++++++++++++++++++++++++++++++++++++----- + src/vdagent/display.h | 2 +- + src/vdagent/x11.c | 53 +++----------------------------------- + src/vdagent/x11.h | 5 +++- + 4 files changed, 62 insertions(+), 58 deletions(-) + +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index 51b0a0d..5f2ba2c 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -48,6 +48,21 @@ struct VDAgentDisplay { + GIOChannel *x11_channel; + }; + ++ ++static gchar *vdagent_display_get_wm_name(VDAgentDisplay *display) ++{ ++#ifdef GDK_WINDOWING_X11 ++ GdkDisplay *gdk_display = gdk_display_get_default(); ++ if (GDK_IS_X11_DISPLAY(gdk_display)) ++ return g_strdup(gdk_x11_screen_get_window_manager_name( ++ gdk_display_get_default_screen(gdk_display))); ++ return g_strdup("unsupported"); ++#else ++ return vdagent_x11_get_wm_name(display->x11); ++#endif ++} ++ ++ + struct vdagent_x11* vdagent_display_get_x11(VDAgentDisplay *display) + { + return display->x11; +@@ -64,6 +79,7 @@ static gboolean x11_io_channel_cb(GIOChannel *source, GIOCondition condition, gp + VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int sync) + { + VDAgentDisplay *display; ++ gchar *net_wm_name = NULL; + + display = g_new0(VDAgentDisplay, 1); + display->x11 = vdagent_x11_create(vdagentd, debug, sync); +@@ -81,6 +97,21 @@ VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int + + g_io_add_watch(display->x11_channel, G_IO_IN, x11_io_channel_cb, display); + ++ ++ /* Since we are started at the same time as the wm, ++ sometimes we need to wait a bit for the _NET_WM_NAME to show up. */ ++ for (int i = 0; i < 9; i++) { ++ g_free(net_wm_name); ++ net_wm_name = vdagent_display_get_wm_name(display); ++ if (strcmp(net_wm_name, "unknown")) ++ break; ++ usleep(100000); ++ } ++ if (display->x11->debug) ++ syslog(LOG_DEBUG, "%s: net_wm_name=\"%s\", has icons=%d", ++ __func__, net_wm_name, vdagent_display_has_icons_on_desktop(display)); ++ g_free(net_wm_name); ++ + return display; + } + +@@ -95,13 +126,30 @@ void vdagent_display_destroy(VDAgentDisplay *display, int vdagentd_disconnected) + } + + /* Function used to determine the default location to save file-xfers, +- xdg desktop dir or xdg download dir. We err on the safe side and use a +- whitelist approach, so any unknown desktop will end up with saving +- file-xfers to the xdg download dir, and opening the xdg download dir with +- xdg-open when the file-xfer completes. */ +-int vdagent_display_has_icons_on_desktop(VDAgentDisplay *display) ++ xdg desktop dir or xdg download dir. We err on the safe side and use a ++ whitelist approach, so any unknown desktop will end up with saving ++ file-xfers to the xdg download dir, and opening the xdg download dir with ++ xdg-open when the file-xfer completes. */ ++gboolean vdagent_display_has_icons_on_desktop(VDAgentDisplay *display) + { +- return vdagent_x11_has_icons_on_desktop(display->x11); ++ static const char * const wms_with_icons_on_desktop[] = { ++ "Metacity", /* GNOME-2 or GNOME-3 fallback */ ++ "Xfwm4", /* Xfce */ ++ "Marco", /* Mate */ ++ "Metacity (Marco)", /* Mate, newer */ ++ NULL ++ }; ++ gchar *net_wm_name = vdagent_display_get_wm_name(display); ++ int i; ++ ++ for (i = 0; wms_with_icons_on_desktop[i]; i++) ++ if (!strcmp(net_wm_name, wms_with_icons_on_desktop[i])) { ++ g_free(net_wm_name); ++ return TRUE; ++ } ++ ++ g_free(net_wm_name); ++ return FALSE; + } + + // handle the device info message from the server. This will allow us to +diff --git a/src/vdagent/display.h b/src/vdagent/display.h +index 1f5365f..af165ef 100644 +--- a/src/vdagent/display.h ++++ b/src/vdagent/display.h +@@ -25,7 +25,7 @@ typedef struct VDAgentDisplay VDAgentDisplay; + VDAgentDisplay *vdagent_display_create(UdscsConnection *vdagentd, int debug, int sync); + void vdagent_display_destroy(VDAgentDisplay *display, int vdagentd_disconnected); + +-int vdagent_display_has_icons_on_desktop(VDAgentDisplay *display); ++gboolean vdagent_display_has_icons_on_desktop(VDAgentDisplay *display); + void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_t *data, size_t size); + void vdagent_display_set_monitor_config(VDAgentDisplay *display, + VDAgentMonitorsConfig *mon_config, +diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c +index 550d097..d171ffd 100644 +--- a/src/vdagent/x11.c ++++ b/src/vdagent/x11.c +@@ -123,15 +123,8 @@ int vdagent_x11_restore_error_handler(struct vdagent_x11 *x11) + return error; + } + +-static gchar *vdagent_x11_get_wm_name(struct vdagent_x11 *x11) ++gchar *vdagent_x11_get_wm_name(struct vdagent_x11 *x11) + { +-#ifdef GDK_WINDOWING_X11 +- GdkDisplay *display = gdk_display_get_default(); +- if (GDK_IS_X11_DISPLAY(display)) +- return g_strdup(gdk_x11_screen_get_window_manager_name( +- gdk_display_get_default_screen(display))); +- return g_strdup("unsupported"); +-#else + Atom type_ret; + int format_ret; + unsigned long len, remain; +@@ -193,8 +186,7 @@ static gchar *vdagent_x11_get_wm_name(struct vdagent_x11 *x11) + if (net_wm_name == NULL) + return g_strdup("unknown"); + return net_wm_name; +-#endif + } + + struct vdagent_x11 *vdagent_x11_create(UdscsConnection *vdagentd, + int debug, int sync) +@@ -206,7 +198,6 @@ struct vdagent_x11 *vdagent_x11_create(UdscsConnection *vdagentd, + #else + int i, j, major, minor; + #endif +- gchar *net_wm_name = NULL; + + x11 = g_new0(struct vdagent_x11, 1); + x11->vdagentd = vdagentd; +@@ -308,19 +299,6 @@ struct vdagent_x11 *vdagent_x11_create(UdscsConnection *vdagentd, + } + vdagent_x11_send_daemon_guest_xorg_res(x11, 1); + +- /* Since we are started at the same time as the wm, +- sometimes we need to wait a bit for the _NET_WM_NAME to show up. */ +- for (i = 0; i < 9; i++) { +- g_free(net_wm_name); +- net_wm_name = vdagent_x11_get_wm_name(x11); +- if (strcmp(net_wm_name, "unknown")) +- break; +- usleep(100000); +- } +- if (x11->debug) +- syslog(LOG_DEBUG, "%s: net_wm_name=\"%s\", has icons=%d", +- __func__, net_wm_name, vdagent_x11_has_icons_on_desktop(x11)); +- g_free(net_wm_name); + + /* Flush output buffers and consume any pending events */ + vdagent_x11_do_read(x11); +@@ -1407,30 +1365,3 @@ void vdagent_x11_client_disconnected(struct vdagent_x11 *x11) + } + } + #endif +- +-/* Function used to determine the default location to save file-xfers, +- xdg desktop dir or xdg download dir. We err on the safe side and use a +- whitelist approach, so any unknown desktop will end up with saving +- file-xfers to the xdg download dir, and opening the xdg download dir with +- xdg-open when the file-xfer completes. */ +-int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11) +-{ +- const char * const wms_with_icons_on_desktop[] = { +- "Metacity", /* GNOME-2 or GNOME-3 fallback */ +- "Xfwm4", /* Xfce */ +- "Marco", /* Mate */ +- "Metacity (Marco)", /* Mate, newer */ +- NULL +- }; +- gchar *net_wm_name = vdagent_x11_get_wm_name(x11); +- int i; +- +- for (i = 0; wms_with_icons_on_desktop[i]; i++) +- if (!strcmp(net_wm_name, wms_with_icons_on_desktop[i])) { +- g_free(net_wm_name); +- return 1; +- } +- +- g_free(net_wm_name); +- return 0; +-} +diff --git a/src/vdagent/x11.h b/src/vdagent/x11.h +index 6b3bcc1..6afee7f 100644 +--- a/src/vdagent/x11.h ++++ b/src/vdagent/x11.h +@@ -50,7 +50,8 @@ void vdagent_x11_clipboard_release(struct vdagent_x11 *x11, uint8_t selection); + void vdagent_x11_client_disconnected(struct vdagent_x11 *x11); + #endif + +-int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11); ++gchar *vdagent_x11_get_wm_name(struct vdagent_x11 *x11); ++ + void vdagent_x11_handle_device_display_info(struct vdagent_x11 *x11, + VDAgentDeviceDisplayInfo *device_display_info); + +-- +2.29.2 + + +From ef54256abff5fabb3158bff34c96781d09e25f45 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Mon, 6 Jul 2020 15:09:00 +0200 +Subject: [PATCH 4/4] Use RROutput directly in guest_output_map. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +No need to allocate a separate variable - the RROutput value can be used +directly in the hashtable. + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + src/vdagent/x11-randr.c | 14 +++++--------- + src/vdagent/x11.c | 2 +- + 2 files changed, 6 insertions(+), 10 deletions(-) + +diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c +index 9148563..6cd9012 100644 +--- a/src/vdagent/x11-randr.c ++++ b/src/vdagent/x11-randr.c +@@ -414,7 +414,7 @@ static RROutput get_xrandr_output_for_display_id(struct vdagent_x11 *x11, int di + gpointer value; + if (g_hash_table_lookup_extended(x11->guest_output_map, GINT_TO_POINTER(display_id), + NULL, &value)) { +- return *(gint64*)value; ++ return (RROutput)value; + } + } + +@@ -784,9 +784,6 @@ void vdagent_x11_handle_device_display_info(struct vdagent_x11 *x11, + RROutput x_output; + if (lookup_xrandr_output_for_device_info(device_display_info, x11->display, + x11->randr.res, &x_output)) { +- gint64 *value = g_new(gint64, 1); +- *value = x_output; +- + syslog(LOG_INFO, "Adding graphics device info: channel_id: %u monitor_id: " + "%u device_address: %s, device_display_id: %u xrandr output ID: %lu", + device_display_info->channel_id, +@@ -797,7 +794,7 @@ void vdagent_x11_handle_device_display_info(struct vdagent_x11 *x11, + + g_hash_table_insert(x11->guest_output_map, + GUINT_TO_POINTER(device_display_info->channel_id + device_display_info->monitor_id), +- value); ++ (gpointer)x_output); + } else { + syslog(LOG_INFO, "channel_id: %u monitor_id: %u device_address: %s, " + "device_display_id: %u xrandr output ID NOT FOUND", +@@ -1058,12 +1055,11 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update) + // all down. + RROutput output_id = x11->randr.res->outputs[i]; + GHashTableIter iter; +- gpointer key, value; ++ gpointer key, other_id; + g_hash_table_iter_init(&iter, x11->guest_output_map); + bool found = false; +- while (g_hash_table_iter_next(&iter, &key, &value)) { +- gint64 *other_id = value; +- if (*other_id == output_id) { ++ while (g_hash_table_iter_next(&iter, &key, &other_id)) { ++ if ((RROutput)other_id == output_id) { + curr.display_id = GPOINTER_TO_INT(key); + g_array_append_val(res_array, curr); + found = true; +diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c +index d171ffd..ee0533f 100644 +--- a/src/vdagent/x11.c ++++ b/src/vdagent/x11.c +@@ -208,7 +206,7 @@ struct vdagent_x11 *vdagent_x11_create(UdscsConnection *vdagentd, + x11->guest_output_map = g_hash_table_new_full(&g_direct_hash, + &g_direct_equal, + NULL, +- &g_free); ++ NULL); + + + x11->display = XOpenDisplay(NULL); +-- +2.29.2 + +From 6e5e6c2ca6dde2fa489344e497c46fcdec364012 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Tue, 24 Mar 2020 18:13:15 +0100 +Subject: [PATCH 1/7] Add a non-X11 function to send the guest resolution to + the daemon/server. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The purpose here is to further separate X11-specific code, and have the +actual sending to the daemon to be out of X11 code, and reusable by +other APIs. + +- Replace vdagent_x11_send_daemon_guest_res() with a function that +builds + and returns the list of monitor resolutions. +- Create the vdagent_display_send_daemon_guest_res() that uses this to + send the resolutions to the daemon. +- Call this function in place of vdagent_x11_send_daemon_guest_res() + +In later commits, we will be able to use a different API to generate +the list of monitor resolution, without having to change the X11 code. + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + src/vdagent/display.c | 37 +++++++++++++++++++- + src/vdagent/display.h | 2 ++ + src/vdagent/x11-priv.h | 7 ++-- + src/vdagent/x11-randr.c | 75 ++++++++++++++++++++++++----------------- + src/vdagent/x11.c | 2 -- + 5 files changed, 88 insertions(+), 35 deletions(-) + +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index d34a6e5..f84b1b3 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -35,6 +32,9 @@ + #include "x11.h" + #include "x11-priv.h" + ++#include "device-info.h" ++#include "vdagentd-proto.h" ++ + #include "display.h" + + /** +@@ -51,6 +51,35 @@ struct VDAgentDisplay { + GIOChannel *x11_channel; + }; + ++void vdagent_display_send_daemon_guest_res(VDAgentDisplay *display, gboolean update) ++{ ++ GArray *res_array; ++ int width, height, screen_count; ++ ++ res_array = vdagent_x11_get_resolutions(display->x11, update, &width, &height, &screen_count); ++ if (res_array == NULL) { ++ return; ++ } ++ ++ if (display->x11->debug) { ++ syslog(LOG_DEBUG, "Sending guest screen resolutions to vdagentd:"); ++ if (res_array->len > screen_count) { ++ syslog(LOG_DEBUG, "(NOTE: list may contain overlapping areas when " ++ "multiple spice displays show the same guest output)"); ++ } ++ struct vdagentd_guest_xorg_resolution *res = ++ (struct vdagentd_guest_xorg_resolution*)res_array->data; ++ for (int i = 0; i < res_array->len; i++) { ++ syslog(LOG_DEBUG, " display_id=%d - %dx%d%+d%+d", ++ res[i].display_id, res[i].width, res[i].height, res[i].x, res[i].y); ++ } ++ } ++ ++ udscs_write(display->x11->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height, ++ (uint8_t *)res_array->data, ++ res_array->len * sizeof(struct vdagentd_guest_xorg_resolution)); ++ g_array_free(res_array, TRUE); ++} + + static gchar *vdagent_display_get_wm_name(VDAgentDisplay *display) + { +@@ -94,6 +120,8 @@ VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int + return NULL; + } + ++ display->x11->vdagent_display = display; ++ + display->x11_channel = g_io_channel_unix_new(vdagent_x11_get_fd(display->x11)); + if (display->x11_channel == NULL) { + vdagent_x11_destroy(display->x11, TRUE); +@@ -118,6 +146,7 @@ VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int + __func__, net_wm_name, vdagent_display_has_icons_on_desktop(display)); + g_free(net_wm_name); + ++ vdagent_display_send_daemon_guest_res(display, TRUE); + return display; + } + +@@ -194,7 +223,7 @@ void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_ + } + + // make sure daemon is up-to-date with (possibly updated) device IDs +- vdagent_x11_send_daemon_guest_xorg_res(display->x11, 1); ++ vdagent_display_send_daemon_guest_res(display, TRUE); + } + + /* +diff --git a/src/vdagent/display.h b/src/vdagent/display.h +index af165ef..5858237 100644 +--- a/src/vdagent/display.h ++++ b/src/vdagent/display.h +@@ -33,5 +33,7 @@ void vdagent_display_set_monitor_config(VDAgentDisplay *display, + + struct vdagent_x11 *vdagent_display_get_x11(VDAgentDisplay *display); + ++void vdagent_display_send_daemon_guest_res(VDAgentDisplay *display, gboolean update); ++ + + #endif /* SRC_VDAGENT_DISPLAY_H_ */ +diff --git a/src/vdagent/x11-priv.h b/src/vdagent/x11-priv.h +index f6f7efe..805d124 100644 +--- a/src/vdagent/x11-priv.h ++++ b/src/vdagent/x11-priv.h +@@ -7,5 +7,6 @@ + #include + + #include ++#include "display.h" + + #ifndef WITH_GTK +@@ -155,14 +142,16 @@ struct vdagent_x11 { + int has_xinerama; + int dont_send_guest_xorg_res; + GHashTable *guest_output_map; ++ ++ VDAgentDisplay *vdagent_display; + }; + + extern int (*vdagent_x11_prev_error_handler)(Display *, XErrorEvent *); + extern int vdagent_x11_caught_error; + + void vdagent_x11_randr_init(struct vdagent_x11 *x11); +-void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, +- int update); ++GArray *vdagent_x11_get_resolutions(struct vdagent_x11 *x11, gboolean update, ++ int *width, int *height, int *system_screen_count); + void vdagent_x11_randr_handle_root_size_change(struct vdagent_x11 *x11, + int screen, int width, int height); + int vdagent_x11_randr_handle_event(struct vdagent_x11 *x11, +diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c +index b688b55..145e75a 100644 +--- a/src/vdagent/x11-randr.c ++++ b/src/vdagent/x11-randr.c +@@ -524,7 +524,7 @@ void vdagent_x11_randr_handle_root_size_change(struct vdagent_x11 *x11, + x11->width[screen] = width; + x11->height[screen] = height; + if (!x11->dont_send_guest_xorg_res) { +- vdagent_x11_send_daemon_guest_xorg_res(x11, 1); ++ vdagent_display_send_daemon_guest_res(x11->vdagent_display, TRUE); + } + } + +@@ -544,7 +544,7 @@ int vdagent_x11_randr_handle_event(struct vdagent_x11 *x11, + case RRNotify: { + update_randr_res(x11, 0); + if (!x11->dont_send_guest_xorg_res) +- vdagent_x11_send_daemon_guest_xorg_res(x11, 1); ++ vdagent_display_send_daemon_guest_res(x11->vdagent_display, TRUE); + break; + } + default: +@@ -1021,18 +1021,45 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11, + x11->dont_send_guest_xorg_res = 0; + + exit: +- vdagent_x11_send_daemon_guest_xorg_res(x11, 0); ++ vdagent_display_send_daemon_guest_res(x11->vdagent_display, FALSE); + + /* Flush output buffers and consume any pending events */ + vdagent_x11_do_read(x11); + g_free(curr); + } + +-void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update) ++/** ++ * Builds a list of available monitors, associated with their resolutions. ++ * ++ * This function will use the x11->guest_output_map hashtable to map the SPICE display ID to the ++ * XRandr display ID, and make sure we get the right resolution for each SPICE display. ++ * See vdagent_x11_handle_device_display_info() for how this hashtable is populated. ++ * ++ * If the ID can't be matched (which happens when running under XWayland for instance), a default ++ * mapping is made, which assumes the list of X displays are in the same order as the SPICE displays. ++ * This will cause issues if some SPICE displays are not enabled (for instance: display 2 is ++ * enabled, but not display 1, causing a mismatch of IDs). ++ * ++ * Parameters: ++ * x11 - pointer to the X11 structure ++ * update - TRUE if the cached resolutions need to be updated before building the list ++ * width - full width of the desktop area (containing all the displays) ++ * height - full height of the desktop area (containing all the displays) ++ * system_screen_count - number of displays found on the system. In some situations, this may be ++ * different than the number of SPICE displays (for instance: multiple displays ++ * overlapping the same area of the desktop). ++ * Used for logging/troubleshooting purposes. ++ * ++ * Returns: a GArray of vdagentd_guest_xorg_resolution elements, to be sent to the vdagent daemon. ++ * NULL if an error occurs. ++ */ ++GArray *vdagent_x11_get_resolutions(struct vdagent_x11 *x11, gboolean update, ++ int *width, int *height, int *system_screen_count) + { + GArray *res_array = g_array_new(FALSE, FALSE, sizeof(struct vdagentd_guest_xorg_resolution)); +- int i, width = 0, height = 0, screen_count = 0; ++ int i, screen_count = 0; + ++ *width = *height = 0; + if (x11->has_xrandr) { + if (update) + update_randr_res(x11, 0); +@@ -1070,8 +1097,8 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update) + } + } + } +- width = x11->width[0]; +- height = x11->height[0]; ++ *width = x11->width[0]; ++ *height = x11->height[0]; + } else if (x11->has_xinerama) { + XineramaScreenInfo *screen_info = NULL; + +@@ -1085,7 +1112,7 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update) + screen_info[i].screen_number, screen_count); + XFree(screen_info); + g_array_free(res_array, true); +- return; ++ return NULL; + } + struct vdagentd_guest_xorg_resolution *curr = &g_array_index(res_array, + struct vdagentd_guest_xorg_resolution, +@@ -1096,8 +1123,8 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update) + curr->y = screen_info[i].y_org; + } + XFree(screen_info); +- width = x11->width[0]; +- height = x11->height[0]; ++ *width = x11->width[0]; ++ *height = x11->height[0]; + } else { + no_info: + for (i = 0; i < screen_count; i++) { +@@ -1105,11 +1132,12 @@ no_info: + res.width = x11->width[i]; + res.height = x11->height[i]; + /* No way to get screen coordinates, assume rtl order */ +- res.x = width; ++ res.x = *width; + res.y = 0; +- width += x11->width[i]; +- if (x11->height[i] > height) +- height = x11->height[i]; ++ *width += x11->width[i]; ++ if (x11->height[i] > *height) { ++ *height = x11->height[i]; ++ } + g_array_append_val(res_array, res); + } + } +@@ -1117,22 +1145,9 @@ no_info: + if (screen_count == 0) { + syslog(LOG_DEBUG, "Screen count is zero, are we on wayland?"); + g_array_free(res_array, TRUE); +- return; +- } +- +- if (x11->debug) { +- syslog(LOG_DEBUG, "Sending guest screen resolutions to vdagentd:"); +- if (res_array->len > screen_count) { +- syslog(LOG_DEBUG, "(NOTE: list may contain overlapping areas when multiple spice displays show the same guest output)"); +- } +- for (i = 0; i < res_array->len; i++) { +- struct vdagentd_guest_xorg_resolution *res = (struct vdagentd_guest_xorg_resolution*)res_array->data; +- syslog(LOG_DEBUG, " screen %d %dx%d%+d%+d, display_id=%d", i, +- res[i].width, res[i].height, res[i].x, res[i].y, res[i].display_id); +- } ++ return NULL; + } + +- udscs_write(x11->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height, +- (uint8_t *)res_array->data, res_array->len * sizeof(struct vdagentd_guest_xorg_resolution)); +- g_array_free(res_array, TRUE); ++ *system_screen_count = screen_count; ++ return res_array; + } +diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c +index 58b99b7..b867a5a 100644 +--- a/src/vdagent/x11.c ++++ b/src/vdagent/x11.c +@@ -301,8 +297,6 @@ struct vdagent_x11 *vdagent_x11_create(UdscsConnection *vdagentd, + x11->width[i] = attrib.width; + x11->height[i] = attrib.height; + } +- vdagent_x11_send_daemon_guest_xorg_res(x11, 1); +- + + /* Flush output buffers and consume any pending events */ + vdagent_x11_do_read(x11); +-- +2.29.2 + + +From f59192a021ffe5a59eba850ccef9c389681579e5 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Mon, 24 Aug 2020 11:08:43 +0200 +Subject: [PATCH 2/7] Copy the 'debug' flag and 'vdagentd' socket into the + VDAgentDisplay structure. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + src/vdagent/display.c | 11 ++++++++--- + 1 file changed, 8 insertions(+), 3 deletions(-) + +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index f84b1b3..2577f9c 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -51,6 +48,8 @@ + */ + struct VDAgentDisplay { + struct vdagent_x11 *x11; ++ UdscsConnection *vdagentd; ++ int debug; + GIOChannel *x11_channel; + }; + +@@ -64,7 +63,7 @@ void vdagent_display_send_daemon_guest_res(VDAgentDisplay *display, gboolean upd + return; + } + +- if (display->x11->debug) { ++ if (display->debug) { + syslog(LOG_DEBUG, "Sending guest screen resolutions to vdagentd:"); + if (res_array->len > screen_count) { + syslog(LOG_DEBUG, "(NOTE: list may contain overlapping areas when " +@@ -78,7 +77,7 @@ void vdagent_display_send_daemon_guest_res(VDAgentDisplay *display, gboolean upd + } + } + +- udscs_write(display->x11->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height, ++ udscs_write(display->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height, + (uint8_t *)res_array->data, + res_array->len * sizeof(struct vdagentd_guest_xorg_resolution)); + g_array_free(res_array, TRUE); +@@ -120,6 +116,9 @@ VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int + gchar *net_wm_name = NULL; + + display = g_new0(VDAgentDisplay, 1); ++ display->vdagentd = vdagentd; ++ display->debug = debug; ++ + display->x11 = vdagent_x11_create(vdagentd, debug, sync); + if (display->x11 == NULL) { + g_free(display); +@@ -147,7 +146,7 @@ VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int + break; + usleep(100000); + } +- if (display->x11->debug) ++ if (display->debug) + syslog(LOG_DEBUG, "%s: net_wm_name=\"%s\", has icons=%d", + __func__, net_wm_name, vdagent_display_has_icons_on_desktop(display)); + g_free(net_wm_name); +-- +2.29.2 + + +From d92f37ae06aafa77b8bc27a458c449c28d0d5f09 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Tue, 24 Mar 2020 16:42:16 +0100 +Subject: [PATCH 3/7] Prepare mapping based on connector name. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Split the lookup_xrandr_output_for_device_info to access the part +retrieving the expected connector name. +Call that separate function in the "non-X11" part of the code, and use a +hashtable to map Spice display ID to its expected connector name. +This hashtable will later be used when we need to report resolution +changes to the daemon/server. + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + src/vdagent/device-info.c | 167 +++++++++++++++++++++++--------------- + src/vdagent/device-info.h | 4 + + src/vdagent/display.c | 37 ++++++++- + 3 files changed, 139 insertions(+), 69 deletions(-) + +diff --git a/src/vdagent/device-info.c b/src/vdagent/device-info.c +index 6b0e28f..8cf72c9 100644 +--- a/src/vdagent/device-info.c ++++ b/src/vdagent/device-info.c +@@ -388,12 +388,14 @@ static char* find_device_at_pci_address(PciAddress *pci_addr, int *vendor_id, in + return NULL; + } + +-// PCI address should be in the following format: +-// pci/$domain/$slot.$fn/$slot.$fn +-bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, +- Display *xdisplay, +- XRRScreenResources *xres, +- RROutput *output_id) ++ ++/** ++ * Look up DRM info for the device, and retrieve the expected connector name. ++ * This name will later be compared to the monitor names found at the display manager level. ++ */ ++int get_connector_name_for_device_info(VDAgentDeviceDisplayInfo *device_info, ++ char *expected_name, size_t name_size, ++ bool has_virtual_zero_display) + { + PciAddress *user_pci_addr = parse_pci_address_from_spice((char*)device_info->device_address); + if (!user_pci_addr) { +@@ -401,7 +403,7 @@ bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, + "Couldn't parse PCI address '%s'. " + "Address should be the form 'pci/$domain/$slot.$fn/$slot.fn...", + device_info->device_address); +- return false; ++ return -1; + } + + int vendor_id = 0; +@@ -412,68 +414,106 @@ bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, + int drm_fd = open(dev_path, O_RDWR); + if (drm_fd < 0) { + syslog(LOG_WARNING, "Unable to open file %s", dev_path); +- return false; ++ g_free(dev_path); ++ return -1; + } + + drmModeResPtr res = drmModeGetResources(drm_fd); +- if (res) { +- // find the drm output that is equal to device_display_id +- if (device_info->device_display_id >= res->count_connectors) { +- syslog(LOG_WARNING, +- "Specified display id %i is higher than the maximum display id " +- "provided by this device (%i)", +- device_info->device_display_id, res->count_connectors - 1); +- close(drm_fd); +- return false; +- } ++ if (res == NULL) { ++ syslog(LOG_WARNING, ++ "Unable to get DRM resources for card %s. " ++ "Falling back to using xrandr output index.", ++ dev_path); ++ close(drm_fd); ++ g_free(dev_path); ++ return 1; // error out - actual handling is deferred to the caller ++ } + +- drmModeConnectorPtr conn = +- drmModeGetConnector(drm_fd, res->connectors[device_info->device_display_id]); +- drmModeFreeResources(res); +- res = NULL; ++ // no need for dev_path anymore ++ g_free(dev_path); ++ ++ // find the drm output that is equal to device_display_id ++ if (device_info->device_display_id >= res->count_connectors) { ++ syslog(LOG_WARNING, ++ "Specified display id %i is higher than the maximum display id " ++ "provided by this device (%i)", ++ device_info->device_display_id, res->count_connectors - 1); + close(drm_fd); ++ return -1; ++ } + +- if (conn == NULL) { +- syslog(LOG_WARNING, "Unable to get drm connector for display id %i", +- device_info->device_display_id); +- return false; +- } ++ drmModeConnectorPtr conn = ++ drmModeGetConnector(drm_fd, res->connectors[device_info->device_display_id]); ++ drmModeFreeResources(res); ++ res = NULL; ++ close(drm_fd); + +- bool decrement_name = false; +- if (vendor_id == PCI_VENDOR_ID_REDHAT && device_id == PCI_DEVICE_ID_QXL) { +- // Older QXL drivers numbered their outputs starting with +- // 0. This contrasts with most drivers who start numbering +- // outputs with 1. In this case, the expected drm connector +- // name will need to be decremented before comparing to the +- // xrandr output name +- for (int i = 0; i < xres->noutput; ++i) { +- XRROutputInfo *oinfo = XRRGetOutputInfo(xdisplay, xres, xres->outputs[i]); +- if (!oinfo) { +- syslog(LOG_WARNING, "Unable to lookup XRandr output info for output %li", +- xres->outputs[i]); +- return false; +- } +- if (strcmp(oinfo->name, "Virtual-0") == 0) { +- decrement_name = true; +- XRRFreeOutputInfo(oinfo); +- break; +- } +- XRRFreeOutputInfo(oinfo); +- } ++ if (conn == NULL) { ++ syslog(LOG_WARNING, "Unable to get drm connector for display id %i", ++ device_info->device_display_id); ++ return -1; ++ } ++ ++ bool decrement_name = false; ++ ++ if (vendor_id == PCI_VENDOR_ID_REDHAT && device_id == PCI_DEVICE_ID_QXL ++ && has_virtual_zero_display) { ++ decrement_name = true; ++ } ++ ++ // Compare the name of the xrandr output against what we would ++ // expect based on the drm connection type. The xrandr names ++ // are driver-specific, so we need to special-case some ++ // drivers. Most hardware these days uses the 'modesetting' ++ // driver, but the QXL device uses its own driver which has ++ // different naming conventions ++ if (vendor_id == PCI_VENDOR_ID_REDHAT && device_id == PCI_DEVICE_ID_QXL) { ++ drm_conn_name_qxl(conn, expected_name, name_size, decrement_name); ++ } else { ++ drm_conn_name_modesetting(conn, expected_name, name_size); ++ } ++ drmModeFreeConnector(conn); ++ ++ return 0; ++} ++ ++// PCI address should be in the following format: ++// pci/$domain/$slot.$fn/$slot.$fn ++bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, ++ Display *xdisplay, ++ XRRScreenResources *xres, ++ RROutput *output_id) ++{ ++ char expected_name[100]; ++ int ret; ++ ++ // Older QXL drivers numbered their outputs starting with ++ // 0. This contrasts with most drivers who start numbering ++ // outputs with 1. In this case, the expected drm connector ++ // name will need to be decremented before comparing to the ++ // xrandr output name ++ bool has_virtual_zero_display = false; ++ for (int i = 0; i < xres->noutput; ++i) { ++ XRROutputInfo *oinfo = XRRGetOutputInfo(xdisplay, xres, xres->outputs[i]); ++ if (!oinfo) { ++ syslog(LOG_WARNING, "Unable to lookup XRandr output info for output %li", ++ xres->outputs[i]); ++ return false; + } +- // Compare the name of the xrandr output against what we would +- // expect based on the drm connection type. The xrandr names +- // are driver-specific, so we need to special-case some +- // drivers. Most hardware these days uses the 'modesetting' +- // driver, but the QXL device uses its own driver which has +- // different naming conventions +- char expected_name[100]; +- if (vendor_id == PCI_VENDOR_ID_REDHAT && device_id == PCI_DEVICE_ID_QXL) { +- drm_conn_name_qxl(conn, expected_name, sizeof(expected_name), decrement_name); +- } else { +- drm_conn_name_modesetting(conn, expected_name, sizeof(expected_name)); ++ if (strcmp(oinfo->name, "Virtual-0") == 0) { ++ has_virtual_zero_display = true; ++ XRRFreeOutputInfo(oinfo); ++ break; + } ++ XRRFreeOutputInfo(oinfo); ++ } + ++ ret = get_connector_name_for_device_info(device_info, expected_name, sizeof(expected_name), ++ has_virtual_zero_display); ++ switch (ret) { ++ case -1: // generic error => exit ++ return false; ++ case 0: + // Loop through xrandr outputs and check whether the xrandr + // output name matches the drm connector name + for (int i = 0; i < xres->noutput; ++i) { +@@ -493,13 +533,8 @@ bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, + } + XRRFreeOutputInfo(oinfo); + } +- drmModeFreeConnector(conn); +- } else { +- close(drm_fd); +- syslog(LOG_WARNING, +- "Unable to get DRM resources for card %s. " +- "Falling back to using xrandr output index.", +- dev_path); ++ break; ++ case 1: // no DRM info found + // This is probably a proprietary driver (e.g. Nvidia) that does + // not provide outputs via drm, so the only thing we can do is just + // assume that it is the only device assigned to X, and use the +diff --git a/src/vdagent/device-info.h b/src/vdagent/device-info.h +index 8646cc5..d4d8cbd 100644 +--- a/src/vdagent/device-info.h ++++ b/src/vdagent/device-info.h +@@ -28,3 +28,7 @@ bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, + Display *xdisplay, + XRRScreenResources *xres, + RROutput *output_id); ++ ++int get_connector_name_for_device_info(VDAgentDeviceDisplayInfo *device_info, ++ char *expected_name, size_t name_size, ++ bool has_virtual_zero_display); +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index 2577f9c..b82db5c 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -50,6 +47,8 @@ + * The x11.c and x11-randr.c files contains the x11-specific functions. + */ + struct VDAgentDisplay { ++ // association between SPICE display ID and expected connector name ++ GHashTable *connector_mapping; + struct vdagent_x11 *x11; + UdscsConnection *vdagentd; + int debug; +@@ -132,6 +130,7 @@ VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int + } + + display->x11->vdagent_display = display; ++ display->connector_mapping = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); + + display->x11_channel = g_io_channel_unix_new(vdagent_x11_get_fd(display->x11)); + if (display->x11_channel == NULL) { +@@ -167,6 +168,8 @@ void vdagent_display_destroy(VDAgentDisplay *display, int vdagentd_disconnected) + return; + } + ++ g_hash_table_destroy(display->connector_mapping); ++ + g_clear_pointer(&display->x11_channel, g_io_channel_unref); + vdagent_x11_destroy(display->x11, vdagentd_disconnected); + } +@@ -227,6 +232,19 @@ void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_ + device_display_info->channel_id, device_display_info->monitor_id); + } + ++ // Get the expected connector name from hardware info. Store it with the SPICE display ID. ++ char expected_name[100]; ++ int ret = get_connector_name_for_device_info(device_display_info, expected_name, ++ sizeof(expected_name), false); ++ if (ret == 0) { ++ g_hash_table_insert(display->connector_mapping, ++ g_strdup(expected_name), ++ GUINT_TO_POINTER(device_display_info->channel_id + device_display_info->monitor_id)); ++ syslog(LOG_DEBUG, "Mapping connector %s to display #%d", expected_name, ++ (device_display_info->channel_id + device_display_info->monitor_id)); ++ } ++ ++ // Also map the SPICE display ID to the corresponding X server object. + vdagent_x11_handle_device_display_info(display->x11, device_display_info); + + device_display_info = (VDAgentDeviceDisplayInfo*) ((char*) device_display_info + +-- +2.29.2 + + +From 4c8009068115b6f89491e0e3e108d38084b2417a Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Wed, 29 Apr 2020 16:31:58 +0200 +Subject: [PATCH 4/7] Split the "lookup_xrandr_output_for_device_info()" + function to reuse some of it with different backends. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Most display drivers will number their outputs starting with "1", but +some (like older QXL drivers) will start from 0. +As we need to map device names with SPICE display IDs, we need to know +what to expect. + +The logic that finds whether we have a O-based index or not is +independent from the backend, and can be reused outside of the X11 +implementation. + +This commit separates this logic for that purpose, to prevent code +duplication. +- create the has_zero_based_display_id() function to call whatever the + backend, and learn whether our IDs should be decremented or not. + Its code is taken out of lookup_xrandr_output_for_device_info() +- keep the lookup_xrandr_output_for_device_info() for x11 implementation +- add parameters to some functions to give them the information that IDs + should be decremented because of the 0-based index. + +Signed-off-by: Julien Ropé +Acked-by: Frediano Ziglio +--- + src/vdagent/device-info.c | 24 ++--------------- + src/vdagent/device-info.h | 3 ++- + src/vdagent/display.c | 56 +++++++++++++++++++++++++++++++++++++-- + src/vdagent/x11-randr.c | 5 ++-- + src/vdagent/x11.h | 3 ++- + 5 files changed, 63 insertions(+), 28 deletions(-) + +diff --git a/src/vdagent/device-info.c b/src/vdagent/device-info.c +index 8cf72c9..f07de03 100644 +--- a/src/vdagent/device-info.c ++++ b/src/vdagent/device-info.c +@@ -482,32 +482,12 @@ int get_connector_name_for_device_info(VDAgentDeviceDisplayInfo *device_info, + bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, + Display *xdisplay, + XRRScreenResources *xres, +- RROutput *output_id) ++ RROutput *output_id, ++ bool has_virtual_zero_display) + { + char expected_name[100]; + int ret; + +- // Older QXL drivers numbered their outputs starting with +- // 0. This contrasts with most drivers who start numbering +- // outputs with 1. In this case, the expected drm connector +- // name will need to be decremented before comparing to the +- // xrandr output name +- bool has_virtual_zero_display = false; +- for (int i = 0; i < xres->noutput; ++i) { +- XRROutputInfo *oinfo = XRRGetOutputInfo(xdisplay, xres, xres->outputs[i]); +- if (!oinfo) { +- syslog(LOG_WARNING, "Unable to lookup XRandr output info for output %li", +- xres->outputs[i]); +- return false; +- } +- if (strcmp(oinfo->name, "Virtual-0") == 0) { +- has_virtual_zero_display = true; +- XRRFreeOutputInfo(oinfo); +- break; +- } +- XRRFreeOutputInfo(oinfo); +- } +- + ret = get_connector_name_for_device_info(device_info, expected_name, sizeof(expected_name), + has_virtual_zero_display); + switch (ret) { +diff --git a/src/vdagent/device-info.h b/src/vdagent/device-info.h +index d4d8cbd..f90fbe2 100644 +--- a/src/vdagent/device-info.h ++++ b/src/vdagent/device-info.h +@@ -27,7 +27,8 @@ struct vdagent_x11; + bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo *device_info, + Display *xdisplay, + XRRScreenResources *xres, +- RROutput *output_id); ++ RROutput *output_id, ++ bool has_virtual_zero_display); + + int get_connector_name_for_device_info(VDAgentDeviceDisplayInfo *device_info, + char *expected_name, size_t name_size, +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index b82db5c..d556f92 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -212,6 +197,57 @@ gboolean vdagent_display_has_icons_on_desktop(VDAgentDisplay *display) + return FALSE; + } + ++static bool has_zero_based_display_id(VDAgentDisplay *display) ++{ ++ // Older QXL drivers numbered their outputs starting with ++ // 0. This contrasts with most drivers who start numbering ++ // outputs with 1. In this case, the expected drm connector ++ // name will need to be decremented before comparing to the ++ // display manager output name ++ bool ret = false; ++#ifdef USE_GTK_FOR_MONITORS ++ GdkDisplay *gdk_display = gdk_display_get_default(); ++ if (GDK_IS_WAYLAND_DISPLAY(gdk_display)) { ++ gdk_display_sync(gdk_display); ++ ++ GListModel *monitors = gdk_display_get_monitors(gdk_display); ++ int screen_count = g_list_model_get_n_items(monitors); ++ for (int i = 0; i < screen_count; i++) { ++ GdkMonitor *monitor = (GdkMonitor *)g_list_model_get_item(monitors, i); ++ const char *name = gdk_monitor_get_connector(monitor); ++ if (!name) { ++ continue; ++ } ++ ++ if (strcmp(name, "Virtual-0") == 0) { ++ ret = true; ++ break; ++ } ++ } ++ } ++ else // otherwise, use the X11 code (below) ++#endif ++ { ++ XRRScreenResources *xres = display->x11->randr.res; ++ Display *xdisplay = display->x11->display; ++ for (int i = 0; i < xres->noutput; ++i) { ++ XRROutputInfo *oinfo = XRRGetOutputInfo(xdisplay, xres, xres->outputs[i]); ++ if (!oinfo) { ++ syslog(LOG_WARNING, "Unable to lookup XRandr output info for output %li", ++ xres->outputs[i]); ++ return false; ++ } ++ if (strcmp(oinfo->name, "Virtual-0") == 0) { ++ ret = true; ++ XRRFreeOutputInfo(oinfo); ++ break; ++ } ++ XRRFreeOutputInfo(oinfo); ++ } ++ } ++ return ret; ++} ++ + // handle the device info message from the server. This will allow us to + // maintain a mapping from spice display id to xrandr output + void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_t *data, +@@ -221,6 +257,7 @@ void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_ + VDAgentDeviceDisplayInfo *device_display_info = graphics_device_info->display_info; + + void *buffer_end = data + size; ++ bool decrement_id = has_zero_based_display_id(display); + + syslog(LOG_INFO, "Received Graphics Device Info:"); + +@@ -246,7 +281,7 @@ void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_ + // Get the expected connector name from hardware info. Store it with the SPICE display ID. + char expected_name[100]; + int ret = get_connector_name_for_device_info(device_display_info, expected_name, +- sizeof(expected_name), false); ++ sizeof(expected_name), decrement_id); + if (ret == 0) { + g_hash_table_insert(display->connector_mapping, + g_strdup(expected_name), +@@ -258,7 +291,7 @@ void vdagent_display_handle_graphics_device_info(VDAgentDisplay *display, uint8_ + } + + // Also map the SPICE display ID to the corresponding X server object. +- vdagent_x11_handle_device_display_info(display->x11, device_display_info); ++ vdagent_x11_handle_device_display_info(display->x11, device_display_info, decrement_id); + + device_display_info = (VDAgentDeviceDisplayInfo*) ((char*) device_display_info + + sizeof(VDAgentDeviceDisplayInfo) + device_display_info->device_address_len); +diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c +index 145e75a..cac9643 100644 +--- a/src/vdagent/x11-randr.c ++++ b/src/vdagent/x11-randr.c +@@ -779,11 +779,12 @@ static void dump_monitors_config(struct vdagent_x11 *x11, + // handle the device info message from the server. This will allow us to + // maintain a mapping from spice display id to xrandr output + void vdagent_x11_handle_device_display_info(struct vdagent_x11 *x11, +- VDAgentDeviceDisplayInfo *device_display_info) ++ VDAgentDeviceDisplayInfo *device_display_info, ++ gboolean has_virtual_zero_display) + { + RROutput x_output; + if (lookup_xrandr_output_for_device_info(device_display_info, x11->display, +- x11->randr.res, &x_output)) { ++ x11->randr.res, &x_output, has_virtual_zero_display)) { + syslog(LOG_INFO, "Adding graphics device info: channel_id: %u monitor_id: " + "%u device_address: %s, device_display_id: %u xrandr output ID: %lu", + device_display_info->channel_id, +diff --git a/src/vdagent/x11.h b/src/vdagent/x11.h +index a75638c..45018bd 100644 +--- a/src/vdagent/x11.h ++++ b/src/vdagent/x11.h +@@ -53,6 +53,7 @@ void vdagent_x11_client_disconnected(struct vdagent_x11 *x11); + gchar *vdagent_x11_get_wm_name(struct vdagent_x11 *x11); + + void vdagent_x11_handle_device_display_info(struct vdagent_x11 *x11, +- VDAgentDeviceDisplayInfo *device_display_info); ++ VDAgentDeviceDisplayInfo *device_display_info, ++ gboolean has_virtual_zero_display); + + #endif +-- +2.29.2 + + +From 6c475b368d34f24509bd03412b0a8a4d81a9f2e7 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Thu, 26 Mar 2020 15:16:51 +0100 +Subject: [PATCH 5/7] Send guest resolution using GDK functions. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +- RHEL 8.4: skeleton implementation of vdagent_gtk_get_resolutions(). + Do not introduce GTK4 code into RHEL8.4 as this will not be used anyway. +- Add disabled monitors to the list before sending + (vdagent_display_send_daemon_guest_res) +Signed-off-by: Julien Ropé +--- + src/vdagent/display.c | 149 +++++++++++++++++++++++++++++++++++++++++- + 1 file changed, 146 insertions(+), 3 deletions(-) + +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index d556f92..a6de9c0 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -63,14 +55,76 @@ struct VDAgentDisplay { + GIOChannel *x11_channel; + }; + ++static gint vdagent_guest_xorg_resolution_compare(gconstpointer a, gconstpointer b) ++{ ++ struct vdagentd_guest_xorg_resolution *ptr_a, *ptr_b; ++ ++ ptr_a = (struct vdagentd_guest_xorg_resolution *)a; ++ ptr_b = (struct vdagentd_guest_xorg_resolution *)b; ++ ++ return ptr_a->display_id - ptr_b->display_id; ++} ++ ++static GArray *vdagent_gtk_get_resolutions(VDAgentDisplay *display, ++ int *width, int *height, int *screen_count) ++{ ++ return NULL; ++} ++ + void vdagent_display_send_daemon_guest_res(VDAgentDisplay *display, gboolean update) + { + GArray *res_array; +- int width, height, screen_count; ++ int width = 0, height = 0, screen_count = 0; + +- res_array = vdagent_x11_get_resolutions(display->x11, update, &width, &height, &screen_count); ++ res_array = vdagent_gtk_get_resolutions(display, &width, &height, &screen_count); + if (res_array == NULL) { +- return; ++ if (display->x11->dont_send_guest_xorg_res) { ++ return; ++ } ++ ++ res_array = vdagent_x11_get_resolutions(display->x11, update, ++ &width, &height, &screen_count); ++ if (res_array == NULL) { ++ return; ++ } + } ++ ++ if (res_array->len < g_hash_table_size(display->connector_mapping)) { ++ // Complete the array with disabled displays. ++ // We need to send 0x0 resolution to let the daemon know the display is not there anymore. ++ ++ syslog(LOG_DEBUG, "%d/%d displays found - completing with disabled displays.", ++ res_array->len, g_hash_table_size(display->connector_mapping)); ++ ++ GHashTableIter iter; ++ gpointer key, value; ++ g_hash_table_iter_init(&iter, display->connector_mapping); ++ while (g_hash_table_iter_next(&iter, &key, &value)) { ++ bool found = false; ++ int display_id = GPOINTER_TO_INT(value); ++ for (int i = 0; i < res_array->len; i++) { ++ struct vdagentd_guest_xorg_resolution *res = ++ (struct vdagentd_guest_xorg_resolution*)res_array->data; ++ if (res[i].display_id == display_id) { ++ found = true; ++ break; ++ } ++ } ++ if (!found) { ++ struct vdagentd_guest_xorg_resolution res; ++ ++ res.x = 0; ++ res.y = 0; ++ res.height = 0; ++ res.width = 0; ++ res.display_id = display_id; ++ ++ g_array_append_val(res_array, res); ++ } ++ } ++ } ++ ++ // sort the list to make sure we send them in the display_id order ++ g_array_sort(res_array, vdagent_guest_xorg_resolution_compare); + + if (display->debug) { +@@ -183,6 +231,7 @@ void vdagent_display_destroy(VDAgentDisplay *display, int vdagentd_disconnected) + + g_clear_pointer(&display->x11_channel, g_io_channel_unref); + vdagent_x11_destroy(display->x11, vdagentd_disconnected); ++ g_free(display); + } + + /* Function used to determine the default location to save file-xfers, +-- +2.29.2 + + +From 73bf8367268e7ef5a00fd23674b0a8700d0e4a85 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Mon, 12 Oct 2020 10:07:42 +0200 +Subject: [PATCH 3/4] Add implementation of the DBUS interface to Mutter. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This can be used to retrieve monitor resolutions directly from the +Mutter compositor under Wayland. +Under environments using a different compositor, the X11 lib will keep +being used. + +Signed-off-by: Julien Ropé +Acked-by: Jakub Janků +--- + Makefile.am | 2 + + src/vdagent/mutter.c | 276 +++++++++++++++++++++++++++++++++++++++++++ + src/vdagent/mutter.h | 33 ++++++ + 3 files changed, 311 insertions(+) + create mode 100644 src/vdagent/mutter.c + create mode 100644 src/vdagent/mutter.h + +diff --git a/Makefile.am b/Makefile.am +index 47d7820..e8fa4a6 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -49,6 +47,8 @@ src_spice_vdagent_SOURCES = \ + src/vdagent/display.h \ + src/vdagent/file-xfers.c \ + src/vdagent/file-xfers.h \ ++ src/vdagent/mutter.c \ ++ src/vdagent/mutter.h \ + src/vdagent/x11-priv.h \ + src/vdagent/x11-randr.c \ + src/vdagent/x11.c \ +diff --git a/src/vdagent/mutter.c b/src/vdagent/mutter.c +new file mode 100644 +index 0000000..f6ff11b +--- /dev/null ++++ b/src/vdagent/mutter.c +@@ -0,0 +1,276 @@ ++/* mutter.c - implements the DBUS interface to mutter ++ ++ Copyright 2020 Red Hat, Inc. ++ ++ Red Hat Authors: ++ Julien Ropé ++ ++ This program is free software: you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation, either version 3 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with this program. If not, see . ++ */ ++ ++#include ++ ++#include ++#include ++ ++#include ++ ++#include "vdagentd-proto.h" ++#include "mutter.h" ++ ++// MUTTER DBUS FORMAT STRINGS ++#define MODE_BASE_FORMAT "siiddad" ++#define MODE_FORMAT "(" MODE_BASE_FORMAT "a{sv})" ++#define MODES_FORMAT "a" MODE_FORMAT ++#define MONITOR_SPEC_FORMAT "(ssss)" ++#define MONITOR_FORMAT "(" MONITOR_SPEC_FORMAT MODES_FORMAT "a{sv})" ++#define MONITORS_FORMAT "a" MONITOR_FORMAT ++ ++#define LOGICAL_MONITOR_MONITORS_FORMAT "a" MONITOR_SPEC_FORMAT ++#define LOGICAL_MONITOR_FORMAT "(iidub" LOGICAL_MONITOR_MONITORS_FORMAT "a{sv})" ++#define LOGICAL_MONITORS_FORMAT "a" LOGICAL_MONITOR_FORMAT ++ ++#define CURRENT_STATE_FORMAT "(u" MONITORS_FORMAT LOGICAL_MONITORS_FORMAT "a{sv})" ++ ++ ++struct VDAgentMutterDBus { ++ GDBusProxy *dbus_proxy; ++ GHashTable *connector_mapping; ++}; ++ ++/** ++ * Initialise a communication to Mutter through its DBUS interface. ++ * ++ * Errors can indicate that another compositor is used. This is not a blocker, and we should default ++ * to use a different API then. ++ * ++ * Returns: ++ * An initialise VDAgentMutterDBus structure if successful. ++ * NULL if an error occured. ++ */ ++VDAgentMutterDBus *vdagent_mutter_create(GHashTable *connector_mapping) ++{ ++ GError *error = NULL; ++ VDAgentMutterDBus *mutter = g_new0(VDAgentMutterDBus, 1); ++ ++ mutter->connector_mapping = g_hash_table_ref(connector_mapping); ++ ++ GDBusProxyFlags flags = (G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START ++ | G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES ++ | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS); ++ ++ mutter->dbus_proxy = g_dbus_proxy_new_for_bus_sync(G_BUS_TYPE_SESSION, ++ flags, ++ NULL, ++ "org.gnome.Mutter.DisplayConfig", ++ "/org/gnome/Mutter/DisplayConfig", ++ "org.gnome.Mutter.DisplayConfig", ++ NULL, ++ &error); ++ if (!mutter->dbus_proxy) { ++ syslog(LOG_WARNING, "display: failed to create dbus proxy: %s", error->message); ++ g_clear_error(&error); ++ vdagent_mutter_destroy(mutter); ++ return NULL; ++ } ++ ++ return mutter; ++} ++ ++ ++void vdagent_mutter_destroy(VDAgentMutterDBus *mutter) ++{ ++ g_clear_object(&mutter->dbus_proxy); ++ g_hash_table_unref(mutter->connector_mapping); ++ g_free(mutter); ++} ++ ++/** Look through a list of logical monitor to find the one provided. ++ * Returns the corresponding x and y position of the monitor on the desktop. ++ * This function is a helper to vdagent_mutter_get_resolution(). ++ * ++ * Parameters: ++ * logical_monitor: initialized GVariant iterator. It will be copied to look through the items ++ * so that its original position is not modified. ++ * connector: name of the connector that must be found ++ * x and y: will received the found position ++ * ++ */ ++static void vdagent_mutter_get_monitor_position(GVariantIter *logical_monitors, ++ const gchar *connector, int *x, int *y) ++{ ++ GVariant *logical_monitor = NULL; ++ GVariantIter *logical_monitor_iterator = g_variant_iter_copy(logical_monitors); ++ while (g_variant_iter_next(logical_monitor_iterator, "@"LOGICAL_MONITOR_FORMAT, ++ &logical_monitor)) { ++ GVariantIter *tmp_monitors = NULL; ++ ++ g_variant_get_child(logical_monitor, 0, "i", x); ++ g_variant_get_child(logical_monitor, 1, "i", y); ++ g_variant_get_child(logical_monitor, 5, LOGICAL_MONITOR_MONITORS_FORMAT, &tmp_monitors); ++ ++ g_variant_unref(logical_monitor); ++ ++ GVariant *tmp_monitor = NULL; ++ gboolean found = FALSE; ++ while (!found && g_variant_iter_next(tmp_monitors, "@"MONITOR_SPEC_FORMAT, &tmp_monitor)) { ++ const gchar *tmp_connector; ++ ++ g_variant_get_child(tmp_monitor, 0, "&s", &tmp_connector); ++ ++ if (g_strcmp0(connector, tmp_connector) == 0) { ++ found = TRUE; ++ } ++ g_variant_unref(tmp_monitor); ++ } ++ ++ g_variant_iter_free(tmp_monitors); ++ ++ if (found) { ++ break; ++ } ++ *x = *y = 0; ++ } ++ g_variant_iter_free(logical_monitor_iterator); ++} ++ ++GArray *vdagent_mutter_get_resolutions(VDAgentMutterDBus *mutter, ++ int *desktop_width, int *desktop_height, int *screen_count) ++{ ++ GError *error = NULL; ++ GArray *res_array = NULL; ++ ++ // keep track of monitors we find and are not mapped to SPICE displays ++ // we will map them back later (assuming display ID == monitor index) ++ // this prevents the need from looping twice on all DBUS items ++ GArray *not_found_array = NULL; ++ ++ if (!mutter) { ++ return res_array; ++ } ++ ++ GVariant *values = g_dbus_proxy_call_sync(mutter->dbus_proxy, ++ "GetCurrentState", ++ NULL, ++ G_DBUS_CALL_FLAGS_NONE, ++ -1, // use proxy default timeout ++ NULL, ++ &error); ++ if (!values) { ++ syslog(LOG_WARNING, "display: failed to call GetCurrentState from mutter over DBUS"); ++ if (error != NULL) { ++ syslog(LOG_WARNING, " error message: %s", error->message); ++ g_clear_error(&error); ++ } ++ return res_array; ++ } ++ ++ res_array = g_array_new(FALSE, FALSE, sizeof(struct vdagentd_guest_xorg_resolution)); ++ not_found_array = g_array_new(FALSE, FALSE, sizeof(struct vdagentd_guest_xorg_resolution)); ++ ++ GVariantIter *monitors = NULL; ++ GVariantIter *logical_monitors = NULL; ++ ++ g_variant_get_child(values, 1, MONITORS_FORMAT, &monitors); ++ g_variant_get_child(values, 2, LOGICAL_MONITORS_FORMAT, &logical_monitors); ++ ++ // list monitors ++ GVariant *monitor = NULL; ++ *screen_count = g_variant_iter_n_children(monitors); ++ ++ while (g_variant_iter_next(monitors, "@"MONITOR_FORMAT, &monitor)) { ++ ++ const gchar *connector = NULL; ++ GVariantIter *modes = NULL; ++ GVariant *monitor_specs = NULL; ++ ++ g_variant_get_child(monitor, 0, "@"MONITOR_SPEC_FORMAT, &monitor_specs); ++ g_variant_get_child(monitor_specs, 0, "&s", &connector); ++ g_variant_get_child(monitor, 1, MODES_FORMAT, &modes); ++ ++ g_variant_unref(monitor_specs); ++ g_variant_unref(monitor); ++ ++ // list modes ++ GVariant *mode = NULL; ++ while (g_variant_iter_next(modes, "@"MODE_FORMAT, &mode)) { ++ GVariant *properties = NULL; ++ gboolean is_current; ++ ++ g_variant_get_child(mode, 6, "@a{sv}", &properties); ++ if (!g_variant_lookup(properties, "is-current", "b", &is_current)) { ++ is_current = FALSE; ++ } ++ g_variant_unref(properties); ++ ++ if (!is_current) { ++ g_variant_unref(mode); ++ continue; ++ } ++ ++ struct vdagentd_guest_xorg_resolution curr; ++ vdagent_mutter_get_monitor_position(logical_monitors, connector, &curr.x, &curr.y); ++ g_variant_get_child(mode, 1, "i", &curr.width); ++ g_variant_get_child(mode, 2, "i", &curr.height); ++ g_variant_unref(mode); ++ ++ // compute the size of the desktop based on the dimension of the monitors ++ if (curr.x + curr.width > *desktop_width) { ++ *desktop_width = curr.x + curr.width; ++ } ++ if (curr.y + curr.height > *desktop_height) { ++ *desktop_height = curr.y + curr.height; ++ } ++ ++ gpointer value; ++ if (g_hash_table_lookup_extended(mutter->connector_mapping, connector, NULL, &value)) { ++ curr.display_id = GPOINTER_TO_UINT(value); ++ syslog(LOG_DEBUG, ++ "Found monitor %s with geometry %dx%d+%d-%d - associating it to SPICE display #%d", ++ connector, curr.width, curr.height, curr.x, curr.y, curr.display_id); ++ g_array_append_val(res_array, curr); ++ } else { ++ syslog(LOG_DEBUG, "No SPICE display found for connector %s", connector); ++ g_array_append_val(not_found_array, curr); ++ } ++ ++ break; ++ } ++ g_variant_iter_free(modes); ++ } ++ ++ g_variant_iter_free(logical_monitors); ++ g_variant_iter_free(monitors); ++ ++ int i; ++ ++ if (res_array->len == 0) { ++ syslog(LOG_DEBUG, "%s: No Spice display ID matching - assuming display ID == Monitor index", ++ __FUNCTION__); ++ g_array_free(res_array, TRUE); ++ res_array = not_found_array; ++ ++ struct vdagentd_guest_xorg_resolution *res; ++ res = (struct vdagentd_guest_xorg_resolution*)res_array->data; ++ for (i = 0; i < res_array->len; i++) { ++ res[i].display_id = i; ++ } ++ } ++ else { ++ g_array_free(not_found_array, TRUE); ++ } ++ ++ g_variant_unref(values); ++ return res_array; ++} +diff --git a/src/vdagent/mutter.h b/src/vdagent/mutter.h +new file mode 100644 +index 0000000..abd2bd2 +--- /dev/null ++++ b/src/vdagent/mutter.h +@@ -0,0 +1,33 @@ ++/* mutter.h - implements the DBUS interface to mutter ++ ++ Copyright 2020 Red Hat, Inc. ++ ++ Red Hat Authors: ++ Julien Ropé ++ ++ This program is free software: you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation, either version 3 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with this program. If not, see . ++ */ ++ ++#ifndef SRC_VDAGENT_MUTTER_H_ ++#define SRC_VDAGENT_MUTTER_H_ ++ ++typedef struct VDAgentMutterDBus VDAgentMutterDBus; ++ ++VDAgentMutterDBus *vdagent_mutter_create(GHashTable *connector_mapping); ++void vdagent_mutter_destroy(VDAgentMutterDBus *mutter); ++ ++GArray *vdagent_mutter_get_resolutions(VDAgentMutterDBus *mutter, int *width, int *height, int *screen_count); ++ ++ ++#endif /* SRC_VDAGENT_MUTTER_H_ */ +-- +2.29.2 + + +From 58d97554e52be79f450c4da664c2191966bf60a9 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Julien=20Rop=C3=A9?= +Date: Mon, 12 Oct 2020 14:55:51 +0200 +Subject: [PATCH 4/4] Use the Mutter API from display.c +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Signed-off-by: Julien Ropé +Acked-by: Jakub Janků +--- + src/vdagent/display.c | 32 ++++++++++++++++++++++---------- + 1 file changed, 22 insertions(+), 10 deletions(-) + +diff --git a/src/vdagent/display.c b/src/vdagent/display.c +index 9569173..790d9ad 100644 +--- a/src/vdagent/display.c ++++ b/src/vdagent/display.c +@@ -41,6 +35,7 @@ + #include "device-info.h" + #include "vdagentd-proto.h" + ++#include "mutter.h" + #include "display.h" + + /** +@@ -59,6 +54,7 @@ struct VDAgentDisplay { + UdscsConnection *vdagentd; + int debug; + GIOChannel *x11_channel; ++ VDAgentMutterDBus *mutter; + }; + + static gint vdagent_guest_xorg_resolution_compare(gconstpointer a, gconstpointer b) +@@ -161,17 +78,22 @@ void vdagent_display_send_daemon_guest_res(VDAgentDisplay *display, gboolean upd + GArray *res_array; + int width = 0, height = 0, screen_count = 0; + +- res_array = vdagent_gtk_get_resolutions(display, &width, &height, &screen_count); ++ // Try various backends one after the other. ++ // We try Mutter first, because it has a bigger probability of being available. ++ // Second GTK, because if/when we build with GTK4, this is the one that will work best. ++ // Finally we try X11. This is the default, and should work OK in most circumstances. ++ res_array = vdagent_mutter_get_resolutions(display->mutter, &width, &height, &screen_count); ++ + if (res_array == NULL) { +- if (display->x11->dont_send_guest_xorg_res) { +- return; +- } ++ res_array = vdagent_gtk_get_resolutions(display, &width, &height, &screen_count); ++ } + +- res_array = vdagent_x11_get_resolutions(display->x11, update, +- &width, &height, &screen_count); +- if (res_array == NULL) { +- return; +- } ++ if (res_array == NULL) { ++ res_array = vdagent_x11_get_resolutions(display->x11, update, &width, &height, &screen_count); ++ } ++ ++ if (res_array == NULL) { ++ return; + } + + if (res_array->len < g_hash_table_size(display->connector_mapping)) { +@@ -280,6 +199,8 @@ VDAgentDisplay* vdagent_display_create(UdscsConnection *vdagentd, int debug, int + display->x11->vdagent_display = display; + display->connector_mapping = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); + ++ display->mutter = vdagent_mutter_create(display->connector_mapping); ++ + display->x11_channel = g_io_channel_unix_new(vdagent_x11_get_fd(display->x11)); + if (display->x11_channel == NULL) { + vdagent_x11_destroy(display->x11, TRUE); +@@ -314,10 +235,13 @@ void vdagent_display_destroy(VDAgentDisplay *display, int vdagentd_disconnected) + return; + } + +- g_hash_table_destroy(display->connector_mapping); + + g_clear_pointer(&display->x11_channel, g_io_channel_unref); + vdagent_x11_destroy(display->x11, vdagentd_disconnected); ++ ++ vdagent_mutter_destroy(display->mutter); ++ ++ g_hash_table_destroy(display->connector_mapping); + g_free(display); + } + +-- +2.29.2 + diff --git a/SPECS/spice-vdagent.spec b/SPECS/spice-vdagent.spec index 3149b1a..e469cf9 100644 --- a/SPECS/spice-vdagent.spec +++ b/SPECS/spice-vdagent.spec @@ -1,6 +1,6 @@ Name: spice-vdagent Version: 0.20.0 -Release: 1%{?dist} +Release: 3%{?dist} Summary: Agent for Spice guests Group: Applications/System License: GPLv3+ @@ -12,6 +12,21 @@ Patch0001: 0001-vdagentd-work-around-GLib-s-fork-issues.patch Patch0002: 0002-vdagentd-init-static-uinput-before-fork.patch Patch0003: 0003-systemd-login-Avoid-a-crash-on-container.patch Patch0004: 0004-Fix-possible-compile-error-using-former-GLib2-versio.patch +Patch0005: 0005-vdagentd-Use-bool-for-agent_owns_clipboard-and-clien.patch +Patch0006: 0006-vdagentd-Automatically-release-agent_data.patch +Patch0007: 0007-vdagent-connection-Pass-err-to-g_credentials_get_uni.patch +Patch0008: 0008-vdagentd-Better-check-for-vdagent_connection_get_pee.patch +Patch0009: 0009-vdagentd-Avoid-calling-chmod.patch +Patch0010: 0010-Avoids-unchecked-file-transfer-IDs-allocation-and-us.patch +Patch0011: 0011-Avoids-uncontrolled-active_xfers-allocations.patch +Patch0012: 0012-Avoids-unlimited-agent-connections.patch +Patch0013: 0013-Avoids-user-session-hijacking.patch +Patch0014: 0014-Better-check-for-sessions.patch +Patch0015: 0015-vdagentd-Limit-number-of-agents-per-session-to-1.patch +Patch0016: 0016-cleanup-active_xfers-when-the-client-disconnects.patch +Patch0017: 0017-vdagentd-do-not-allow-to-use-an-already-used-file-xf.patch +Patch0018: 0018-Add-a-test-for-session_info.patch +Patch0019: 0019-wayland-fix-monitor-mapping-issues.patch BuildRequires: git-core gnupg2 BuildRequires: systemd-devel @@ -77,7 +92,14 @@ make install DESTDIR=$RPM_BUILD_ROOT V=2 %changelog -* Mon May 15 2020 Victor Toso 0.20.0-1 +* Wed Jan 20 2021 Julien Ropé - 0.20.0-3 +- Fix mouse problems in multi-monitor environments under Wayland + Resolves: rhbz#1790904 rhbz#1824610 + +* Mon Oct 19 2020 Frediano Ziglio 0.20.0-2 +- Resolves: CVE-2020-25650, CVE-2020-25651, CVE-2020-25652, CVE-2020-25653 + +* Fri May 15 2020 Victor Toso 0.20.0-1 - Update to 0.20.0 - Backport fixes post-release Resolves: rhbz#1817476