From 2449aa500d391e0d3c33bb95e5bbb5c9432d9a11 Mon Sep 17 00:00:00 2001 From: pletch <17855860+pletch@users.noreply.github.com> Date: Mon, 31 Aug 2026 23:03:57 -0400 Subject: [PATCH] fix(rdp): install the RDPGFX wrappers once, and only for H.264 guac_rdp_rdpgfx_channel_connected() installed the SurfaceCommand and CapsConfirm wrappers unconditionally, every time it ran. It runs again when the RDPGFX channel reconnects, which xrdp provokes with the display resize it performs at login, and the second pass saved the wrapper the first pass had installed: rdp_client->orig_surface_command = (void*) rdpgfx->SurfaceCommand; rdpgfx->SurfaceCommand = guac_rdp_gfx_surface_command; leaving orig_surface_command pointing at guac_rdp_gfx_surface_command itself. Any surface command that is not H.264 falls through to orig(context, cmd), re-enters the wrapper, and recurses until the stack is exhausted and the guacd child dies -- a white screen in the browser, with no "Client terminated successfully" in the log and the segfault visible only outside a container. H.264 commands take skip_decode and never reach that call, so a server sending nothing but H.264 is unaffected. A stock xrdp is the opposite case: one trace showed 7440 planar and 12 progressive surface commands against 437 AVC420, so it dies on the first frame. Distro xrdp packages (0.10.1 on Ubuntu) are built --enable-rfxcodec with no H.264 encoder at all, so they send planar for everything and hit this immediately. The wrappers were also installed for every GFX session regardless of enable_h264, so this reproduced with RFX alone and H.264 unticked -- no H.264-capable server required. Both halves are fixed: install only when the connection asked for H.264, and only when nothing is installed yet. guac_rdp_client is guac_mem_zalloc'd, so the NULL check is sound on first entry. --- patches/004-h264-passthrough.patch | 37 ++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/patches/004-h264-passthrough.patch b/patches/004-h264-passthrough.patch index 7bb89e0..aa10297 100644 --- a/patches/004-h264-passthrough.patch +++ b/patches/004-h264-passthrough.patch @@ -992,7 +992,7 @@ index 327e7c2b..c49737d7 100644 /** * Callback which associates handlers specific to Guacamole with the * RdpgfxClientContext instance allocated by FreeRDP to deal with received -@@ -63,13 +438,28 @@ static void guac_rdp_rdpgfx_channel_connected(rdpContext* context, +@@ -63,13 +438,45 @@ static void guac_rdp_rdpgfx_channel_connected(rdpContext* context, RdpgfxClientContext* rdpgfx = (RdpgfxClientContext*) args->pInterface; rdpGdi* gdi = context->gdi; @@ -1006,18 +1006,35 @@ index 327e7c2b..c49737d7 100644 guac_client_log(client, GUAC_LOG_DEBUG, "RDPGFX channel will be used for " "the RDP Graphics Pipeline Extension."); -+ /* Wrap SurfaceCommand to intercept H.264 data after GDI decode. -+ * Store the original callback per-connection to avoid clobbering -+ * when multiple RDP sessions are active concurrently. */ + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; -+ rdp_client->orig_surface_command = (void*) rdpgfx->SurfaceCommand; -+ rdpgfx->SurfaceCommand = guac_rdp_gfx_surface_command; + -+ rdp_client->orig_caps_confirm = (void*) rdpgfx->CapsConfirm; -+ rdpgfx->CapsConfirm = guac_rdp_gfx_caps_confirm; ++ /* Installed only for a connection that asked for H.264, and only ++ * once. This handler runs again whenever the RDPGFX channel ++ * reconnects -- the display resize xrdp performs at login triggers ++ * exactly that -- and a second install reads back the wrapper the ++ * first one left behind, so orig_surface_command ends up pointing at ++ * guac_rdp_gfx_surface_command itself. Every surface command that is ++ * not H.264 then calls the wrapper from inside the wrapper and ++ * recurses until the stack is gone. H.264 commands take the ++ * skip_decode path and never make that call, so a server sending ++ * nothing but H.264 survives it, while one mixing in planar or ++ * progressive surfaces dies on the first of them. */ ++ if (rdp_client->settings->enable_h264 ++ && rdp_client->orig_surface_command == NULL) { + -+ guac_client_log(client, GUAC_LOG_INFO, -+ "H.264 passthrough enabled for RDPGFX channel."); ++ /* Wrap SurfaceCommand to intercept H.264 data after GDI decode. ++ * Store the original callback per-connection to avoid clobbering ++ * when multiple RDP sessions are active concurrently. */ ++ rdp_client->orig_surface_command = (void*) rdpgfx->SurfaceCommand; ++ rdpgfx->SurfaceCommand = guac_rdp_gfx_surface_command; ++ ++ rdp_client->orig_caps_confirm = (void*) rdpgfx->CapsConfirm; ++ rdpgfx->CapsConfirm = guac_rdp_gfx_caps_confirm; ++ ++ guac_client_log(client, GUAC_LOG_INFO, ++ "H.264 passthrough enabled for RDPGFX channel."); ++ ++ } + } + }