xwm: Fix double-close

When an FD is passed to xcb_connect_to_fd(), xcb takes ownership of that
FD and is responsible for closing it, which it does when
xcb_disconnect() is called.  But the xwayland handler code also keeps a
copy of the FD and closes it via safe_close() in
server_finish_process().

This double-close can cause all sorts of problems if another part of
wlroots allocates another FD between the two closes - the latter close
will close the wrong FD and things go horribly wrong (in my case leading
to use-after-free and segfaults).

Fix this by setting wm_fd[0]=-1 after calling xwm_create(), and ensuring
that xwm_create() closes the FD if startup errors occur.
This commit is contained in:
David Turner 2025-10-20 13:55:00 +01:00
parent 989cffe70d
commit 879243e370
3 changed files with 7 additions and 0 deletions

View file

@ -164,6 +164,7 @@ struct wlr_xwm {
struct wl_listener drop_focus_destroy; struct wl_listener drop_focus_destroy;
}; };
// xwm_create takes ownership of wm_fd and will close it under all circumstances.
struct wlr_xwm *xwm_create(struct wlr_xwayland *wlr_xwayland, int wm_fd); struct wlr_xwm *xwm_create(struct wlr_xwayland *wlr_xwayland, int wm_fd);
void xwm_destroy(struct wlr_xwm *xwm); void xwm_destroy(struct wlr_xwm *xwm);

View file

@ -42,6 +42,9 @@ static void handle_server_start(struct wl_listener *listener, void *data) {
static void xwayland_mark_ready(struct wlr_xwayland *xwayland) { static void xwayland_mark_ready(struct wlr_xwayland *xwayland) {
assert(xwayland->server->wm_fd[0] >= 0); assert(xwayland->server->wm_fd[0] >= 0);
xwayland->xwm = xwm_create(xwayland, xwayland->server->wm_fd[0]); xwayland->xwm = xwm_create(xwayland, xwayland->server->wm_fd[0]);
// xwm_create takes ownership of wm_fd[0] under all circumstances
xwayland->server->wm_fd[0] = -1;
if (!xwayland->xwm) { if (!xwayland->xwm) {
return; return;
} }

View file

@ -2530,6 +2530,7 @@ void xwm_set_cursor(struct wlr_xwm *xwm, const uint8_t *pixels, uint32_t stride,
struct wlr_xwm *xwm_create(struct wlr_xwayland *xwayland, int wm_fd) { struct wlr_xwm *xwm_create(struct wlr_xwayland *xwayland, int wm_fd) {
struct wlr_xwm *xwm = calloc(1, sizeof(*xwm)); struct wlr_xwm *xwm = calloc(1, sizeof(*xwm));
if (xwm == NULL) { if (xwm == NULL) {
close(wm_fd);
return NULL; return NULL;
} }
@ -2544,11 +2545,13 @@ struct wlr_xwm *xwm_create(struct wlr_xwayland *xwayland, int wm_fd) {
xwm->ping_timeout = 10000; xwm->ping_timeout = 10000;
// xcb_connect_to_fd takes ownership of the FD regardless of success/failure
xwm->xcb_conn = xcb_connect_to_fd(wm_fd, NULL); xwm->xcb_conn = xcb_connect_to_fd(wm_fd, NULL);
int rc = xcb_connection_has_error(xwm->xcb_conn); int rc = xcb_connection_has_error(xwm->xcb_conn);
if (rc) { if (rc) {
wlr_log(WLR_ERROR, "xcb connect failed: %d", rc); wlr_log(WLR_ERROR, "xcb connect failed: %d", rc);
xcb_disconnect(xwm->xcb_conn);
free(xwm); free(xwm);
return NULL; return NULL;
} }