From 689269dcdef60608f58a4e0cdbf955e4944bdc49 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 8 Feb 2024 15:51:30 +0000 Subject: [PATCH 01/12] Fixes some problems with monitor hotplug This fixes some monitor hotplug issues with non-GFX codepaths. 1) The server_version_message() was working on an out-of-date copy of the client_info. As a result, the X server and the window manager did not agree on the number of windows 2) As a result of 1), a memory leak was found in the VNC module. --- libxrdp/libxrdp.c | 9 --------- vnc/vnc.c | 1 + xrdp/xrdp_mm.c | 8 +++++++- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/libxrdp/libxrdp.c b/libxrdp/libxrdp.c index af1714de87..400de49664 100644 --- a/libxrdp/libxrdp.c +++ b/libxrdp/libxrdp.c @@ -1153,15 +1153,6 @@ libxrdp_reset(struct xrdp_session *session, return 0; } - /* if same (and only one monitor on client) don't need to do anything */ - if (client_info->display_sizes.session_width == width && - client_info->display_sizes.session_height == height && - client_info->bpp == bpp && - (client_info->display_sizes.monitorCount == 0 || client_info->multimon == 0)) - { - return 0; - } - client_info->display_sizes.session_width = width; client_info->display_sizes.session_height = height; client_info->display_sizes.monitorCount = 0; diff --git a/vnc/vnc.c b/vnc/vnc.c index 77845e2f4d..d9b0520fc2 100644 --- a/vnc/vnc.c +++ b/vnc/vnc.c @@ -2133,6 +2133,7 @@ lib_mod_set_param(struct vnc *v, const char *name, const char *value) (const struct xrdp_client_info *) value; g_free(v->client_layout.s); + v->client_layout.count = 0; /* Save monitor information from the client */ if (!client_info->multimon || client_info->display_sizes.monitorCount < 1) diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 4a7e90731f..614175cdc3 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1743,6 +1743,13 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) mm, WMRZ_SERVER_VERSION_MESSAGE_START); break; case WMRZ_SERVER_VERSION_MESSAGE_START: + /* Update the client_info structure with the new description + * and tell the module so it can communicate the new + * screen layout to the backend */ + sync_dynamic_monitor_data(wm, &(description->description)); + module->mod_set_param(module, "client_info", + (const char *) (wm->session->client_info)); + error = module->mod_server_version_message(module); if (error != 0) { @@ -1813,7 +1820,6 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) " xrdp_bitmap_resize failed %d", error); return advance_error(error, mm); } - sync_dynamic_monitor_data(wm, &(description->description)); advance_resize_state_machine(mm, WMRZ_EGFX_INITIALIZE); break; case WMRZ_EGFX_INITIALIZE: From 74598d1cc34639358497a39e332db0b4ba5303f7 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 14 Feb 2024 12:14:10 +0000 Subject: [PATCH 02/12] Fix sending of monitor layout PDU From [MS-RCPBCGR] 3.3.5.12.1:- > ...The contents of this PDU SHOULD NOT be compressed. > > This PDU MUST NOT be sent to a client that has not indicated support for > it by setting the RNS_UD_CS_SUPPORT_MONITOR_LAYOUT_PDU flag (0x0040) > in the earlyCapabilityFlags field of the Client Core Data (section > 2.2.1.3.2). Also, 2.2.12.1 specifies the source channel must be zero. In testing, a compressed monitor layout PDU causes mstsc.exe to exit with a protocol error. --- common/ms-rdpbcgr.h | 2 ++ libxrdp/libxrdp.h | 4 ++++ libxrdp/xrdp_caps.c | 21 ++++++++++++--------- libxrdp/xrdp_rdp.c | 33 +++++++++++++++++++++++++-------- 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/common/ms-rdpbcgr.h b/common/ms-rdpbcgr.h index 11d50bdde6..4538483231 100644 --- a/common/ms-rdpbcgr.h +++ b/common/ms-rdpbcgr.h @@ -74,6 +74,7 @@ /* Client Core Data: earlyCapabilityFlags (2.2.1.3.2) */ #define RNS_UD_CS_WANT_32BPP_SESSION 0x0002 +#define RNS_UD_CS_SUPPORT_MONITOR_LAYOUT_PDU 0x0040 #define RNS_UD_CS_SUPPORT_DYNVC_GFX_PROTOCOL 0x0100 /* Client Core Data: connectionType (2.2.1.3.2) */ @@ -448,6 +449,7 @@ #define RDP_DATA_PDU_LOGON 38 #define RDP_DATA_PDU_FONT2 39 #define RDP_DATA_PDU_DISCONNECT 47 +#define PDUTYPE2_MONITOR_LAYOUT_PDU 55 /* TS_SECURITY_HEADER: flags (2.2.8.1.1.2.1) */ /* TODO: to be renamed */ diff --git a/libxrdp/libxrdp.h b/libxrdp/libxrdp.h index b9d1b8f94b..9b289af9cf 100644 --- a/libxrdp/libxrdp.h +++ b/libxrdp/libxrdp.h @@ -437,6 +437,10 @@ int xrdp_rdp_send_data(struct xrdp_rdp *self, struct stream *s, int data_pdu_type); int +xrdp_rdp_send_data_from_channel(struct xrdp_rdp *self, struct stream *s, + int data_pdu_type, int channel_id, + int compress); +int xrdp_rdp_send_fastpath(struct xrdp_rdp *self, struct stream *s, int data_pdu_type); int diff --git a/libxrdp/xrdp_caps.c b/libxrdp/xrdp_caps.c index 42eafb1fa5..524eff3a8a 100644 --- a/libxrdp/xrdp_caps.c +++ b/libxrdp/xrdp_caps.c @@ -48,6 +48,7 @@ xrdp_caps_send_monitorlayout(struct xrdp_rdp *self) struct stream *s; uint32_t i; struct display_size_description *description; + int rv = 0; make_stream(s); init_stream(s, 8192); @@ -74,14 +75,13 @@ xrdp_caps_send_monitorlayout(struct xrdp_rdp *self) s_mark_end(s); - if (xrdp_rdp_send_data(self, s, 0x37) != 0) - { - free_stream(s); - return 1; - } - + // [MS-RDPBCGR] + // - 2.2.12.1 - ...the pduSource field MUST be set to zero. + // - 3.3.5.12.1 - The contents of this PDU SHOULD NOT be compressed + rv = xrdp_rdp_send_data_from_channel(self, s, + PDUTYPE2_MONITOR_LAYOUT_PDU, 0, 0); free_stream(s); - return 0; + return rv; } /*****************************************************************************/ @@ -1337,8 +1337,11 @@ xrdp_caps_send_demand_active(struct xrdp_rdp *self) return 1; } - /* send Monitor Layout PDU for dual monitor */ - if (self->client_info.display_sizes.monitorCount > 0 && + /* send Monitor Layout PDU for multi-monitor */ + int early_cap_flags = self->client_info.mcs_early_capability_flags; + + if ((early_cap_flags & RNS_UD_CS_SUPPORT_MONITOR_LAYOUT_PDU) != 0 && + self->client_info.display_sizes.monitorCount > 0 && self->client_info.multimon == 1) { LOG_DEVEL(LOG_LEVEL_TRACE, "xrdp_caps_send_demand_active: sending monitor layout pdu"); diff --git a/libxrdp/xrdp_rdp.c b/libxrdp/xrdp_rdp.c index 29901db610..6be1ff6ead 100644 --- a/libxrdp/xrdp_rdp.c +++ b/libxrdp/xrdp_rdp.c @@ -585,11 +585,13 @@ xrdp_rdp_send(struct xrdp_rdp *self, struct stream *s, int pdu_type) } /*****************************************************************************/ -/* Send a [MS-RDPBCGR] Data PDU with for the given pduType2 with the headers +/* Send a [MS-RDPBCGR] Data PDU for the given pduType2 from + * the specified source with the headers added and data compressed */ int -xrdp_rdp_send_data(struct xrdp_rdp *self, struct stream *s, - int data_pdu_type) +xrdp_rdp_send_data_from_channel(struct xrdp_rdp *self, struct stream *s, + int data_pdu_type, int channel_id, + int compress) { int len; int ctype; @@ -614,7 +616,8 @@ xrdp_rdp_send_data(struct xrdp_rdp *self, struct stream *s, clen = len; tocomplen = pdulen - 18; - if (self->client_info.rdp_compression && self->session->up_and_running) + if (compress && self->client_info.rdp_compression && + self->session->up_and_running) { mppc_enc = self->mppc_enc; if (compress_rdp(mppc_enc, (tui8 *)(s->p + 18), tocomplen)) @@ -643,7 +646,8 @@ xrdp_rdp_send_data(struct xrdp_rdp *self, struct stream *s, else { LOG_DEVEL(LOG_LEVEL_TRACE, - "xrdp_rdp_send_data: compress_rdp failed, sending " + "xrdp_rdp_send_data_from_channel: " + "compress_rdp failed, sending " "uncompressed data. type %d, flags %d", mppc_enc->protocol_type, mppc_enc->flags); } @@ -652,11 +656,11 @@ xrdp_rdp_send_data(struct xrdp_rdp *self, struct stream *s, /* TS_SHARECONTROLHEADER */ out_uint16_le(s, pdulen); /* totalLength */ out_uint16_le(s, pdutype); /* pduType */ - out_uint16_le(s, self->mcs_channel); /* pduSource */ + out_uint16_le(s, channel_id); /* pduSource */ LOG_DEVEL(LOG_LEVEL_TRACE, "Adding header [MS-RDPBCGR] TS_SHARECONTROLHEADER " "totalLength %d, pduType.type %s (%d), pduType.PDUVersion %d, " "pduSource %d", pdulen, PDUTYPE_TO_STR(pdutype & 0xf), - pdutype & 0xf, ((pdutype & 0xfff0) >> 4), self->mcs_channel); + pdutype & 0xf, ((pdutype & 0xfff0) >> 4), channel_id); /* TS_SHAREDATAHEADER */ out_uint32_le(s, self->share_id); @@ -673,13 +677,26 @@ xrdp_rdp_send_data(struct xrdp_rdp *self, struct stream *s, if (xrdp_sec_send(self->sec_layer, s, MCS_GLOBAL_CHANNEL) != 0) { - LOG(LOG_LEVEL_ERROR, "xrdp_rdp_send_data: xrdp_sec_send failed"); + LOG(LOG_LEVEL_ERROR, "xrdp_rdp_send_data_from_channel: " + "xrdp_sec_send failed"); return 1; } return 0; } + +/*****************************************************************************/ +/* Send a [MS-RDPBCGR] Data PDU on the MCS channel for the given pduType2 + * with the headers added and data compressed */ +int +xrdp_rdp_send_data(struct xrdp_rdp *self, struct stream *s, + int data_pdu_type) +{ + return xrdp_rdp_send_data_from_channel(self, s, data_pdu_type, + self->mcs_channel, 1); +} + /*****************************************************************************/ /* returns the fastpath rdp byte count */ int From 27ad405cd88229b61f2aa3d91fa2110d43c45f52 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 14:31:29 +0000 Subject: [PATCH 03/12] Add a client_resize_mode field This stores what kind of resizing (if any) can be achieved with a Deactivation-Reactivation sequence. --- common/xrdp_client_info.h | 10 ++++++++++ libxrdp/xrdp_caps.c | 42 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/common/xrdp_client_info.h b/common/xrdp_client_info.h index 03dd5e3c70..0516f63c01 100644 --- a/common/xrdp_client_info.h +++ b/common/xrdp_client_info.h @@ -67,6 +67,13 @@ struct display_size_description unsigned int session_height; }; +enum client_resize_mode +{ + CRMODE_NONE, + CRMODE_SINGLE_SCREEN, + CRMODE_MULTI_SCREEN +}; + /** * Information about the xrdp client * @@ -218,6 +225,9 @@ struct xrdp_client_info int large_pointer_support_flags; int gfx; + + // Can we resize the desktop by using a Deactivation-Reactivation Sequence? + enum client_resize_mode client_resize_mode; }; enum xrdp_encoder_flags diff --git a/libxrdp/xrdp_caps.c b/libxrdp/xrdp_caps.c index 524eff3a8a..dcb754e22a 100644 --- a/libxrdp/xrdp_caps.c +++ b/libxrdp/xrdp_caps.c @@ -120,6 +120,45 @@ xrdp_caps_process_general(struct xrdp_rdp *self, struct stream *s, return 0; } + +/*****************************************************************************/ +static int +xrdp_caps_process_bitmap(struct xrdp_rdp *self, struct stream *s, + int len) +{ + /* [MS-RDPBCGR] 2.2.7.1.2 */ + int desktopResizeFlag; + if (len < 14 + 2) + { + LOG(LOG_LEVEL_ERROR, "Not enough bytes in the stream: " + "len 16, remaining %d", len); + return 1; + } + + in_uint8s(s, 14); + in_uint16_le(s, desktopResizeFlag); + + /* Work out what kind of client resizing we can do from the server */ + int early_cap_flags = self->client_info.mcs_early_capability_flags; + if (desktopResizeFlag == 0) + { + self->client_info.client_resize_mode = CRMODE_NONE; + LOG(LOG_LEVEL_INFO, "Client cannot be resized by xrdp"); + } + else if ((early_cap_flags & RNS_UD_CS_SUPPORT_MONITOR_LAYOUT_PDU) == 0) + { + self->client_info.client_resize_mode = CRMODE_SINGLE_SCREEN; + LOG(LOG_LEVEL_INFO, "Client supports single-screen resizes by xrdp"); + } + else + { + self->client_info.client_resize_mode = CRMODE_MULTI_SCREEN; + LOG(LOG_LEVEL_INFO, "Client supports multi-screen resizes by xrdp"); + } + + return 0; +} + /*****************************************************************************/ /* * Process [MS-RDPBCGR] TS_ORDER_CAPABILITYSET (2.2.7.1.3) message. @@ -766,7 +805,8 @@ xrdp_caps_process_confirm_active(struct xrdp_rdp *self, struct stream *s) break; case CAPSTYPE_BITMAP: LOG_DEVEL(LOG_LEVEL_INFO, "Received [MS-RDPBCGR] TS_CONFIRM_ACTIVE_PDU - TS_CAPS_SET " - "capabilitySetType = CAPSTYPE_BITMAP - Ignored"); + "capabilitySetType = CAPSTYPE_BITMAP"); + xrdp_caps_process_bitmap(self, s, len); break; case CAPSTYPE_ORDER: LOG_DEVEL(LOG_LEVEL_INFO, "Received [MS-RDPBCGR] TS_CONFIRM_ACTIVE_PDU - TS_CAPS_SET " From ae836fb543a454faf0f7974b368b224ee0b0dd1a Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 14:48:54 +0000 Subject: [PATCH 04/12] Update module interfaces with new calls for resizing This commit DOES NOT compile. This change alters these module interface calls:- 1) mod_server_monitor_resize() (Call from xrdp to module). Updated. 2) server_reset() (Call from module to xrdp). Replaced. The mod_server_monitor_resize() call is updated :- 1) to allow a monitor list to be passed in for a multimon resize 2) with an 'in_progress' return value which tells the caller whether or not to expect a callback. The server_reset() call served two purposes up until now:- 1) To allow a module to resize a single monitor session. There is no way to request a multi-monitor resize from the module 2) (with bpp == 0) To signal to the mm resize state machine that a server screen resize hsa finished. This is split into two calls:- 1) client_monitor_resize() to allow a mdule to request a multimon resize. 2) server_monitor_resize_done(). This is called by a module when a resize is completed. --- mc/mc.h | 10 ++++++++-- neutrinordp/xrdp-neutrinordp.h | 12 +++++++++--- vnc/vnc.h | 30 ++++++++++++++++++++++++------ xrdp/xrdp_types.h | 12 +++++++++--- xup/xup.h | 12 +++++++++--- 5 files changed, 59 insertions(+), 17 deletions(-) diff --git a/mc/mc.h b/mc/mc.h index 25444dad66..6aaf441d92 100644 --- a/mc/mc.h +++ b/mc/mc.h @@ -31,6 +31,9 @@ struct source_info; +/* Defined in xrdp_client_info.h */ +struct monitor_info; + struct mod { int size; /* size of this struct */ @@ -81,7 +84,10 @@ struct mod int box_left, int box_top, int box_right, int box_bottom, int x, int y, char *data, int data_len); - int (*server_reset)(struct mod *v, int width, int height, int bpp); + int (*client_monitor_resize)(struct mod *v, int width, int height, + int num_monitors, + const struct monitor_info *monitors); + int (*server_monitor_resize_done)(struct mod *v); int (*server_get_channel_count)(struct mod *v); int (*server_query_channel)(struct mod *v, int index, char *channel_name, @@ -92,7 +98,7 @@ struct mod int total_data_len, int flags); int (*server_bell_trigger)(struct mod *v); int (*server_chansrv_in_use)(struct mod *v); - tintptr server_dumby[100 - 27]; /* align, 100 minus the number of server + tintptr server_dumby[100 - 28]; /* align, 100 minus the number of server functions above */ /* common */ tintptr handle; /* pointer to self as long */ diff --git a/neutrinordp/xrdp-neutrinordp.h b/neutrinordp/xrdp-neutrinordp.h index 2837131fdd..d5dc78a242 100644 --- a/neutrinordp/xrdp-neutrinordp.h +++ b/neutrinordp/xrdp-neutrinordp.h @@ -88,7 +88,10 @@ struct mod int (*mod_suppress_output)(struct mod *mod, int suppress, int left, int top, int right, int bottom); int (*mod_server_monitor_resize)(struct mod *mod, - int width, int height); + int width, int height, + int num_monitors, + const struct monitor_info *monitors, + int *in_progress); int (*mod_server_monitor_full_invalidate)(struct mod *mod, int width, int height); int (*mod_server_version_message)(struct mod *mod); @@ -126,7 +129,10 @@ struct mod int box_left, int box_top, int box_right, int box_bottom, int x, int y, char *data, int data_len); - int (*server_reset)(struct mod *v, int width, int height, int bpp); + int (*client_monitor_resize)(struct mod *v, int width, int height, + int num_monitors, + const struct monitor_info *monitors); + int (*server_monitor_resize_done)(struct mod *v); int (*server_get_channel_count)(struct mod *v); int (*server_query_channel)(struct mod *v, int index, char *channel_name, @@ -191,7 +197,7 @@ struct mod int flags, int frame_id); int (*server_session_info)(struct mod *v, const char *data, int data_bytes); - tintptr server_dumby[100 - 46]; /* align, 100 minus the number of server + tintptr server_dumby[100 - 47]; /* align, 100 minus the number of server functions above */ /* common */ tintptr handle; /* pointer to self as long */ diff --git a/vnc/vnc.h b/vnc/vnc.h index 5cf6539111..22c32c44d5 100644 --- a/vnc/vnc.h +++ b/vnc/vnc.h @@ -27,6 +27,7 @@ #include "os_calls.h" #include "defines.h" #include "guid.h" +#include "ms-rdpbcgr.h" #define CURRENT_MOD_VER 4 @@ -47,7 +48,7 @@ struct vnc_screen_layout int total_height; unsigned int count; /* For comparison, screens are sorted in increasing order of ID */ - struct vnc_screen *s; + struct vnc_screen s[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; }; /** @@ -60,11 +61,21 @@ enum vnc_resize_status VRS_DONE }; +enum vnc_resize_support_status +{ + VRSS_NOT_SUPPORTED, + VRSS_SUPPORTED, + VRSS_UNKNOWN +}; + struct source_info; /* Defined in vnc_clip.c */ struct vnc_clipboard_data; +/* Defined in xrdp_client_info.h */ +struct monitor_info; + struct vnc { int size; /* size of this struct */ @@ -85,7 +96,10 @@ struct vnc int (*mod_suppress_output)(struct vnc *v, int suppress, int left, int top, int right, int bottom); int (*mod_server_monitor_resize)(struct vnc *v, - int width, int height); + int width, int height, + int num_monitors, + const struct monitor_info *monitors, + int *in_progress); int (*mod_server_monitor_full_invalidate)(struct vnc *v, int width, int height); int (*mod_server_version_message)(struct vnc *v); @@ -123,7 +137,10 @@ struct vnc int box_left, int box_top, int box_right, int box_bottom, int x, int y, char *data, int data_len); - int (*server_reset)(struct vnc *v, int width, int height, int bpp); + int (*client_monitor_resize)(struct vnc *v, int width, int height, + int num_monitors, + const struct monitor_info *monitors); + int (*server_monitor_resize_done)(struct vnc *v); int (*server_get_channel_count)(struct vnc *v); int (*server_query_channel)(struct vnc *v, int index, char *channel_name, @@ -134,7 +151,7 @@ struct vnc int total_data_len, int flags); int (*server_bell_trigger)(struct vnc *v); int (*server_chansrv_in_use)(struct vnc *v); - tintptr server_dumby[100 - 27]; /* align, 100 minus the number of server + tintptr server_dumby[100 - 28]; /* align, 100 minus the number of server functions above */ /* common */ tintptr handle; /* pointer to self as long */ @@ -142,8 +159,6 @@ struct vnc tintptr painter; struct source_info *si; /* mod data */ - int server_width; - int server_height; int server_bpp; char mod_name[256]; int mod_mouse_state; @@ -164,8 +179,11 @@ struct vnc int suppress_output; unsigned int enabled_encodings_mask; /* Resizeable support */ + int multimon_configured; struct vnc_screen_layout client_layout; + struct vnc_screen_layout server_layout; enum vnc_resize_status resize_status; + enum vnc_resize_support_status resize_supported; }; /* diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index 0d83a09bb8..472a725042 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -64,7 +64,10 @@ struct xrdp_mod int (*mod_suppress_output)(struct xrdp_mod *v, int suppress, int left, int top, int right, int bottom); int (*mod_server_monitor_resize)(struct xrdp_mod *v, - int width, int height); + int width, int height, + int num_monitors, + const struct monitor_info *monitors, + int *in_progress); int (*mod_server_monitor_full_invalidate)(struct xrdp_mod *v, int width, int height); int (*mod_server_version_message)(struct xrdp_mod *v); @@ -106,7 +109,10 @@ struct xrdp_mod int box_left, int box_top, int box_right, int box_bottom, int x, int y, char *data, int data_len); - int (*server_reset)(struct xrdp_mod *v, int width, int height, int bpp); + int (*client_monitor_resize)(struct xrdp_mod *v, int width, int height, + int num_monitors, + const struct monitor_info *monitors); + int (*server_monitor_resize_done)(struct xrdp_mod *v); int (*server_get_channel_count)(struct xrdp_mod *v); int (*server_query_channel)(struct xrdp_mod *v, int index, char *channel_name, @@ -185,7 +191,7 @@ struct xrdp_mod int (*server_egfx_cmd)(struct xrdp_mod *v, char *cmd, int cmd_bytes, char *data, int data_bytes); - tintptr server_dumby[100 - 49]; /* align, 100 minus the number of server + tintptr server_dumby[100 - 50]; /* align, 100 minus the number of server functions above */ /* common */ tintptr handle; /* pointer to self as int */ diff --git a/xup/xup.h b/xup/xup.h index c316ab14cd..3b95124d94 100644 --- a/xup/xup.h +++ b/xup/xup.h @@ -55,7 +55,10 @@ struct mod int (*mod_suppress_output)(struct mod *v, int suppress, int left, int top, int right, int bottom); int (*mod_server_monitor_resize)(struct mod *v, - int width, int height); + int width, int height, + int num_monitors, + const struct monitor_info *monitors, + int *in_progress); int (*mod_server_monitor_full_invalidate)(struct mod *v, int width, int height); int (*mod_server_version_message)(struct mod *v); @@ -94,7 +97,10 @@ struct mod int box_left, int box_top, int box_right, int box_bottom, int x, int y, char *data, int data_len); - int (*server_reset)(struct mod *v, int width, int height, int bpp); + int (*client_monitor_resize)(struct mod *v, int width, int height, + int num_monitors, + const struct monitor_info *monitors); + int (*server_monitor_resize_done)(struct mod *v); int (*server_get_channel_count)(struct mod *v); int (*server_query_channel)(struct mod *v, int index, char *channel_name, @@ -170,7 +176,7 @@ struct mod int (*server_egfx_cmd)(struct mod *v, char *cmd, int cmd_bytes, char *data, int data_bytes); - tintptr server_dumby[100 - 49]; /* align, 100 minus the number of server + tintptr server_dumby[100 - 50]; /* align, 100 minus the number of server functions above */ /* common */ tintptr handle; /* pointer to self as long */ From bedaf990ac30bebbb5b172ae1b10abdea2d4e01b Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 14:57:59 +0000 Subject: [PATCH 05/12] Update neutrinordp module Neutrinordp module now compiles with updated monitor resize interface --- neutrinordp/xrdp-neutrinordp.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/neutrinordp/xrdp-neutrinordp.c b/neutrinordp/xrdp-neutrinordp.c index 673275ef49..6574c3facb 100644 --- a/neutrinordp/xrdp-neutrinordp.c +++ b/neutrinordp/xrdp-neutrinordp.c @@ -877,8 +877,12 @@ lxrdp_server_version_message(struct mod *mod) /******************************************************************************/ static int -lxrdp_server_monitor_resize(struct mod *mod, int width, int height) +lxrdp_server_monitor_resize(struct mod *mod, int width, int height, + int num_monitors, + const struct monitor_info *monitors, + int *in_progress) { + *in_progress = 0; return 0; } From e27c634970bce6d3d01cc024b4a1c40600158480 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:57:47 +0000 Subject: [PATCH 06/12] Update VNC module resize functionality Significant updates for the VNC module:- 1) Support for the new API calls allowing both server and client multi-monitor resizes. 2) The s member variable of the vnc_screen_layout structure is no longer dynamically allocated. 3) The module server_width and server_height member variables are removed as these are just duplicating server_layout.total_width and server_layout.total_height. 4) When the server screens are resized, there is no need to restart the entire resize state machine as we already know at this point that the server supports resizing. --- vnc/vnc.c | 432 ++++++++++++++++++++++++++---------------------------- vnc/vnc.h | 2 +- 2 files changed, 205 insertions(+), 229 deletions(-) diff --git a/vnc/vnc.c b/vnc/vnc.c index d9b0520fc2..ce53fbfca3 100644 --- a/vnc/vnc.c +++ b/vnc/vnc.c @@ -149,17 +149,13 @@ log_screen_layout(const enum logLevels lvl, const char *source, * @param a First structure * @param b Second structure * - * @return Suitable for sorting structures with ID as the primary key + * @return Suitable for sorting structures on (x, y, width, height) */ static int cmp_vnc_screen(const struct vnc_screen *a, const struct vnc_screen *b) { int result = 0; - if (a->id != b->id) - { - result = a->id - b->id; - } - else if (a->x != b->x) + if (a->x != b->x) { result = a->x - b->x; } @@ -211,9 +207,7 @@ static int vnc_screen_layouts_equal(const struct vnc_screen_layout *a, * @return != 0 for error * * @pre The next octet read from v->trans is the number of screens - * @pre layout is not already allocated * - * @post if call is successful, layout->s must be freed after use. * @post Returned structure is in increasing ID order * @post layout->total_width is untouched * @post layout->total_height is untouched @@ -225,10 +219,8 @@ read_extended_desktop_size_rect(struct vnc *v, struct stream *s; int error; unsigned int count; - struct vnc_screen *screens; layout->count = 0; - layout->s = NULL; make_stream(s); init_stream(s, 8192); @@ -239,46 +231,41 @@ read_extended_desktop_size_rect(struct vnc *v, { /* Get the number of screens */ in_uint8(s, count); - in_uint8s(s, 3); - - error = trans_force_read_s(v->trans, s, 16 * count); - if (error == 0) + if (count <= 0 || count > CLIENT_MONITOR_DATA_MAXIMUM_MONITORS) { - screens = g_new(struct vnc_screen, count); - if (screens == NULL) - { - LOG(LOG_LEVEL_ERROR, - "VNC : Can't alloc for %d screens", count); - error = 1; - } - else + LOG(LOG_LEVEL_ERROR, + "Bad monitor count %d in ExtendedDesktopSize rectangle", + count); + error = 1; + } + else + { + in_uint8s(s, 3); + + error = trans_force_read_s(v->trans, s, 16 * count); + if (error == 0) { unsigned int i; for (i = 0 ; i < count ; ++i) { - in_uint32_be(s, screens[i].id); - in_uint16_be(s, screens[i].x); - in_uint16_be(s, screens[i].y); - in_uint16_be(s, screens[i].width); - in_uint16_be(s, screens[i].height); - in_uint32_be(s, screens[i].flags); + in_uint32_be(s, layout->s[i].id); + in_uint16_be(s, layout->s[i].x); + in_uint16_be(s, layout->s[i].y); + in_uint16_be(s, layout->s[i].width); + in_uint16_be(s, layout->s[i].height); + in_uint32_be(s, layout->s[i].flags); } - /* sort monitors in increasing ID order */ - qsort(screens, count, sizeof(screens[0]), + /* sort monitors in increasing (x,y) order */ + qsort(layout->s, count, sizeof(layout->s[0]), (int (*)(const void *, const void *))cmp_vnc_screen); + layout->count = count; } } } free_stream(s); - if (error == 0) - { - layout->count = count; - layout->s = screens; - } - return error; } @@ -326,88 +313,76 @@ send_set_desktop_size(struct vnc *v, const struct vnc_screen_layout *layout) } /**************************************************************************//** - * Sets up a single-screen vnc_screen_layout structure - * - * @param layout Structure to set up - * @param width New client width - * @param height New client height - * - * @pre layout->count must be valid - * @pre layout->s must be valid + * Initialises a vnc_screen_layout as a single screen + * @param[in] width Screen Width + * @param[in] height Screen Height + * @param[out] layout Layout to initialise */ static void -set_single_screen_layout(struct vnc_screen_layout *layout, - int width, int height) +init_single_screen_layout(int width, int height, + struct vnc_screen_layout *layout) { - int id = 0; - int flags = 0; - layout->total_width = width; layout->total_height = height; - - if (layout->count == 0) - { - /* No previous layout */ - layout->s = g_new(struct vnc_screen, 1); - } - else - { - /* Keep the ID and flags from the previous first screen */ - id = layout->s[0].id; - flags = layout->s[0].flags; - - if (layout->count > 1) - { - g_free(layout->s); - layout->s = g_new(struct vnc_screen, 1); - } - } layout->count = 1; - layout->s[0].id = id; + layout->s[0].id = 0; layout->s[0].x = 0; layout->s[0].y = 0; layout->s[0].width = width; layout->s[0].height = height; - layout->s[0].flags = flags; + layout->s[0].flags = 0; } /**************************************************************************//** - * Resize the client as a single screen + * Resize the client to match the server_layout * * @param v VNC object * @param update_in_progress True if there's a painter update in progress - * @param width New client width - * @param height New client height * @return != 0 for error * - * The new client layout is recorded in v->client_layout. If the client was - * multi-screen before this call, it won't be afterwards. + * The new client layout is recorded in v->client_layout. */ static int -resize_client(struct vnc *v, int update_in_progress, int width, int height) +resize_client_to_server(struct vnc *v, int update_in_progress) { int error = 0; + unsigned int i; + const struct vnc_screen_layout *sl = &v->server_layout; + struct monitor_info client_mons[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS] = {0}; + + if (sl->count <= 0 || + sl->count > CLIENT_MONITOR_DATA_MAXIMUM_MONITORS) + { + LOG(LOG_LEVEL_ERROR, "%s: Programming error. Bad monitors %d", + __func__, sl->count); + return 1; + } - if (v->client_layout.count != 1 || - v->client_layout.total_width != width || - v->client_layout.total_height != height) + // Convert the server monitors into client monitors + for (i = 0; i < sl->count; ++i) { - if (update_in_progress) + client_mons[i].left = sl->s[i].x; + client_mons[i].top = sl->s[i].y; + client_mons[i].right = sl->s[i].x + sl->s[i].width - 1; + client_mons[i].bottom = sl->s[i].y + sl->s[i].height - 1; + } + + if (update_in_progress && v->server_end_update(v) != 0) + { + error = 1; + } + else + { + error = v->client_monitor_resize(v, sl->total_width, sl->total_height, + sl->count, client_mons); + if (error == 0) { - error = v->server_end_update(v); + v->client_layout = *sl; } - if (error == 0) + if (update_in_progress && v->server_begin_update(v) != 0) { - error = v->server_reset(v, width, height, v->server_bpp); - if (error == 0) - { - set_single_screen_layout(&v->client_layout, width, height); - if (update_in_progress) - { - error = v->server_begin_update(v); - } - } + error = 1; } } @@ -416,50 +391,53 @@ resize_client(struct vnc *v, int update_in_progress, int width, int height) /**************************************************************************//** - * Resize the attached client from a layout + * Resize the server to the client layout * * @param v VNC object - * @param update_in_progress True if there's a painter update in progress - * @param layout Desired layout from server * @return != 0 for error * - * This has some limitations. We have no way to move multiple screens about - * on a connected client, and so we are not able to change the client unless - * we're changing to a single screen layout. + * The new client layout is recorded in v->client_layout. */ static int -resize_client_from_layout(struct vnc *v, - int update_in_progress, - const struct vnc_screen_layout *layout) +resize_server_to_client_layout(struct vnc *v) { int error = 0; - if (!vnc_screen_layouts_equal(&v->client_layout, layout)) + if (v->resize_supported != VRSS_SUPPORTED) + { + LOG(LOG_LEVEL_ERROR, "%s: Asked to resize server, but not possible", + __func__); + error = 1; + } + else if (vnc_screen_layouts_equal(&v->server_layout, &v->client_layout)) + { + LOG(LOG_LEVEL_DEBUG, "Server layout is the same " + "as the client layout"); + v->resize_status = VRS_DONE; + } + else { /* - * we don't have the capability to resize to anything other - * than a single screen. + * If we've only got one screen, and the other side has + * only got one screen, we will preserve their screen ID + * and any flags. This may prevent us sending an unwanted + * SetDesktopSize message if the screen dimensions are + * a match. We can't do this with more than one screen, + * as we have no way to map different IDs */ - if (layout->count != 1) - { - LOG(LOG_LEVEL_ERROR, - "VNC Resize to %d screen(s) from %d screen(s) " - "not implemented", - v->client_layout.count, layout->count); - - /* Dump some useful info, in case we get here when we don't - * need to */ - log_screen_layout(LOG_LEVEL_ERROR, "OldLayout", &v->client_layout); - log_screen_layout(LOG_LEVEL_ERROR, "NewLayout", layout); - error = 1; - } - else + if (v->server_layout.count == 1 && v->client_layout.count == 1) { - error = resize_client(v, - update_in_progress, - layout->total_width, - layout->total_height); + LOG(LOG_LEVEL_DEBUG, "VNC " + "setting screen id to %d from server", + v->server_layout.s[0].id); + + v->client_layout.s[0].id = v->server_layout.s[0].id; + v->client_layout.s[0].flags = v->server_layout.s[0].flags; } + + LOG(LOG_LEVEL_DEBUG, "Changing server layout"); + error = send_set_desktop_size(v, &v->client_layout); + v->resize_status = VRS_WAITING_FOR_RESIZE_CONFIRM; } return error; @@ -927,7 +905,6 @@ skip_encoding(struct vnc *v, int x, int y, int cx, int cy, "x=%d, y=%d geom=%dx%d", x, y, cx, cy); error = read_extended_desktop_size_rect(v, &layout); - g_free(layout.s); } break; @@ -944,7 +921,7 @@ skip_encoding(struct vnc *v, int x, int y, int cx, int cy, * Parses an entire framebuffer update message from the wire, and returns the * first matching ExtendedDesktopSize encoding if found. * - * Caller can check for a match by examining match_layout.s after the call + * Caller can check for a match by examining match_layout.count after the call * * @param v VNC object * @param match Function to call to check for a match @@ -952,8 +929,6 @@ skip_encoding(struct vnc *v, int x, int y, int cx, int cy, * @param [out] match_y Matching y parameter for an encoding (if needed) * @param [out] match_layout Returned layout for the encoding * @return != 0 for error - * - * @post After a successful call, match_layout.s must be free'd */ static int find_matching_extended_rect(struct vnc *v, @@ -971,8 +946,7 @@ find_matching_extended_rect(struct vnc *v, int cx; int cy; encoding_type encoding; - - match_layout->s = NULL; + int found = 0; make_stream(s); init_stream(s, 8192); @@ -1002,14 +976,14 @@ find_matching_extended_rect(struct vnc *v, in_uint32_be(s, encoding); if (encoding == RFB_ENC_EXTENDED_DESKTOP_SIZE && - match_layout->s == NULL && + !found && match(x, y, cx, cy)) { LOG(LOG_LEVEL_DEBUG, "VNC matched ExtendedDesktopSize rectangle " "x=%d, y=%d geom=%dx%d", x, y, cx, cy); - + found = 1; error = read_extended_desktop_size_rect(v, match_layout); if (match_x) { @@ -1065,6 +1039,7 @@ send_update_request_for_resize_status(struct vnc *v) switch (v->resize_status) { case VRS_WAITING_FOR_FIRST_UPDATE: + case VRS_WAITING_FOR_RESIZE_CONFIRM: /* * Ask for an immediate, minimal update. */ @@ -1078,20 +1053,6 @@ send_update_request_for_resize_status(struct vnc *v) error = lib_send_copy(v, s); break; - case VRS_WAITING_FOR_RESIZE_CONFIRM: - /* - * Ask for a deferred minimal update. - */ - out_uint8(s, RFB_C2S_FRAMEBUFFER_UPDATE_REQUEST); - out_uint8(s, 1); /* incremental == 1 : Changes only */ - out_uint16_be(s, 0); - out_uint16_be(s, 0); - out_uint16_be(s, 1); - out_uint16_be(s, 1); - s_mark_end(s); - error = lib_send_copy(v, s); - break; - default: /* * Ask for a full update from the server @@ -1102,8 +1063,8 @@ send_update_request_for_resize_status(struct vnc *v) out_uint8(s, 0); /* incremental == 0 : Full update */ out_uint16_be(s, 0); out_uint16_be(s, 0); - out_uint16_be(s, v->server_width); - out_uint16_be(s, v->server_height); + out_uint16_be(s, v->server_layout.total_width); + out_uint16_be(s, v->server_layout.total_height); s_mark_end(s); error = lib_send_copy(v, s); } @@ -1159,12 +1120,15 @@ lib_framebuffer_first_update(struct vnc *v) &layout); if (error == 0) { - if (layout.s != NULL) + if (layout.count > 0) { LOG(LOG_LEVEL_DEBUG, "VNC server supports resizing"); + v->resize_supported = VRSS_SUPPORTED; + v->server_layout = layout; /* Force the client geometry over to the server */ - log_screen_layout(LOG_LEVEL_INFO, "OldLayout", &layout); + log_screen_layout(LOG_LEVEL_INFO, "ClientLayout", &v->client_layout); + log_screen_layout(LOG_LEVEL_INFO, "OldServerLayout", &layout); /* * If we've only got one screen, and the other side has @@ -1184,32 +1148,19 @@ lib_framebuffer_first_update(struct vnc *v) v->client_layout.s[0].flags = layout.s[0].flags; } - if (vnc_screen_layouts_equal(&layout, &v->client_layout)) - { - LOG(LOG_LEVEL_DEBUG, "Server layout is the same " - "as the client layout"); - v->resize_status = VRS_DONE; - } - else - { - LOG(LOG_LEVEL_DEBUG, "Server layout differs from " - "the client layout. Changing server layout"); - error = send_set_desktop_size(v, &v->client_layout); - v->resize_status = VRS_WAITING_FOR_RESIZE_CONFIRM; - } + resize_server_to_client_layout(v); } else { LOG(LOG_LEVEL_DEBUG, "VNC server does not support resizing"); + v->resize_supported = VRSS_NOT_SUPPORTED; /* Force client to same size as server */ LOG(LOG_LEVEL_DEBUG, "Resizing client to server %dx%d", - v->server_width, v->server_height); - error = resize_client(v, 0, v->server_width, v->server_height); + v->server_layout.total_width, v->server_layout.total_height); + error = resize_client_to_server(v, 0); v->resize_status = VRS_DONE; } - - g_free(layout.s); } if (error == 0) @@ -1224,7 +1175,7 @@ lib_framebuffer_first_update(struct vnc *v) * Looks for a resize confirm in a framebuffer update request * * If the server supports resizes from us, this is used to find the - * reply to our initial resize request. See The RFB community wiki for details. + * reply to our resize request. See The RFB community wiki for details. * * @param v VNC object * @return != 0 for error @@ -1243,18 +1194,17 @@ lib_framebuffer_waiting_for_resize_confirm(struct vnc *v) &layout); if (error == 0) { - if (layout.s != NULL) + if (layout.count > 0) { if (response_code == 0) { LOG(LOG_LEVEL_DEBUG, "VNC server successfully resized"); log_screen_layout(LOG_LEVEL_INFO, "NewLayout", &layout); + v->server_layout = layout; // If this resize was requested by the client mid-session // (dynamic resize), we need to tell xrdp_mm that // it's OK to continue with the resize state machine. - // We do this by sending a reset with bpp == 0 - error = v->server_reset(v, v->server_width, - v->server_height, 0); + error = v->server_monitor_resize_done(v); } else { @@ -1263,14 +1213,11 @@ lib_framebuffer_waiting_for_resize_confirm(struct vnc *v) response_code, rfb_get_eds_status_msg(response_code)); /* Force client to same size as server */ - LOG(LOG_LEVEL_WARNING, "Resizing client to server %dx%d", - v->server_width, v->server_height); - error = resize_client(v, 0, v->server_width, v->server_height); + LOG(LOG_LEVEL_WARNING, "Resizing client to server"); + error = resize_client_to_server(v, 0); } v->resize_status = VRS_DONE; } - - g_free(layout.s); } if (error == 0) @@ -1415,9 +1362,8 @@ lib_framebuffer_update(struct vnc *v) else if (encoding == RFB_ENC_DESKTOP_SIZE) { /* Server end has resized */ - v->server_width = cx; - v->server_height = cy; - error = resize_client(v, 1, cx, cy); + init_single_screen_layout(cx, cy, &v->server_layout); + error = resize_client_to_server(v, 1); } else if (encoding == RFB_ENC_EXTENDED_DESKTOP_SIZE) { @@ -1427,11 +1373,14 @@ lib_framebuffer_update(struct vnc *v) /* If this is a reply to a request from us, x == 1 */ if (error == 0 && x != 1) { - v->server_width = layout.total_width; - v->server_height = layout.total_height; - error = resize_client_from_layout(v, 1, &layout); + if (!vnc_screen_layouts_equal(&v->server_layout, &layout)) + { + v->server_layout = layout; + log_screen_layout(LOG_LEVEL_INFO, "NewServerLayout", + &v->server_layout); + error = resize_client_to_server(v, 1); + } } - g_free(layout.s); } else { @@ -1456,8 +1405,8 @@ lib_framebuffer_update(struct vnc *v) out_uint8(s, 1); /* incremental == 1 : Changes only */ out_uint16_be(s, 0); out_uint16_be(s, 0); - out_uint16_be(s, v->server_width); - out_uint16_be(s, v->server_height); + out_uint16_be(s, v->server_layout.total_width); + out_uint16_be(s, v->server_layout.total_height); s_mark_end(s); error = lib_send_copy(v, s); } @@ -1831,8 +1780,11 @@ lib_mod_connect(struct vnc *v) if (error == 0) { - in_uint16_be(s, v->server_width); - in_uint16_be(s, v->server_height); + int width; + int height; + in_uint16_be(s, width); + in_uint16_be(s, height); + init_single_screen_layout(width, height, &v->server_layout); init_stream(pixel_format, 8192); v->server_msg(v, "VNC receiving pixel format", 0); @@ -2000,6 +1952,7 @@ lib_mod_connect(struct vnc *v) if (error == 0) { + v->resize_supported = VRSS_UNKNOWN; v->resize_status = VRS_WAITING_FOR_FIRST_UPDATE; error = send_update_request_for_resize_status(v); } @@ -2061,33 +2014,40 @@ lib_mod_end(struct vnc *v) /**************************************************************************//** * Initialises the client layout from the Windows monitor definition. * - * @param [out] layout Our layout - * @param [in] client_info WM info + * @param v VNC module + * @param [in] width session width + * @param [in] height session height + * @param [in] num_monitors (can be 0, meaning one monitor) + * @param [in] monitors Monitor definitions for num_monitors > 0 + * @param [in] multimon_configured Whether multimon is configured */ static void -init_client_layout(struct vnc_screen_layout *layout, - const struct xrdp_client_info *client_info) +init_client_layout(struct vnc *v, + int width, int height, + int num_monitors, + const struct monitor_info *monitors) { - uint32_t i; - - layout->total_width = client_info->display_sizes.session_width; - layout->total_height = client_info->display_sizes.session_height; - - layout->count = client_info->display_sizes.monitorCount; - layout->s = g_new(struct vnc_screen, layout->count); - - for (i = 0 ; i < client_info->display_sizes.monitorCount ; ++i) + struct vnc_screen_layout *layout = &v->client_layout; + if (!v->multimon_configured || num_monitors < 1) { - /* Use minfo_wm, as this is normalised for a top-left of (0,0) - * as required by RFC6143 */ - layout->s[i].id = i; - layout->s[i].x = client_info->display_sizes.minfo_wm[i].left; - layout->s[i].y = client_info->display_sizes.minfo_wm[i].top; - layout->s[i].width = client_info->display_sizes.minfo_wm[i].right - - client_info->display_sizes.minfo_wm[i].left + 1; - layout->s[i].height = client_info->display_sizes.minfo_wm[i].bottom - - client_info->display_sizes.minfo_wm[i].top + 1; - layout->s[i].flags = 0; + init_single_screen_layout(width, height, layout); + } + else + { + layout->total_width = width; + layout->total_height = height; + layout->count = num_monitors; + + unsigned int i; + for (i = 0 ; i < layout->count; ++i) + { + layout->s[i].id = i; + layout->s[i].x = monitors[i].left; + layout->s[i].y = monitors[i].top; + layout->s[i].width = monitors[i].right - monitors[i].left + 1; + layout->s[i].height = monitors[i].bottom - monitors[i].top + 1; + layout->s[i].flags = 0; + } } } @@ -2132,20 +2092,16 @@ lib_mod_set_param(struct vnc *v, const char *name, const char *value) const struct xrdp_client_info *client_info = (const struct xrdp_client_info *) value; - g_free(v->client_layout.s); - v->client_layout.count = 0; + v->multimon_configured = client_info->multimon; - /* Save monitor information from the client */ - if (!client_info->multimon || client_info->display_sizes.monitorCount < 1) - { - set_single_screen_layout(&v->client_layout, - client_info->display_sizes.session_width, - client_info->display_sizes.session_height); - } - else - { - init_client_layout(&v->client_layout, client_info); - } + /* Save monitor information from the client + * Use minfo_wm, as this is normalised for a top-left of (0,0) + * as required by RFC6143 */ + init_client_layout(v, + client_info->display_sizes.session_width, + client_info->display_sizes.session_height, + client_info->display_sizes.monitorCount, + client_info->display_sizes.minfo_wm); log_screen_layout(LOG_LEVEL_DEBUG, "client_info", &v->client_layout); } @@ -2186,6 +2142,10 @@ lib_mod_check_wait_objs(struct vnc *v) if (v->trans != 0) { rv = trans_check_wait_objs(v->trans); + if (rv != 0) + { + LOG(LOG_LEVEL_ERROR, "VNC server closed connection"); + } } } return rv; @@ -2218,8 +2178,8 @@ lib_mod_suppress_output(struct vnc *v, int suppress, out_uint8(s, 0); /* incremental == 0 : Full contents */ out_uint16_be(s, 0); out_uint16_be(s, 0); - out_uint16_be(s, v->server_width); - out_uint16_be(s, v->server_height); + out_uint16_be(s, v->server_layout.total_width); + out_uint16_be(s, v->server_layout.total_height); s_mark_end(s); error = lib_send_copy(v, s); free_stream(s); @@ -2238,12 +2198,29 @@ lib_mod_server_version_message(struct vnc *v) /******************************************************************************/ /* return error */ int -lib_mod_server_monitor_resize(struct vnc *v, int width, int height) +lib_mod_server_monitor_resize(struct vnc *v, int width, int height, + int num_monitors, + const struct monitor_info *monitors, + int *in_progress) { - int error = 0; - set_single_screen_layout(&v->client_layout, width, height); - v->resize_status = VRS_WAITING_FOR_FIRST_UPDATE; - error = send_update_request_for_resize_status(v); + int error; + *in_progress = 0; + init_client_layout(v, width, height, num_monitors, monitors); + + if ((error = resize_server_to_client_layout(v)) == 0) + { + // If we're waiting for a confirmation, send an update request. + // According to the spec this should not be needed, but + // it works around a buggy VNC server not sending an + // ExtendedDesktopSize rectangle if the desktop change is + // small (eg. same dimensions, but 2 monitors -> 1 monitor) + if (v->resize_status == VRS_WAITING_FOR_RESIZE_CONFIRM && + (error = send_update_request_for_resize_status(v)) == 0) + { + *in_progress = 1; + } + } + return error; } @@ -2299,7 +2276,6 @@ mod_exit(tintptr handle) return 0; } trans_delete(v->trans); - g_free(v->client_layout.s); vnc_clip_exit(v); g_free(v); return 0; diff --git a/vnc/vnc.h b/vnc/vnc.h index 22c32c44d5..39a19f22d5 100644 --- a/vnc/vnc.h +++ b/vnc/vnc.h @@ -47,7 +47,7 @@ struct vnc_screen_layout int total_width; int total_height; unsigned int count; - /* For comparison, screens are sorted in increasing order of ID */ + /* For comparison, screens are sorted in x, y, width, height) order */ struct vnc_screen s[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; }; From dd377201880232135182bc4e4cef52f4bacd639b Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 16:04:49 +0000 Subject: [PATCH 07/12] Change to the XUP module for the new resizing interface Input message 300 to xorgxrdp which communicated a screen size has been replaced with message 302 which sends a list of monitors to xorgxrdp. This simplifies the initialisation and resize logic somewhat, but a compatible version of xorgxrdp must be used --- xup/xup.c | 56 +++++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/xup/xup.c b/xup/xup.c index b9c17d7443..fc96c58f0f 100644 --- a/xup/xup.c +++ b/xup/xup.c @@ -28,8 +28,10 @@ #include "string_calls.h" static int -send_server_monitor_resize( - struct mod *mod, struct stream *s, int width, int height, int bpp); +send_server_monitor_update(struct mod *v, struct stream *s, + int width, int height, + int num_monitors, + const struct monitor_info *monitors); static int send_server_monitor_full_invalidate( @@ -224,13 +226,6 @@ lib_mod_connect(struct mod *mod) error = send_server_version_message(mod, s); } - if (error == 0) - { - /* send screen size message */ - error = send_server_monitor_resize( - mod, s, mod->width, mod->height, mod->bpp); - } - if (error == 0) { error = send_server_monitor_full_invalidate( @@ -1466,33 +1461,30 @@ send_server_version_message(struct mod *mod, struct stream *s) /******************************************************************************/ /* return error */ static int -send_server_monitor_resize( - struct mod *mod, struct stream *s, int width, int height, int bpp) +send_server_monitor_update(struct mod *mod, struct stream *s, + int width, int height, + int num_monitors, + const struct monitor_info *monitors) { - /* send screen size message */ + /* send monitor update message */ init_stream(s, 8192); s_push_layer(s, iso_hdr, 4); out_uint16_le(s, 103); - out_uint32_le(s, 300); + out_uint32_le(s, 302); out_uint32_le(s, width); out_uint32_le(s, height); - /* - TODO: The bpp here is only necessary for initial creation. We should - modify XUP to require this only on server initialization, but not on - resize. Microsoft's RDP protocol does not support changing - the bpp on resize. - */ - out_uint32_le(s, bpp); + out_uint32_le(s, num_monitors); out_uint32_le(s, 0); + out_uint8a(s, monitors, sizeof(monitors[0]) * num_monitors); s_mark_end(s); int len = (int)(s->end - s->data); s_pop_layer(s, iso_hdr); out_uint32_le(s, len); int rv = lib_send_copy(mod, s); - LOG_DEVEL(LOG_LEVEL_DEBUG, "send_server_monitor_resize:" - " sent resize message with following properties to" - " xorgxrdp backend width=%d, height=%d, bpp=%d, return value=%d", - width, height, bpp, rv); + LOG_DEVEL(LOG_LEVEL_DEBUG, "send_server_monitor_update:" + " sent monitor updsate message with following properties to" + " xorgxrdp backend width=%d, height=%d, num=%d, return value=%d", + width, height, num_monitors, rv); return rv; } @@ -1542,12 +1534,16 @@ lib_send_server_version_message(struct mod *mod) /******************************************************************************/ /* return error */ static int -lib_send_server_monitor_resize(struct mod *mod, int width, int height) +lib_send_server_monitor_resize(struct mod *mod, int width, int height, + int num_monitors, + const struct monitor_info *monitors, + int *in_progress) { - /* send screen size message */ struct stream *s; make_stream(s); - int rv = send_server_monitor_resize(mod, s, width, height, mod->bpp); + int rv = send_server_monitor_update(mod, s, width, height, + num_monitors, monitors); + *in_progress = (rv == 0); free_stream(s); return rv; } @@ -1802,7 +1798,7 @@ lib_mod_process_message(struct mod *mod, struct stream *s) LOG(LOG_LEVEL_INFO, "Received memory_allocation_complete" " command. width: %d, height: %d", width, height); - rv = mod->server_reset(mod, width, height, 0); + rv = mod->server_monitor_resize_done(mod); break; } s->p = phold + len; @@ -1897,6 +1893,10 @@ lib_mod_check_wait_objs(struct mod *mod) if (mod->trans != 0) { rv = trans_check_wait_objs(mod->trans); + if (rv != 0) + { + LOG(LOG_LEVEL_ERROR, "Xorg server closed connection"); + } } } From bc70a86de6139f6effe4f1e20be39188e9bde172 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 16:20:04 +0000 Subject: [PATCH 08/12] Rework xrdp to support new module resize interface This commit compiles. --- libxrdp/libxrdp.c | 303 +++++++++++++++++++++++-------------------- libxrdp/libxrdpinc.h | 21 ++- xrdp/xrdp.h | 5 +- xrdp/xrdp_mm.c | 171 ++++++++++++------------ xrdp/xrdp_types.h | 3 - 5 files changed, 270 insertions(+), 233 deletions(-) diff --git a/libxrdp/libxrdp.c b/libxrdp/libxrdp.c index 400de49664..f9009162b6 100644 --- a/libxrdp/libxrdp.c +++ b/libxrdp/libxrdp.c @@ -1136,34 +1136,10 @@ libxrdp_orders_send_font(struct xrdp_session *session, } /*****************************************************************************/ -/* Note : if this is called on a multimon setup, the client is resized - * to a single monitor */ int EXPORT_CC -libxrdp_reset(struct xrdp_session *session, - unsigned int width, unsigned int height, int bpp) +libxrdp_reset(struct xrdp_session *session) { LOG_DEVEL(LOG_LEVEL_TRACE, "libxrdp_reset:"); - if (session->client_info != 0) - { - struct xrdp_client_info *client_info = session->client_info; - - /* older client can't resize */ - if (client_info->build <= 419) - { - return 0; - } - - client_info->display_sizes.session_width = width; - client_info->display_sizes.session_height = height; - client_info->display_sizes.monitorCount = 0; - client_info->bpp = bpp; - client_info->multimon = 0; - } - else - { - LOG(LOG_LEVEL_ERROR, "libxrdp_reset: session->client_info is NULL"); - return 1; - } /* this will send any lingering orders */ if (xrdp_orders_reset((struct xrdp_orders *)session->orders) != 0) @@ -1792,6 +1768,19 @@ libxrdp_send_session_info(struct xrdp_session *session, const char *data, static void sanitise_extended_monitor_attributes(struct monitor_info *monitor_layout) { + if (monitor_layout->physical_width == 0 + && monitor_layout->physical_width == 0 + && monitor_layout->orientation == 0 + && monitor_layout->desktop_scale_factor == 0 + && monitor_layout->device_scale_factor == 0) + { + /* Module expects us to provide defaults */ + monitor_layout->orientation = ORIENTATION_LANDSCAPE; + monitor_layout->desktop_scale_factor = 100; + monitor_layout->device_scale_factor = 100; + return; + } + /* if EITHER physical_width or physical_height are * out of range, BOTH must be ignored. */ @@ -1882,16 +1871,16 @@ libxrdp_process_monitor_stream(struct stream *s, { uint32_t num_monitor; uint32_t monitor_index; + struct monitor_info monitors[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; struct monitor_info *monitor_layout; - struct xrdp_rect all_monitors_encompassing_bounds = {0}; - int got_primary = 0; int monitor_struct_stream_check_bytes; const char *monitor_struct_stream_check_message; LOG_DEVEL(LOG_LEVEL_TRACE, "libxrdp_process_monitor_stream:"); if (description == NULL) { - LOG_DEVEL(LOG_LEVEL_ERROR, "libxrdp_process_monitor_stream: description was" + LOG_DEVEL(LOG_LEVEL_ERROR, + "libxrdp_process_monitor_stream: description was" " null. Valid pointer to allocated description expected."); return SEC_PROCESS_MONITORS_ERR; } @@ -1939,7 +1928,7 @@ libxrdp_process_monitor_stream(struct stream *s, " from [MS-RDPEDISP] 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT."; } - description->monitorCount = num_monitor; + memset(monitors, 0, sizeof(monitors[0]) * num_monitor); for (monitor_index = 0; monitor_index < num_monitor; ++monitor_index) { @@ -1951,7 +1940,8 @@ libxrdp_process_monitor_stream(struct stream *s, return SEC_PROCESS_MONITORS_ERR; } - monitor_layout = description->minfo + monitor_index; + monitor_layout = &monitors[monitor_index]; + if (full_parameters != 0) { in_uint32_le(s, monitor_layout->flags); @@ -2017,8 +2007,6 @@ libxrdp_process_monitor_stream(struct stream *s, in_uint32_le(s, monitor_layout->desktop_scale_factor); in_uint32_le(s, monitor_layout->device_scale_factor); - sanitise_extended_monitor_attributes(monitor_layout); - /* * 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT */ @@ -2047,6 +2035,138 @@ libxrdp_process_monitor_stream(struct stream *s, monitor_layout->is_primary = TS_MONITOR_PRIMARY; } } + } + + return libxrdp_init_display_size_description( + num_monitor, monitors, description); +} + +/*****************************************************************************/ +int +libxrdp_process_monitor_ex_stream(struct stream *s, + struct display_size_description *description) +{ + uint32_t num_monitor; + uint32_t monitor_index; + uint32_t attribute_size; + struct monitor_info *monitor_layout; + + LOG_DEVEL(LOG_LEVEL_TRACE, "libxrdp_process_monitor_ex_stream:"); + if (description == NULL) + { + LOG_DEVEL(LOG_LEVEL_ERROR, "libxrdp_process_monitor_ex_stream: " + "description was null. " + " Valid pointer to allocated description expected."); + return SEC_PROCESS_MONITORS_ERR; + } + + if (!s_check_rem_and_log(s, 4, + "libxrdp_process_monitor_ex_stream:" + " Parsing [MS-RDPBCGR] TS_UD_CS_MONITOR_EX")) + { + return SEC_PROCESS_MONITORS_ERR; + } + + in_uint32_le(s, attribute_size); + if (attribute_size != TS_MONITOR_ATTRIBUTES_SIZE) + { + LOG(LOG_LEVEL_ERROR, + "libxrdp_process_monitor_ex_stream: [MS-RDPBCGR] Protocol" + " error: TS_UD_CS_MONITOR_EX monitorAttributeSize" + " MUST be %d, received: %d", + TS_MONITOR_ATTRIBUTES_SIZE, attribute_size); + return SEC_PROCESS_MONITORS_ERR; + } + + in_uint32_le(s, num_monitor); + LOG(LOG_LEVEL_DEBUG, "libxrdp_process_monitor_ex_stream:" + " The number of monitors received is: %d", + num_monitor); + + if (num_monitor != description->monitorCount) + { + LOG(LOG_LEVEL_ERROR, + "libxrdp_process_monitor_ex_stream: [MS-RDPBCGR] Protocol" + " error: TS_UD_CS_MONITOR monitorCount" + " MUST be %d, received: %d", + description->monitorCount, num_monitor); + return SEC_PROCESS_MONITORS_ERR; + } + + for (monitor_index = 0; monitor_index < num_monitor; ++monitor_index) + { + if (!s_check_rem_and_log(s, attribute_size, + "libxrdp_process_monitor_ex_stream:" + " Parsing TS_UD_CS_MONITOR_EX")) + { + return SEC_PROCESS_MONITORS_ERR; + } + + monitor_layout = description->minfo + monitor_index; + + in_uint32_le(s, monitor_layout->physical_width); + in_uint32_le(s, monitor_layout->physical_height); + in_uint32_le(s, monitor_layout->orientation); + in_uint32_le(s, monitor_layout->desktop_scale_factor); + in_uint32_le(s, monitor_layout->device_scale_factor); + + sanitise_extended_monitor_attributes(monitor_layout); + + LOG_DEVEL(LOG_LEVEL_INFO, "libxrdp_process_monitor_ex_stream:" + " Received [MS-RDPBCGR] 2.2.1.3.9.1 " + " TS_MONITOR_ATTRIBUTES" + " Index: %d, PhysicalWidth %d, PhysicalHeight %d," + " Orientation %d, DesktopScaleFactor %d," + " DeviceScaleFactor %d", + monitor_index, + monitor_layout->physical_width, + monitor_layout->physical_height, + monitor_layout->orientation, + monitor_layout->desktop_scale_factor, + monitor_layout->device_scale_factor); + } + + /* Update non negative monitor info values */ + const struct monitor_info *src = description->minfo; + struct monitor_info *dst = description->minfo_wm; + for (monitor_index = 0; monitor_index < num_monitor; ++monitor_index) + { + dst->physical_width = src->physical_width; + dst->physical_height = src->physical_height; + dst->orientation = src->orientation; + dst->desktop_scale_factor = src->desktop_scale_factor; + dst->device_scale_factor = src->device_scale_factor; + ++src; + ++dst; + } + + return 0; +} + +/*****************************************************************************/ +int +libxrdp_init_display_size_description( + unsigned int num_monitor, + const struct monitor_info *monitors, + struct display_size_description *description) +{ + unsigned int monitor_index; + struct monitor_info *monitor_layout; + struct xrdp_rect all_monitors_encompassing_bounds = {0}; + int got_primary = 0; + + /* Caller should have checked this, so don't log an error */ + if (num_monitor > CLIENT_MONITOR_DATA_MAXIMUM_MONITORS) + { + return SEC_PROCESS_MONITORS_ERR_TOO_MANY_MONITORS; + } + + description->monitorCount = num_monitor; + for (monitor_index = 0 ; monitor_index < num_monitor; ++monitor_index) + { + monitor_layout = &description->minfo[monitor_index]; + *monitor_layout = monitors[monitor_index]; + sanitise_extended_monitor_attributes(monitor_layout); if (monitor_index == 0) { @@ -2073,7 +2193,15 @@ libxrdp_process_monitor_stream(struct stream *s, if (monitor_layout->is_primary == TS_MONITOR_PRIMARY) { - got_primary = 1; + if (got_primary) + { + // Already got one - don't have two + monitor_layout->is_primary = 0; + } + else + { + got_primary = 1; + } } } @@ -2113,7 +2241,7 @@ libxrdp_process_monitor_stream(struct stream *s, } else { - LOG(LOG_LEVEL_ERROR, "libxrdp_process_monitor_stream:" + LOG(LOG_LEVEL_ERROR, "libxrdp_init_display_size_description:" " The area encompassing the monitors is not a" " well-formed rectangle. Received" " (top: %d, left: %d, right: %d, bottom: %d)." @@ -2138,7 +2266,7 @@ libxrdp_process_monitor_stream(struct stream *s, < CLIENT_MONITOR_DATA_MINIMUM_VIRTUAL_DESKTOP_HEIGHT) { LOG(LOG_LEVEL_ERROR, - "libxrdp_process_monitor_stream: Client supplied virtual" + "libxrdp_init_display_size_description: Calculated virtual" " desktop width or height is invalid." " Allowed width range: min %d, max %d. Width received: %d." " Allowed height range: min %d, max %d. Height received: %d", @@ -2156,9 +2284,7 @@ libxrdp_process_monitor_stream(struct stream *s, { monitor_layout = description->minfo_wm + monitor_index; - g_memcpy(monitor_layout, - description->minfo + monitor_index, - sizeof(struct monitor_info)); + *monitor_layout = description->minfo[monitor_index]; monitor_layout->left = monitor_layout->left - all_monitors_encompassing_bounds.left; @@ -2173,107 +2299,6 @@ libxrdp_process_monitor_stream(struct stream *s, return 0; } -/*****************************************************************************/ -int -libxrdp_process_monitor_ex_stream(struct stream *s, - struct display_size_description *description) -{ - uint32_t num_monitor; - uint32_t monitor_index; - uint32_t attribute_size; - struct monitor_info *monitor_layout; - - LOG_DEVEL(LOG_LEVEL_TRACE, "libxrdp_process_monitor_ex_stream:"); - if (description == NULL) - { - LOG_DEVEL(LOG_LEVEL_ERROR, "libxrdp_process_monitor_ex_stream: " - "description was null. " - " Valid pointer to allocated description expected."); - return SEC_PROCESS_MONITORS_ERR; - } - - if (!s_check_rem_and_log(s, 4, - "libxrdp_process_monitor_ex_stream:" - " Parsing [MS-RDPBCGR] TS_UD_CS_MONITOR_EX")) - { - return SEC_PROCESS_MONITORS_ERR; - } - - in_uint32_le(s, attribute_size); - if (attribute_size != TS_MONITOR_ATTRIBUTES_SIZE) - { - LOG(LOG_LEVEL_ERROR, - "libxrdp_process_monitor_ex_stream: [MS-RDPBCGR] Protocol" - " error: TS_UD_CS_MONITOR_EX monitorAttributeSize" - " MUST be %d, received: %d", - TS_MONITOR_ATTRIBUTES_SIZE, attribute_size); - return SEC_PROCESS_MONITORS_ERR; - } - - in_uint32_le(s, num_monitor); - LOG(LOG_LEVEL_DEBUG, "libxrdp_process_monitor_ex_stream:" - " The number of monitors received is: %d", - num_monitor); - - if (num_monitor != description->monitorCount) - { - LOG(LOG_LEVEL_ERROR, - "libxrdp_process_monitor_ex_stream: [MS-RDPBCGR] Protocol" - " error: TS_UD_CS_MONITOR monitorCount" - " MUST be %d, received: %d", - description->monitorCount, num_monitor); - return SEC_PROCESS_MONITORS_ERR; - } - - for (monitor_index = 0; monitor_index < num_monitor; ++monitor_index) - { - if (!s_check_rem_and_log(s, attribute_size, - "libxrdp_process_monitor_ex_stream:" - " Parsing TS_UD_CS_MONITOR_EX")) - { - return SEC_PROCESS_MONITORS_ERR; - } - - monitor_layout = description->minfo + monitor_index; - - in_uint32_le(s, monitor_layout->physical_width); - in_uint32_le(s, monitor_layout->physical_height); - in_uint32_le(s, monitor_layout->orientation); - in_uint32_le(s, monitor_layout->desktop_scale_factor); - in_uint32_le(s, monitor_layout->device_scale_factor); - - sanitise_extended_monitor_attributes(monitor_layout); - - LOG_DEVEL(LOG_LEVEL_INFO, "libxrdp_process_monitor_ex_stream:" - " Received [MS-RDPBCGR] 2.2.1.3.9.1 " - " TS_MONITOR_ATTRIBUTES" - " Index: %d, PhysicalWidth %d, PhysicalHeight %d," - " Orientation %d, DesktopScaleFactor %d," - " DeviceScaleFactor %d", - monitor_index, - monitor_layout->physical_width, - monitor_layout->physical_height, - monitor_layout->orientation, - monitor_layout->desktop_scale_factor, - monitor_layout->device_scale_factor); - } - - /* Update non negative monitor info values */ - const struct monitor_info *src = description->minfo; - struct monitor_info *dst = description->minfo_wm; - for (monitor_index = 0; monitor_index < num_monitor; ++monitor_index) - { - dst->physical_width = src->physical_width; - dst->physical_height = src->physical_height; - dst->orientation = src->orientation; - dst->desktop_scale_factor = src->desktop_scale_factor; - dst->device_scale_factor = src->device_scale_factor; - ++src; - ++dst; - } - - return 0; -} /*****************************************************************************/ int EXPORT_CC libxrdp_planar_compress(char *in_data, int width, int height, diff --git a/libxrdp/libxrdpinc.h b/libxrdp/libxrdpinc.h index 5d1c8fff47..69de7801ac 100644 --- a/libxrdp/libxrdpinc.h +++ b/libxrdp/libxrdpinc.h @@ -24,6 +24,7 @@ #include "xrdp_rail.h" struct list; +struct monitor_info; /* struct xrdp_client_info moved to xrdp_client_info.h */ @@ -195,8 +196,7 @@ libxrdp_orders_send_font(struct xrdp_session *session, struct xrdp_font_char *font_char, int font_index, int char_index); int -libxrdp_reset(struct xrdp_session *session, - unsigned int width, unsigned int height, int bpp); +libxrdp_reset(struct xrdp_session *session); int libxrdp_orders_send_raw_bitmap2(struct xrdp_session *session, int width, int height, int bpp, char *data, @@ -346,4 +346,21 @@ int EXPORT_CC libxrdp_process_monitor_ex_stream(struct stream *s, struct display_size_description *description); +/** + * Convert a list of monitors into a full description + * + * Monitor data is sanitised during the conversion + * + * @param num_monitor Monitor count (> 0) + * @param monitors List of monitors + * @param[out] description Display size description + * + * @return 0 if the data is processed, non-zero if there is an error. + */ +int +libxrdp_init_display_size_description( + unsigned int num_monitor, + const struct monitor_info *monitors, + struct display_size_description *description); + #endif diff --git a/xrdp/xrdp.h b/xrdp/xrdp.h index 6c1c4e0c52..f477b3b491 100644 --- a/xrdp/xrdp.h +++ b/xrdp/xrdp.h @@ -591,7 +591,10 @@ server_draw_text(struct xrdp_mod *mod, int font, int box_right, int box_bottom, int x, int y, char *data, int data_len); int -server_reset(struct xrdp_mod *mod, int width, int height, int bpp); +client_monitor_resize(struct xrdp_mod *mod, int width, int height, + int num_monitors, const struct monitor_info *monitors); +int +server_monitor_resize_done(struct xrdp_mod *mod); int is_channel_allowed(struct xrdp_wm *wm, int channel_id); int diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 614175cdc3..882dfbd504 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -396,7 +396,8 @@ xrdp_mm_setup_mod1(struct xrdp_mm *self) self->mod->server_draw_line = server_draw_line; self->mod->server_add_char = server_add_char; self->mod->server_draw_text = server_draw_text; - self->mod->server_reset = server_reset; + self->mod->client_monitor_resize = client_monitor_resize; + self->mod->server_monitor_resize_done = server_monitor_resize_done; self->mod->server_get_channel_count = server_get_channel_count; self->mod->server_query_channel = server_query_channel; self->mod->server_get_channel_id = server_get_channel_id; @@ -1635,6 +1636,7 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) struct xrdp_rdp *rdp; struct xrdp_sec *sec; struct xrdp_channel *chan; + int in_progress; LOG_DEVEL(LOG_LEVEL_TRACE, "process_display_control_monitor_layout_data:"); @@ -1731,7 +1733,10 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) break; case WMRZ_SERVER_MONITOR_RESIZE: error = module->mod_server_monitor_resize( - module, desc_width, desc_height); + module, desc_width, desc_height, + description->description.monitorCount, + description->description.minfo, + &in_progress); if (error != 0) { LOG_DEVEL(LOG_LEVEL_INFO, @@ -1739,37 +1744,27 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) " mod_server_monitor_resize failed %d", error); return advance_error(error, mm); } - advance_resize_state_machine( - mm, WMRZ_SERVER_VERSION_MESSAGE_START); - break; - case WMRZ_SERVER_VERSION_MESSAGE_START: - /* Update the client_info structure with the new description - * and tell the module so it can communicate the new - * screen layout to the backend */ - sync_dynamic_monitor_data(wm, &(description->description)); - module->mod_set_param(module, "client_info", - (const char *) (wm->session->client_info)); - - error = module->mod_server_version_message(module); - if (error != 0) + else if (in_progress) { - LOG_DEVEL(LOG_LEVEL_INFO, - "process_display_control_monitor_layout_data:" - " mod_server_version_message failed %d", error); - return advance_error(error, mm); + // Call is proceeding asynchronously + advance_resize_state_machine( + mm, WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING); + } + else + { + // Call is done + advance_resize_state_machine( + mm, WMRZ_SERVER_MONITOR_MESSAGE_PROCESSED); } - advance_resize_state_machine( - mm, WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING); break; - // Not processed here. Processed in server_reset + // Not processed here. Processed in client_monitor_resize // case WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING: case WMRZ_SERVER_MONITOR_MESSAGE_PROCESSED: advance_resize_state_machine(mm, WMRZ_XRDP_CORE_RESET); break; case WMRZ_XRDP_CORE_RESET: - // TODO: Unify this logic with server_reset - error = libxrdp_reset( - wm->session, desc_width, desc_height, wm->screen->bpp); + sync_dynamic_monitor_data(wm, &(description->description)); + error = libxrdp_reset(wm->session); if (error != 0) { LOG_DEVEL(LOG_LEVEL_INFO, @@ -4439,95 +4434,95 @@ server_draw_text(struct xrdp_mod *mod, int font, } /*****************************************************************************/ - -/* Note : if this is called on a multimon setup, the client is resized - * to a single monitor */ int -server_reset(struct xrdp_mod *mod, int width, int height, int bpp) +client_monitor_resize(struct xrdp_mod *mod, int width, int height, + int num_monitors, const struct monitor_info *monitors) { + int error = 0; struct xrdp_wm *wm; - struct xrdp_mm *mm; - - LOG(LOG_LEVEL_TRACE, "server_reset:"); + struct display_size_description *display_size_data; + LOG_DEVEL(LOG_LEVEL_TRACE, "client_monitor_resize:"); wm = (struct xrdp_wm *)(mod->wm); - if (wm == 0) + if (wm == 0 || wm->mm == 0 || wm->client_info == 0) { return 1; } - mm = wm->mm; - if (wm->client_info == 0) + if (wm->client_info->client_resize_mode == CRMODE_NONE) { + LOG(LOG_LEVEL_WARNING, "Server is not allowed to resize this client"); return 1; } - /* older client can't resize */ - if (wm->client_info->build <= 419) + if (wm->client_info->client_resize_mode == CRMODE_SINGLE_SCREEN && + num_monitors > 1) { - return 0; + LOG(LOG_LEVEL_WARNING, + "Server cannot resize this client with multiple monitors"); + return 1; } - // bpp of zero is impossible. - // This is a signal from xup that - // It is finished resizing. - if (bpp == 0) + display_size_data = g_new0(struct display_size_description, 1); + if (display_size_data == NULL) { - if (mm == 0) - { - return 1; - } - if (!xrdp_wm_can_resize(wm)) - { - return 1; - } - if (mm->resize_data == NULL) - { - mm->mod->mod_server_monitor_full_invalidate(mm->mod, width, height); - return 0; - } - if (mm->resize_data != NULL - && mm->resize_data->state - == WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING) - { - LOG(LOG_LEVEL_INFO, - "server_reset: Advancing server monitor resized."); - advance_resize_state_machine( - mm, WMRZ_SERVER_MONITOR_MESSAGE_PROCESSED); - } - else if (mm->resize_data != NULL - && mm->resize_data->description.session_height == 0 - && mm->resize_data->description.session_width == 0) - { - mm->mod->mod_server_monitor_full_invalidate(mm->mod, width, height); - } - return 0; + LOG(LOG_LEVEL_ERROR, "client_monitor_resize: Out of memory"); + return 1; } + error = libxrdp_init_display_size_description(num_monitors, + monitors, + display_size_data); + if (error) + { + LOG(LOG_LEVEL_ERROR, "client_monitor_resize:" + " libxrdp_init_display_size_description" + " failed with error %d.", error); + free(display_size_data); + return error; + } + list_add_item(wm->mm->resize_queue, (tintptr)display_size_data); + g_set_wait_obj(wm->mm->resize_ready); - /* if same (and only one monitor on client) don't need to do anything */ - if (wm->client_info->display_sizes.session_width == (uint32_t)width && - wm->client_info->display_sizes.session_height == (uint32_t)height && - wm->client_info->bpp == bpp && - (wm->client_info->display_sizes.monitorCount == 0 || - wm->client_info->multimon == 0)) + return 0; +} + +/*****************************************************************************/ + +/* Note : if this is called on a multimon setup, the client is resized + * to a single monitor */ +int +server_monitor_resize_done(struct xrdp_mod *mod) +{ + struct xrdp_wm *wm; + struct xrdp_mm *mm; + + LOG(LOG_LEVEL_TRACE, "server_monitor_resize_done:"); + + wm = (struct xrdp_wm *)(mod->wm); + if (wm == 0) { - return 0; + return 1; + } + mm = wm->mm; + if (mm == 0) + { + return 1; } - /* reset lib, client_info gets updated in libxrdp_reset */ - if (libxrdp_reset(wm->session, width, height, bpp) != 0) + if (wm->client_info == 0) { return 1; } - /* reset cache */ - xrdp_cache_reset(wm->cache, wm->client_info); - /* resize the main window */ - xrdp_bitmap_resize(wm->screen, wm->client_info->display_sizes.session_width, - wm->client_info->display_sizes.session_height); - /* load some stuff */ - xrdp_wm_load_static_colors_plus(wm, 0); - xrdp_wm_load_static_pointers(wm); + if (mm->resize_data != NULL + && mm->resize_data->state + == WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING) + { + LOG(LOG_LEVEL_INFO, + "server_monitor_resize_done: Advancing server monitor resized."); + advance_resize_state_machine( + mm, WMRZ_SERVER_MONITOR_MESSAGE_PROCESSED); + } return 0; } diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index 472a725042..7607c39c67 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -353,7 +353,6 @@ enum display_resize_state WMRZ_EGFX_CONN_CLOSED, WRMZ_EGFX_DELETE, WMRZ_SERVER_MONITOR_RESIZE, - WMRZ_SERVER_VERSION_MESSAGE_START, WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING, WMRZ_SERVER_MONITOR_MESSAGE_PROCESSED, WMRZ_XRDP_CORE_RESET, @@ -376,8 +375,6 @@ enum display_resize_state (status) == WMRZ_EGFX_CONN_CLOSED ? "WMRZ_EGFX_CONN_CLOSED" : \ (status) == WRMZ_EGFX_DELETE ? "WMRZ_EGFX_DELETE" : \ (status) == WMRZ_SERVER_MONITOR_RESIZE ? "WMRZ_SERVER_MONITOR_RESIZE" : \ - (status) == WMRZ_SERVER_VERSION_MESSAGE_START ? \ - "WMRZ_SERVER_VERSION_MESSAGE_START" : \ (status) == WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING ? \ "WMRZ_SERVER_MONITOR_MESSAGE_PROCESSING" : \ (status) == WMRZ_SERVER_MONITOR_MESSAGE_PROCESSED ? \ From f3fe06a0c7724c958387f943891504d6374b4991 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 10:29:36 +0000 Subject: [PATCH 09/12] EGFX: Ignore incoming messages after close sent --- xrdp/xrdp_egfx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xrdp/xrdp_egfx.c b/xrdp/xrdp_egfx.c index 26b097834c..8fe0cda0ef 100644 --- a/xrdp/xrdp_egfx.c +++ b/xrdp/xrdp_egfx.c @@ -1121,6 +1121,10 @@ xrdp_egfx_shutdown_close_connection(struct xrdp_egfx *egfx) return error; } + // Ignore any messages we haven't processed yet + egfx->caps_advertise = NULL; + egfx->frame_ack = NULL; + return error; } From 3ca3bd197f421e2d5a73f0fd0827dc9a8cb991b5 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 15 Feb 2024 11:23:46 +0000 Subject: [PATCH 10/12] Prevent EGFX drawing while channel is down Clear egfx_up as soon as the channel starts to close so that xrdp_mm_draw_dirty() doesn't send data to a closed channel. --- xrdp/xrdp_mm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 882dfbd504..650dfb48b9 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1699,6 +1699,7 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) if (error == 0 && module != 0) { xrdp_egfx_shutdown_close_connection(wm->mm->egfx); + mm->egfx_up = 0; } advance_resize_state_machine(mm, WMRZ_EGFX_CONN_CLOSING); break; @@ -1727,7 +1728,6 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) { xrdp_egfx_shutdown_delete(wm->mm->egfx); mm->egfx = NULL; - mm->egfx_up = 0; } advance_resize_state_machine(mm, WMRZ_SERVER_MONITOR_RESIZE); break; From c21521f80d030307ae5c7b56b0af1f339c69dee4 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Sat, 17 Feb 2024 17:36:01 +0000 Subject: [PATCH 11/12] Clear memory allocated to resized bitmaps This prevents valgrind errors when resizing the screen to a larger size on GFX systems. --- xrdp/xrdp_bitmap_common.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/xrdp/xrdp_bitmap_common.c b/xrdp/xrdp_bitmap_common.c index 836ba819e1..9d0625fceb 100644 --- a/xrdp/xrdp_bitmap_common.c +++ b/xrdp/xrdp_bitmap_common.c @@ -301,8 +301,6 @@ xrdp_bitmap_resize(struct xrdp_bitmap *self, int width, int height) return 1; } - self->width = width; - self->height = height; Bpp = 4; switch (self->bpp) @@ -318,8 +316,28 @@ xrdp_bitmap_resize(struct xrdp_bitmap *self, int width, int height) break; } - g_free(self->data); - self->data = (char *)g_malloc(width * height * Bpp, 0); + /* To prevent valgrind errors (particularly on a screen resize), + clear extra memory */ + unsigned long old_size = self->width * self->height * Bpp; + unsigned long new_size = width * height * Bpp; + + char *new_data = (char *)realloc(self->data, new_size); + if (new_data == NULL) + { + return 1; + } + + self->width = width; + self->height = height; + if (new_data != self->data) + { + self->data = new_data; + memset(self->data, 0, new_size); + } + else if (new_size > old_size) + { + memset(self->data + old_size, 0, new_size - old_size); + } self->line_size = width * Bpp; return 0; } From 09715f4cf48b942202a70fb3715f938ea46a212b Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 19 Feb 2024 16:26:04 +0000 Subject: [PATCH 12/12] GFX: Prevent MM screen being written to the client In GFX mode, if we're using xorgxrdp, frame updates are send directly from the client, bypassing the screen buffer in xrdp_mm. This commit only allows the xrdp_mm screen buffer to be invalidated if the painter has drawn into it since the module was loaded. This prevents the unused (and invalid) frame buffer being pushed to the client in GFX mode with xorgxrdp. --- xrdp/xrdp.h | 3 ++ xrdp/xrdp_mm.c | 83 +++++++++++++++++++++++++++++++-------------- xrdp/xrdp_painter.c | 14 ++------ xrdp/xrdp_types.h | 1 + 4 files changed, 64 insertions(+), 37 deletions(-) diff --git a/xrdp/xrdp.h b/xrdp/xrdp.h index f477b3b491..b2a8ae9717 100644 --- a/xrdp/xrdp.h +++ b/xrdp/xrdp.h @@ -502,6 +502,9 @@ int xrdp_mm_check_wait_objs(struct xrdp_mm *self); int xrdp_mm_frame_ack(struct xrdp_mm *self, int frame_id); +void +xrdp_mm_efgx_add_dirty_region_to_planar_list(struct xrdp_mm *self, + struct xrdp_region *dirty_region); int xrdp_mm_egfx_send_planar_bitmap(struct xrdp_mm *self, struct xrdp_bitmap *bitmap, diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 650dfb48b9..c136894a26 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1095,25 +1095,32 @@ xrdp_mm_egfx_send_planar_bitmap(struct xrdp_mm *self, /******************************************************************************/ static int -xrdp_mm_egfx_invalidate_all(struct xrdp_mm *self) +xrdp_mm_egfx_invalidate_wm_screen(struct xrdp_mm *self) { - struct xrdp_rect xr_rect; - struct xrdp_bitmap *screen; - int error; - - LOG(LOG_LEVEL_INFO, "xrdp_mm_egfx_invalidate_all:"); + int error = 0; - screen = self->wm->screen; + LOG(LOG_LEVEL_INFO, "xrdp_mm_egfx_invalidate_wm_screen:"); - xr_rect.left = 0; - xr_rect.top = 0; - xr_rect.right = screen->width; - xr_rect.bottom = screen->height; - if (self->wm->screen_dirty_region == NULL) + // Only invalidate the WM screen if the module is using the WM screen, + // or we haven't loaded the module yet. + // + // Otherwise we may send client updates which conflict with the + // updates sent directly from the module via the encoder. + if (self->mod_uses_wm_screen_for_gfx || self->mod_handle == 0) { - self->wm->screen_dirty_region = xrdp_region_create(self->wm); + struct xrdp_bitmap *screen = self->wm->screen; + struct xrdp_rect xr_rect; + + xr_rect.left = 0; + xr_rect.top = 0; + xr_rect.right = screen->width; + xr_rect.bottom = screen->height; + if (self->wm->screen_dirty_region == NULL) + { + self->wm->screen_dirty_region = xrdp_region_create(self->wm); + } + error = xrdp_region_add_rect(self->wm->screen_dirty_region, &xr_rect); } - error = xrdp_region_add_rect(self->wm->screen_dirty_region, &xr_rect); return error; } @@ -1370,7 +1377,7 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, self->egfx_up = 1; xrdp_mm_egfx_create_surfaces(self); self->encoder = xrdp_encoder_create(self); - xrdp_mm_egfx_invalidate_all(self); + xrdp_mm_egfx_invalidate_wm_screen(self); if (self->resize_data != NULL && self->resize_data->state == WMRZ_EGFX_INITALIZING) @@ -1846,20 +1853,13 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) // Ack all frames to speed up resize. module->mod_frame_ack(module, 0, INT_MAX); } - // Restart module output + // Restart module output after invalidating + // the screen. This causes an automatic redraw. + xrdp_bitmap_invalidate(wm->screen, 0); rdp = (struct xrdp_rdp *) (wm->session->rdp); xrdp_rdp_suppress_output(rdp, 0, XSO_REASON_DYNAMIC_RESIZE, 0, 0, desc_width, desc_height); - /* redraw */ - error = xrdp_bitmap_invalidate(wm->screen, 0); - if (error != 0) - { - LOG_DEVEL(LOG_LEVEL_INFO, - "process_display_control_monitor_layout_data:" - " xrdp_bitmap_invalidate failed %d", error); - return advance_error(error, mm); - } advance_resize_state_machine(mm, WMRZ_COMPLETE); break; default: @@ -3535,6 +3535,39 @@ xrdp_mm_process_enc_done(struct xrdp_mm *self) return 0; } +/*****************************************************************************/ +void +xrdp_mm_efgx_add_dirty_region_to_planar_list(struct xrdp_mm *self, + struct xrdp_region *dirty_region) +{ + int jndex = 0; + struct xrdp_rect rect; + + int error = xrdp_region_get_rect(dirty_region, jndex, &rect); + if (error == 0) + { + if (self->wm->screen_dirty_region == NULL) + { + self->wm->screen_dirty_region = xrdp_region_create(self->wm); + } + + do + { + xrdp_region_add_rect(self->wm->screen_dirty_region, &rect); + jndex++; + error = xrdp_region_get_rect(dirty_region, jndex, &rect); + } + while (error == 0); + + if (self->mod_handle != 0) + { + // Module has been writing to WM screen using GFX + self->mod_uses_wm_screen_for_gfx = 1; + } + } + +} + /*****************************************************************************/ static int xrdp_mm_draw_dirty(struct xrdp_mm *self) diff --git a/xrdp/xrdp_painter.c b/xrdp/xrdp_painter.c index fce0d3780d..a5ef48a026 100644 --- a/xrdp/xrdp_painter.c +++ b/xrdp/xrdp_painter.c @@ -95,18 +95,8 @@ xrdp_painter_send_dirty(struct xrdp_painter *self) if (self->session->client_info->gfx) { - if (self->wm->screen_dirty_region == NULL) - { - self->wm->screen_dirty_region = xrdp_region_create(self->wm); - } - jndex = 0; - error = xrdp_region_get_rect(self->dirty_region, jndex, &rect); - while (error == 0) - { - xrdp_region_add_rect(self->wm->screen_dirty_region, &rect); - jndex++; - error = xrdp_region_get_rect(self->dirty_region, jndex, &rect); - } + xrdp_mm_efgx_add_dirty_region_to_planar_list(self->wm->mm, + self->dirty_region); } else { diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index 7607c39c67..c2195a9cf5 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -439,6 +439,7 @@ struct xrdp_mm int egfx_up; enum xrdp_egfx_flags egfx_flags; int gfx_delay_autologin; + int mod_uses_wm_screen_for_gfx; /* Resize on-the-fly control */ struct display_control_monitor_layout_data *resize_data; struct list *resize_queue;