Debugging xrdp Session Sharing: A Tale of Buffer Overflows, Double Frees, and State Machine Nightmares #
TL;DR #
We implemented session sharing (shadowing) in xrdp — allowing multiple RDP clients to connect to the same desktop session with independent cursors and input routing. The feature required fixes across three layers: the xorgxrdp X driver (per-client MPX input dispatch), the sesman session manager (don’t kill existing clients when new ones join), and the xrdp window manager (non-terminal state to keep RDP connections alive). Along the way we fixed a 128-byte buffer overflow, a double-free from packed struct allocations, a state machine that re-processed its own state, and a UI that wouldn’t paint or respond to clicks. Core dumps and gdb inside a privileged container were essential for cracking the hardest bugs.
The Goal #
Enable session sharing (shadowing) in xrdp. Two RDP clients should be able to connect to the same X session, each with their own cursor, with input routed to the correct client. The architecture:
- xorgxrdp (X driver): Creates per-client MPX (Multi-Pointer X) keyboard and pointer devices
- xrdp (RDP server): Shows a session selector UI after login, lets users join existing sessions
- sesman (session manager): Manages session lifecycle, forwards connect requests to the running sesexec
- sesexec: The per-session executor that talks to the X server
The session sharing flag (SessionSharing "true" in xorg.conf) was already wired in the Docker test environment. What wasn’t working was… everything after the first client connected.
Problems Encountered & Solutions #
1. xorgxrdp: Global Input Callback Overwritten by Each Client #
Context: In xrdp’s architecture, each RDP client connection creates a clientCon (client connection) object in the xorgxrdp X driver. When session sharing is enabled, multiple clientCons exist simultaneously, each representing a different RDP client viewing the same X desktop. The previous agent had added MPX (Multi-Pointer X) device support — creating separate X input devices per client so each RDP client gets its own cursor. The idea was sound: X11’s MPX extension supports multiple independent pointers/keyboards on the same display.
What the previous change tried to do: The agent registered per-client keyboard and pointer devices during clientCon creation, and set up input callbacks to route events to those devices. However, the input dispatch mechanism used a global callback array g_input_proc[0/1] (index 0 for keyboard, 1 for mouse) that was shared across all clients.
The problem: Every time a new client connected, its MPX control function (rdpKeyboardControl / rdpMouseControl) overwrote g_input_proc[0/1] with its own callback. This meant client B’s callbacks replaced client A’s, so all input from the X server went to whichever client connected last. Client A’s cursor would freeze or receive no input.
Root cause: The global callback array is a singleton — it can only point to one client’s dispatch function at a time. The previous change registered each client’s device but reused the same global dispatch slot.
Fix: Added rdpInputKeyboardEventClient() and rdpInputMouseEventClient() — per-client dispatch functions that take a clientCon pointer and route input directly to that client’s MPX device (clientCon->keyboard_device / clientCon->pointer_device). The rdpClientCon.c input handler now calls these per-client functions instead of the global dispatch. Falls back to the global callback when no MPX device exists (non-sharing mode).
2. xauth Cookie Mismatch #
Context: When sesman creates a new X session, it needs to set up X authentication cookies so that the waitforx utility (which polls until the X server is ready) and the xrdp child process can connect to it. The add_xauth_cookie() function in sesman/sesexec/xauth.c generates a random MIT-MAGIC-COOKIE-1 and registers it with xauth.
What the previous change tried to do: The original code only created a hostname-based xauth entry: xauth add hostname/unix:10 . <cookie>. This works for TCP connections to the X server but not for Unix domain socket connections.
The problem: waitforx connects to the X server via a Unix domain socket (/tmp/.X11-unix/X10), which requires an xauth entry with the form unix:10. The hostname-based entry doesn’t match, so waitforx gets XAUTHORITY authentication failures and can’t detect that the X server is running. This causes session creation to hang or fail.
Root cause: X authentication is connection-type-specific. A cookie registered for hostname/unix:10 doesn’t authorize connections to unix:10 (Unix socket) or INET:hostname:6010 (TCP).
Fix: Added a second xauth add unix:%d . <cookie> entry alongside the hostname-based one, so both Unix socket and TCP connections are authorized.
3. Session Selector Infinite Loop #
Context: After a successful login to sesman, the xrdp window manager transitions through a series of login states (WMLS_CONNECT_IN_PROGRESS → WMLS_SESSION_SELECTOR). The xrdp_wm_login_state_changed() function in xrdp_wm.c handles state transitions — when the state is WMLS_SESSION_SELECTOR, it creates the session selector UI and calls xrdp_mm_list_sessions() to fetch the list of existing sessions from sesman.
What the previous change tried to do: The agent added the session selector window creation code inside the WMLS_SESSION_SELECTOR handler. At the end of the handler, it called xrdp_wm_set_login_state(self, WMLS_SESSION_SELECTOR) — presumably to ensure the state was set correctly.
The problem: xrdp_wm_set_login_state() calls g_set_wait_obj(self->login_state_event), which signals the main loop that the login state has changed. The main loop then calls xrdp_wm_login_state_changed() again, which sees WMLS_SESSION_SELECTOR and recreates the UI and re-fetches the session list — ad infinitum. This caused a tight loop that consumed 100% CPU and prevented the UI from ever rendering.
Root cause: The handler was re-triggering its own state, creating an infinite event loop.
Fix: Removed the recursive xrdp_wm_set_login_state(self, WMLS_SESSION_SELECTOR) call at the end of the handler. The state was already set by the login response handler; the WMLS_SESSION_SELECTOR handler just needs to build the UI once.
4. libipm GUID Parsing Bug #
Context: Each X session has a unique GUID that identifies it. When the xrdp client requests a list of sessions from sesman, sesman sends back E_SCP_LIST_SESSIONS_RESPONSE messages, each containing session info including the GUID. The GUID is serialized as a “flexible-size buffer” (FSB) in the libipm protocol — a length-prefixed binary blob.
What the previous change tried to do: In scp_get_list_sessions_response() (the client-side parser), the agent initialized the GUID descriptor as struct libipm_fsb guid_descriptor = { NULL, 0 } and passed it to libipm_msg_in_parse(). The idea was that the parser would fill in the descriptor with the received GUID data.
The problem: The extract_fsb_type() function in libipm (which parses FSB fields) requires the data pointer in the descriptor to be non-NULL — it copies the received bytes into the pre-allocated buffer. With data = NULL, the parser either skipped the GUID entirely or wrote to NULL (undefined behavior). This meant the GUID was never received, so clicking a session button would pass an empty or garbage GUID to sesman.
Root cause: The FSB parsing API requires the caller to pre-allocate the destination buffer. Initializing with { NULL, 0 } is only valid for the send side (where the library reads from the buffer), not the receive side (where the library writes to it).
Fix: Pre-allocated a stack buffer struct guid recv_guid and initialized the descriptor as { &recv_guid, sizeof(recv_guid) }. After parsing, the GUID is copied directly: p->guid = recv_guid.
5. SCP Transport Buffer Too Small (128 bytes) #
Context: The SCP (Session Control Protocol) is the Unix socket-based protocol between the xrdp child process and sesman/sesexec. The scp_connect() function in libipm/scp.c creates a trans (transport) object for this communication. The transport has an input buffer that holds incoming messages before they’re parsed.
What the previous change tried to do: The original code created the transport with trans_create(TRANS_MODE_UNIX, 128, 128) — 128 bytes for both input and output buffers. This was fine for simple messages like login requests/responses, which are just a few integers and short strings.
The problem: Session list responses are much larger. Each E_SCP_LS_SESSION_INFO message contains:
displaystring (e.g., “X11-10”)start_ip_addrstringclient_ipstringclient_namestring (e.g., “odroid”)xrdp_instance_namestring- A 16-byte GUID
- Protocol overhead (message headers, length prefixes)
A single session info message easily exceeds 128 bytes. When libipm_msg_in_parse() tries to read beyond the buffer boundary, parser_stream_overflow_check() in common/parse.c detects the overflow and calls abort() — a deliberate safety measure to prevent buffer overread exploits.
Root cause: The 128-byte buffer was sized for simple login messages, not for the larger session list responses that the new session selector feature introduced.
Fix: Increased the buffer from 128 to 8192 bytes in scp_connect(). This provides ample headroom for session list responses with multiple sessions.
6. Double-Free in Session List Response Handler #
Context: When sesman sends session info, it uses scp_send_list_sessions_response() which allocates a single memory block containing the scp_session_info struct and all its string fields packed together. The COPY_STRING macro in the parser copies each string into this block at the appropriate offset. This is an optimization — one allocation, one free.
What the previous change tried to do: In xrdp_mm_process_list_sessions_response(), the agent wrote cleanup code that freed each string field individually before freeing the struct:
g_free(info->display);
g_free(info->start_ip_addr);
g_free(info->client_ip);
g_free(info->client_name);
g_free(info->xrdp_instance_name);
g_free(info);This pattern is common when each string is individually malloc’d. It was a reasonable assumption without reading the allocation side.
The problem: Since all strings live inside the same allocation as the struct, g_free(info->display) passes an interior pointer (not the start of the allocation) to free(). glibc’s malloc detects this as heap corruption and aborts with munmap_chunk(): invalid pointer. The crash manifested as SIGSEGV when clicking any session selector button, because the session list was processed (and corrupted) before the button click handler ran.
Root cause: Mismatched allocation/free patterns. The sender packs everything into one block; the receiver tried to free each piece separately.
Fix: Changed to a single g_free(info) since all strings are in the same allocation. Added a comment explaining the allocation layout so future maintainers don’t reintroduce the bug.
7. Missing mmcs_expecting_msg in xrdp_mm_list_sessions()
#
Context: The xrdp connection state machine (xrdp_mm_connect_sm) uses mmcs_expecting_msg to track whether it’s waiting for a response from sesman. When mmcs_expecting_msg = 1, the state machine pauses and lets the event loop wait for incoming data. When a response arrives via xrdp_mm_scp_data_in(), the appropriate handler processes it and clears the flag.
What the previous change tried to do: The agent added xrdp_mm_list_sessions() as a standalone function that sends scp_send_list_sessions_request() to sesman. It was called from the session selector UI handler to populate the list of existing sessions.
The problem: The function sent the request but never set mmcs_expecting_msg = 1. This meant the state machine didn’t know a response was pending. If another sesman message arrived (e.g., a login response from a concurrent connection), it could be misinterpreted as the list sessions response, or the list sessions response could arrive when the state machine wasn’t expecting it, causing response interleaving.
Root cause: The mmcs_expecting_msg flag is the state machine’s way of correlating requests with responses. Without it, the state machine has no way to know which incoming message corresponds to which outgoing request.
Fix: Set self->mmcs_expecting_msg = 1 after a successful scp_send_list_sessions_request() call. The flag is cleared by xrdp_mm_process_list_sessions_response() when the response is processed.
8. sesexec Terminating Existing Client on New Connection #
Context: When a client wants to join an existing session, the xrdp child sends E_SCP_CONNECT_SESSION_REQUEST to sesman, which forwards it to the sesexec process managing that session. sesexec’s handle_connect_session_request() in ercp_server.c is responsible for passing the session’s display and chansrv file descriptors to the new client.
What the previous change tried to do: The original sesexec code had this logic:
if (g_ccp_trans != NULL) {
sesexec_terminate_connected_xrdp_process(
CCP_CLOSE_DISCONNECTED_BY_OTHERCONNECTION);
g_sleep(500);
}This makes sense for the default xrdp behavior: when a new client connects to an existing session, the old client should be kicked off (session takeover). The CCP_CLOSE_DISCONNECTED_BY_OTHERCONNECTION reason tells the old xrdp child to disconnect its RDP client with an appropriate error message.
The problem: In session sharing mode, we want both clients to stay connected. But sesexec had no way to know whether the new connection was a takeover or a sharing request — it always terminated the existing client. So when client B joined client A’s session, sesexec sent a close request to client A’s xrdp child, which disconnected client A from the RDP session.
Root cause: sesexec operates in a binary world: either one client is connected, or it isn’t. It had no concept of “multiple concurrent clients.”
Fix: Added E_SCP_SCONNECT_FLAG_SESSION_SHARING (bit 1) to the connect session request flags. The xrdp client always sets this flag when connecting to an existing session. sesexec checks the flag and skips the termination step when it’s set, allowing both clients to remain connected.
9. State Machine Re-Processing Its Own State #
Context: The xrdp_mm_connect_sm() function is a synchronous state machine that runs in a while loop. Each iteration processes the current connect_state, then auto-increments it:
self->connect_state = (enum mm_connect_state)(self->connect_state + 1);The states are: MMCS_CONNECT_TO_SESMAN → MMCS_GATEWAY_LOGIN → MMCS_SESSION_LOGIN → MMCS_CREATE_SESSION → MMCS_GET_SESSION_FILE_DESCRIPTORS → MMCS_CONNECT_TO_DISPLAY_SERVER → MMCS_CONNECT_TO_CHANSRV → MMCS_DONE.
What the previous change tried to do: When connecting to an existing session, xrdp_mm_connect_session() sends the connect request and sets connect_state = MMCS_GET_SESSION_FILE_DESCRIPTORS. The response arrives asynchronously via xrdp_mm_scp_data_in() → xrdp_mm_process_connect_session_response(), which calls xrdp_mm_connect_sm() to continue the state machine.
The problem: xrdp_mm_connect_sm() resets mmcs_expecting_msg = 0 at the top, then enters its while loop. Since connect_state was still MMCS_GET_SESSION_FILE_DESCRIPTORS, the loop processed that state again — calling xrdp_mm_get_session_fds() a second time. This sent a duplicate connect request to sesman, which incremented the connection count again and confused the session tracking.
Root cause: The response handler called the state machine without advancing past the state that had just been completed. The state machine’s auto-increment happens at the end of each iteration, so re-entering at the same state re-processes it.
Fix: Set connect_state = MMCS_CONNECT_TO_DISPLAY_SERVER before calling xrdp_mm_connect_sm(). This skips past the completed state and lets the state machine continue from where it should.
10. sesman_trans Check Failed After Connect Session Response #
Context: After sesman sends the connect session response (containing the display and chansrv file descriptors), it closes its end of the Unix socket. The xrdp child receives the file descriptors via SCM_RIGHTS and then uses them to connect directly to the X server. The sesman transport (sesman_trans) is no longer needed.
What the previous change tried to do: The xrdp_mm_process_connect_session_response() handler converted the transport from SCP to CCP (Client Control Protocol) mode using ccp_trans_from_scp_trans(), then called xrdp_mm_connect_sm() to continue connecting to the display server.
The problem: After the state machine completed, the next event loop iteration called xrdp_mm_check_wait_objs(), which checked trans_check_wait_objs(self->sesman_trans). Since sesman had already closed its end of the socket, trans_check_wait_objs() returned an error. This triggered the error handler path, which called xrdp_mm_set_fatal() and disconnected the RDP client.
Root cause: The sesman transport was dead (sesman closed it) but the xrdp child still had it registered in its wait objects list.
Fix: Set delete_sesman_trans = 1 before calling xrdp_mm_connect_sm(). This marks the transport for deletion, so xrdp_mm_check_wait_objs() skips it and cleans it up properly.
11. Session Selector UI Not Painting #
Context: xrdp’s UI is built from xrdp_bitmap objects — windows, buttons, labels, etc. When a bitmap is created, it’s added to its parent’s child list but not automatically painted. The xrdp_bitmap_invalidate() function marks a region as needing repaint, which triggers the painter to draw it on the next event loop iteration.
What the previous change tried to do: The agent created the session selector window and all its child controls (title label, buttons, session buttons) inside the WMLS_SESSION_SELECTOR handler. After creating everything, it called xrdp_wm_set_focused() to set keyboard focus.
The problem: No xrdp_bitmap_invalidate() call was made. The window and its children existed in memory but were never marked as needing repaint. The X server’s damage tracking didn’t know anything had changed, so nothing was drawn. When the user manually dragged the window, the drag operation triggered a repaint of the affected region, making the window suddenly appear.
Root cause: xrdp’s UI uses explicit invalidation — creating a control doesn’t automatically schedule a repaint. This is a common pattern in low-level UI frameworks to avoid redundant repaints.
Fix: Added xrdp_bitmap_invalidate(self->session_selector_window, 0) after xrdp_wm_set_focused(). The 0 (NULL rect) means invalidate the entire window.
12. Buttons Not Responding to Clicks #
Context: xrdp’s UI event dispatch works as follows: when a button is clicked, xrdp_bitmap_mouse_button_down() finds the clicked control, then calls control->parent->notify(control->owner, control, 1, x, y). The notify callback is on the parent (the window), not on the control itself. The owner field is passed as the first argument to the callback — it’s typically the window itself.
What the previous change tried to do: The agent set btn->notify = xrdp_wm_session_selector_notify on each button, assuming the notify callback would be called on the button. It also didn’t set btn->owner at all (left as NULL from xrdp_bitmap_create()).
The problem: Two issues compounded:
- The dispatch calls
control->parent->notify(...), notcontrol->notify(...). Sobtn->notifywas never called. The parent window (session_selector_window) had no notify callback set, so the dispatch was a no-op. - Even if the parent’s notify was set, the dispatch passes
control->owneras the first argument. Sincebtn->ownerwas NULL, the callback receivedwnd = NULL, andwnd->wmsegfaulted.
Root cause: Misunderstanding of the UI framework’s event dispatch model. The notify callback belongs to the parent window, not the child control. The owner field must be set for the callback to receive valid arguments.
Fix:
- Set
session_selector_window->notify = xrdp_wm_session_selector_notifyon the parent window. - Set
btn->owner = self->session_selector_windowon all buttons (New Session, Cancel, and session buttons). - Removed the incorrect
btn->notifyassignments.
13. WMLS_INACTIVE Disconnecting the RDP Client #
Context: The xrdp window manager has a login state machine that controls the lifecycle of an RDP connection. The states are: WMLS_RESET → WMLS_USER_PROMPT → WMLS_START_CONNECT → WMLS_CONNECT_IN_PROGRESS → (success) → WMLS_CLEANUP → WMLS_INACTIVE. The WMLS_INACTIVE state is the terminal state — it means “login is done, the session is active, and the window manager has nothing more to do.”
What the previous change tried to do: After a successful connection to the X server, xrdp_wm_mod_connect_done() was called with status = 0. The original code set WMLS_CLEANUP, which then transitioned to WMLS_INACTIVE. In the normal (non-sharing) flow, this is correct — the login UI is cleaned up and the RDP session is handed off to the X server.
The problem: In the xrdp process main loop (xrdp_process_main_loop), when the window manager reaches WMLS_INACTIVE, the main loop exits and calls libxrdp_disconnect(), which sends a disconnect PDU to the RDP client and closes the TCP connection. For session sharing, we need the xrdp child to stay alive and keep the RDP connection open so the client can continue interacting with the shared desktop.
Root cause: WMLS_INACTIVE was designed as a terminal state for the login flow, not as a persistent state for an active session. The main loop interprets it as “we’re done here, time to disconnect.”
Fix: Added WMLS_SESSION_ACTIVE as a new non-terminal state. xrdp_wm_mod_connect_done() now sets WMLS_SESSION_ACTIVE instead of WMLS_CLEANUP. The WMLS_SESSION_ACTIVE handler deletes login UI elements (like WMLS_CLEANUP does) but does not transition to WMLS_INACTIVE — it just returns, keeping the main loop running. Also updated xrdp_wm_can_resize() to allow resizing in this state.
Meta-Level Problems & How We Overcame Them #
The Previous Agent’s Mess #
The session started with the user reporting that the previous AI agent “did a real shitty job.” The codebase had partial implementations with subtle bugs that only manifested under real testing. We had to audit every change, understand the xrdp architecture from scratch, and rebuild the fixes systematically.
Lesson: Always verify assumptions by reading the actual code, not just the commit messages or comments.
Docker vs nerdctl #
Initially tried docker build which failed — the system uses nerdctl (containerd-based). Wasted time on docker commands that silently failed.
Lesson: Check what container runtime is actually available before assuming Docker.
Core Dumps: A Journey #
Getting core dumps to work in a container required three iterations:
- First attempt: core pattern write failed (container can’t write to
/proc/sys/kernel/core_pattern) - Second attempt: Changed CWD to
/tmp/coresso defaultcorepattern would drop files there — still empty - Third attempt:
--privilegedmode + explicit core pattern set = success
Lesson: Containers sandbox core dump handling. You need privileged mode to change the core pattern.
Binary Mismatch with Core Dumps #
The first core dump analysis with gdb showed “no debugging symbols found” and mismatched build IDs. The binaries in the container were built during the Docker image build, not from the local source tree.
Solution: Ran gdb inside the container where the binaries matched the core dumps exactly.
Lesson: When analyzing container crashes, run gdb inside the container or extract the exact binaries from it.
Forgetting to Copy sesexec #
Rebuilt xrdp and xrdp-sesman but forgot that xrdp-sesexec is a separate binary in sesman/sesexec/. The session sharing flag fix was in sesexec, but the container was running the old version. Debugging showed the flag check wasn’t being hit because the old binary didn’t have the code.
Lesson: Map the full binary dependency chain. In xrdp, sesman, sesexec, and xrdp are all separate processes.
Log Confusion #
With multiple connection attempts, the logs became a wall of xrdp_mm_check_wait_objs messages. It was hard to distinguish which flow belonged to which client.
Solution: Used grep -n to get line numbers, then sed -n to extract specific ranges. Also filtered for specific markers like “Connecting to session” vs “Getting session connections.”
Lesson: When debugging multi-client systems, add unique identifiers (PIDs, connection IDs) to log messages.
Build System Quirks #
make install DESTDIR=... failed on a pre-existing symlink issue. Worked around by copying binaries directly from .libs/ directories.
Lesson: Libtool’s .libs/ directories contain the actual built binaries. Don’t rely on make install when you just need the executables.
Key Lessons Learned & Takeaways #
1. Always Match Allocation and Free Patterns #
The double-free bug (Problem #6) was a textbook example of mismatched allocation/free patterns. When a library packs multiple strings into a single allocation, you can’t free them individually. Always trace the allocation path before writing the free path.
2. Buffer Sizes Matter More Than You Think #
128 bytes sounds reasonable for a “simple” protocol message until you have five strings plus a GUID. Size buffers for realistic worst-case payloads, not happy-path minimums.
3. State Machines Are Fragile #
The connect state machine (xrdp_mm_connect_sm) auto-increments connect_state at the end of each loop iteration. If you call it from a response handler without advancing the state first, it re-processes the current state. When calling a state machine from a callback, always ensure the state is correct for the next iteration.
4. Terminal States Are Terminal #
WMLS_INACTIVE was designed as “done, disconnect.” Repurposing it for “done but stay connected” is a category error. Add a new state rather than overloading an existing one.
5. Core Dumps Are Worth the Setup Effort #
Without core dumps, we would have been guessing about the double-free and buffer overflow. Setting up core dumps in a container is annoying but pays off immediately. Invest in crash diagnostics early.
6. Read the Dispatch Chain #
The button click bug (Problem #12) was caused by not understanding how the notify dispatch chain works: control->parent->notify(control->owner, ...). Setting btn->notify was useless. Always trace the full callback path before wiring up handlers.
7. Test with Real Traffic, Not Just Unit Tests #
Every single bug here was found by actually connecting two RDP clients. Unit tests would have caught the double-free and buffer overflow, but never the state machine re-processing or the sesexec termination issue. Integration testing is irreplaceable for distributed systems.
8. The Previous Agent’s “Shitty Job” Was Actually a Good Starting Point #
Despite the bugs, the previous agent had wired up the core architecture: MPX device creation, session selector UI, GUID passing, and the connect state machine bypass. The bugs were all in the details — buffer sizes, free patterns, state transitions, callback wiring. A broken implementation that compiles is often closer to working than a perfect design that doesn’t.