Wlroots: Sway crashes when an XWayland-client copies large contents to the clipboard and the copyq clipboard manager is running

[zsolt-donca]

Sway Version:

❯ swaymsg -t get_version
sway version 1.5-a1591c23 (Dec  9 2020, branch 'master')

To reproduce this issue (and to get debug symbols), I installed https://aur.archlinux.org/packages/sway-git/ and https://aur.archlinux.org/packages/wlroots-git/, and both should be using the current master branch.

Debug Log:
I obtained the following logs using sway -d 2> ~/sway.log:
https://gist.github.com/zsolt-donca/62f7348ad71de1b566198109612eedef

Configuration File:
I reproduced this issue with the default configuration, with the exception of some keybinding coming from /etc/sway/config.d/swayshot, and of /etc/sway/config.d/50-systemd-user.conf.

Stack Trace:

#0  0x00007f26711255c0 in xcb_get_property_value_length () at /usr/lib/libxcb.so.1
swaywm/sway#1  0x00007f26711f0b8b in xwm_data_source_write (fd=90, mask=<optimized out>, data=0x564108ffa5f0) at ../xwayland/selection/incoming.c:22
        transfer = 0x564108ffa5f0
        xwm = 0x564108ffa490
        property = 0x20 <error: Cannot access memory at address 0x20>
        remainder = <optimized out>
        len = <optimized out>
swaywm/sway#2  0x00007f267123efaa in wl_event_loop_dispatch () at /usr/lib/libwayland-server.so.0
swaywm/sway#3  0x00007f267123d4e7 in wl_display_run () at /usr/lib/libwayland-server.so.0
swaywm/sway#4  0x00005641074c0422 in server_run (server=<optimized out>) at ../sway/sway/server.c:247
swaywm/sway#5  0x00005641074b4614 in main (argc=2, argv=0x7ffcacd2fc58) at ../sway/sway/main.c:412
        verbose = 0
        debug = 1
        validate = 0
        allow_unsupported_gpu = 0
        long_options = 
            {{name = 0x56410750647b "help", has_arg = 0, flag = 0x0, val = 104}, {name = 0x564107509da9 "config", has_arg = 1, flag = 0x0, val = 99}, {name = 0x564107506480 "validate", has_arg = 0, flag = 0x0, val = 67}, {name = 0x564107506489 "debug", has_arg = 0, flag = 0x0, val = 100}, {name = 0x5641075063df "version", has_arg = 0, flag = 0x0, val = 118}, {name = 0x564107505557 "verbose", has_arg = 0, flag = 0x0, val = 86}, {name = 0x56410750648f "get-socketpath", has_arg = 0, flag = 0x0, val = 112}, {name = 0x56410750649e "unsupported-gpu", has_arg = 0, flag = 0x0, val = 117}, {name = 0x5641075064ae "my-next-gpu-wont-be-nvidia", has_arg = 0, flag = 0x0, val = 117}, {name = 0x0, has_arg = 0, flag = 0x0, val = 0}}
        config_path = 0x0
        usage = 0x564107506820 "Usage: sway [options] [command]\n\n  -h, --help", ' ' <repeats 13 times>, "Show help message and quit.\n  -c, --config <config>  Specify a config file.\n  -C, --validate         Check the validity of the config file, th"...
        c = <optimized out>

Description:
Sway crashes when an XWayland-client copies the contents of a large file to the clipboard and the copyq clipboard manager is running. The issue does not appear I am copying the same file contents from a Wayland-native app, such as gedit. Also, the issue is not present if the copyq clipboard manager is not running. When sway crashes, I lose my entire session and get back to the TTY login.

I am not very convinced that this issue is indeed related to the size of the clipboard contents, as so far it reproduced to me with two files only, both being large-ish. It most definitely does not reproduce with the contents of just any large file. So far I couldn't find a pattern.

I am using a wayland-native build of copyq, using the wayland branch (https://github.com/hluk/CopyQ/tree/wayland).

The issue happens to me very random, only with some files (few), but it always reproduces with the same file. In the latest case the file is just 330KiB, and is a .csv (not sure if relevant). I cannot share the file itself, as it contains sensitive data.

I followed exactly the following steps to reproduce the issue and to obtain the above logs and stack trace:

1. Started sway;
2. Opened copyq;
3. Opened jetbrains-toolbox;
4. Started IntelliJ IDEA from jetbrains-toolbox;
5. Opened a specific file that I know to reproduce the issue;
6. Selected all contents of the file and hit CTRL+C.

Even though the issue is very specific, I feel I need to raise this issue as the impact of losing all my running apps and my work is very inconvenient. I feel like sway should never, ever crash, even if this turns out to be a bug in some other component (copyq, xwayland).

[Xyene]

property being 0x20 indicates memory corruption, around here:

https://github.com/swaywm/wlroots/blob/5012121d331c97bc6af856dd58d68b5be336b573/xwayland/selection/incoming.c#L17-L21

Could you add this line to wlroots:

diff --git a/xwayland/selection/incoming.c b/xwayland/selection/incoming.c
index ceca1190..3db6518c 100644
--- a/xwayland/selection/incoming.c
+++ b/xwayland/selection/incoming.c
@@ -15,6 +15,7 @@
  * Write the X11 selection to a Wayland client.
  */
 static int xwm_data_source_write(int fd, uint32_t mask, void *data) {
+   wlr_log(WLR_DEBUG, "xwm_data_source_write(%d, %08x, %p)", fd, mask, data);
    struct wlr_xwm_selection_transfer *transfer = data;
    struct wlr_xwm *xwm = transfer->selection->xwm;

and recompile with ASan enabled? You can find instructions on how to do so here: Here are some instructions to compile from source: https://github.com/swaywm/sway/wiki/Development-Setup#compiling-as-a-subproject

[zsolt-donca]

@Xyene
I cannot run my local build of sway from TTY following the steps outlined in the wiki, because it gives me this error:

00:00:01.277 [ERROR] [wlr] [backend/session/logind.c:107] Failed to stat device '10': Bad file descriptor
00:00:01.295 [ERROR] [wlr] [backend/session/logind.c:107] Failed to stat device '9': Bad file descriptor

I am having the same error with our without the address sanitizer enabled, and with or without the debug log statement that I added.

Please find the full logs here.

For the next thing I will try to build just wlroots by the AUR package, with the logging statement and the address sanitizer enabled, and install it system-wise. I hope to succeed, but for further investigations, it would be great if somebody could tell me what is going wrong with me compiling it as a sub-project.

[zsolt-donca]

I succeeded with the AUR packages: I rebuilt both sway and wlroots with the address sanitizer argument (-Db_sanitize=address), and with the log statement added to wlroots.

Please find the logs here: https://gist.github.com/zsolt-donca/78f9d7c8298c72926a9514354d1790b6

[Xyene]

Thanks, this confirmed what I expected that the source is firing an event when it should already be removed. Perhaps we're registering the transfer twice.

Can you also add

wlr_log(WLR_DEBUG, "transfer %p, source %p, fd %p, reply %p", transfer,  transfer->source, transfer->source_fd, reply);

before this line?

Also, it would be really helpful if you can try to see if you can reproduce this in Weston. wlroots' selection code is nearly identical to Weston's, so it'd be strange if this doesn't fail in Weston as well.

[zsolt-donca]

Here are the latest logs: https://gist.github.com/zsolt-donca/72e67feb89ee67a25b3ae69df2dec159

Will try with Weston.

[zsolt-donca]

I cannot reproduce this issue on weston because copyq doesn't support it - on startup it gives an error related to the clipboard, and otherwise doesn't seem to work. On their current "wayland" development branch, they state that not all compositors are supported: https://github.com/hluk/CopyQ/issues/27#issuecomment-723546751

Clipboard/selection support - this depends on compositor - tested with Sway which supports the clipboard manager protocol.

[zsolt-donca]

I've just found a simpler way to reproduce the issue, one that doesn't involve copyq: I copy the text from my xwayland client (intellij idea in my case), and paste it into some wayland-native application (such as chromium 87 started with the flags --enable-features=UseOzonePlatform --ozone-platform=wayland).

This also works on weston, and weston indeed crashes with this stack trace:

Core was generated by `weston'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007fbee2fa85c0 in xcb_get_property_value_length () from /usr/lib/libxcb.so.1
[Current thread is 1 (Thread 0x7fbef2fc48c0 (LWP 6426))]
(gdb) bt full
#0  0x00007fbee2fa85c0 in xcb_get_property_value_length () at /usr/lib/libxcb.so.1
#1  0x00007fbee2ddb18d in  () at /usr/lib/libweston-9/xwayland.so
#2  0x00007fbef34f3faa in wl_event_loop_dispatch () at /usr/lib/libwayland-server.so.0
#3  0x00007fbef34f24e7 in wl_display_run () at /usr/lib/libwayland-server.so.0
#4  0x00007fbef37523b8 in wet_main () at /usr/lib/weston/libexec_weston.so.0
#5  0x00007fbef357e152 in __libc_start_main () at /usr/lib/libc.so.6
#6  0x000055d0fab8605e in  ()

(UPDATE: I fixed the stracktrace, it looks like that originally I accidentally pasted some previous stack-trace belonging to swaywm)

[Xyene]

Thanks for confirming this doesn't work in Weston either. Could you go ahead and open a ticket there? I can guess at a patch (below), but I'm not very familiar with the X11 incremental copy protocol.

In the meantime, I suspect replacing xwm_write_property with something like the below should prevent the crash. I can't guarantee it won't break copy/paste entirely, though.

static void xwm_write_property(struct wlr_xwm_selection_transfer *transfer,
        xcb_get_property_reply_t *reply) {
    if (transfer->property_reply == NULL && reply != NULL) {
            struct wlr_xwm *xwm = transfer->selection->xwm;
        struct wl_event_loop *loop =
            wl_display_get_event_loop(xwm->xwayland->wl_display);
        transfer->source = wl_event_loop_add_fd(loop,
            transfer->source_fd, WL_EVENT_WRITABLE, xwm_data_source_write,
            transfer);
    }

    transfer->property_start = 0;
    transfer->property_reply = reply;

    xwm_data_source_write(transfer->source_fd, WL_EVENT_WRITABLE, transfer);
}

[zsolt-donca]

I just opened a bug report in weston: https://gitlab.freedesktop.org/wayland/weston/-/issues/461

Thanks for the patch, I will try it soon and get back with the results.

[zsolt-donca]

@Xyene The patch seems to be working! 🎊 There are no more crashes, and copy-pasting seems to work just fine even with larger contents, at least as long as I don't have copyq running.

The only issue I've found is around copyq: if I have it running, pasting doesn't work immediately after copying, and I need to try to paste a couple of times to get something out, and even then the output is messed up with sections of the text repeating. If I take a look at the contents of the clipboard inside copyq's UI, there isn't only a single entry (as it should), but multiple ones with various fragments of the text, and also some weird binary-looking entries. Can this be because of issues with the above code? Either way, I am running copyq's experimental wayland branch, so this might be just a bug in it.

Will give this patch some further testing tomorrow with more applications and files, to see if everything is all right.

[Xyene]

Yeah, I'm not surprised it doesn't work well. What's basically happening is:

• the X11 application requests the contents of the clipboard
• the clipboard contents are too large to be sent in one go, so they have to be sent incrementally
• wlroots starts sending chunks of clipboard data
• partway through this, the application requests the contents of the clipboard again

I'm not sure what should be done in this case. The patch above just resets the transfer to the beginning, which is probably why you're seeing garbled text: you'd get part of the old transfer and part of the new.

Here's another (again untested) take, ignoring the second transfer request and just servicing the first to completion:

static void xwm_write_property(struct wlr_xwm_selection_transfer *transfer,
        xcb_get_property_reply_t *reply) {
    if (transfer->source != NULL) {
        return;
    }

    if (reply != NULL) {
        struct wlr_xwm *xwm = transfer->selection->xwm;
        struct wl_event_loop *loop =
            wl_display_get_event_loop(xwm->xwayland->wl_display);
        transfer->source = wl_event_loop_add_fd(loop,
            transfer->source_fd, WL_EVENT_WRITABLE, xwm_data_source_write,
            transfer);
    }

    transfer->property_start = 0;
    transfer->property_reply = reply;

    xwm_data_source_write(transfer->source_fd, WL_EVENT_WRITABLE, transfer);
}

[zsolt-donca]

@Xyene Thanks for the updated patch! Unfortunately, I don't seem to notice any difference in copyq's behavior: it still produces lots of entries, none of them seem to be complete (though it's hard to tell), some of them binary garbage.

But I am not so sure that the issue is in wlroots anymore. Copy/pasting now works flawlessly from xwayland-clients to wayland-native apps (with either of your patches), and with the latest patch I've also tried another clipboard manager, clipman, and it seems to work just fine, including with large files.

I will raise this issue in copyq's wayland merge request, to see what they think about this.

In the meantime, what do you think should be the next step in here? Do you want to wait for feedback on https://gitlab.freedesktop.org/wayland/weston/-/issues/461 first, or do you want to merge something sooner than that?

[Xyene]

Yeah, I'd like to wait to hear what the Weston folks have to say before merging anything. My patch just hides the symptoms, but doesn't resolve the root issue. While I suspect it's a buggy application/framework (there've been a number of Qt-induced clipboard issues before), it'd be good to get some confirmation from someone who actually knows what they're talking about :)

[Xyene]

@zsolt-donca I've become more familiar with X11 and Wayland copy protocols through some work on other projects, so I can say that my original patch was in fact just hiding the symptoms.

I've begun working on restructuring how wlroots does transfers, which will hopefully resolve a lot of the weird clipboard issues people have been experiencing. https://github.com/swaywm/wlroots/pull/2687 is the first PR of that work, and should also fix this issue. If you have a chance, I'd be interested in whether it works for you.

[zsolt-donca]

@Xyene First of all, thanks for working on this! However, I am sorry to say but for me the new code seems to behave worse than the previous one. After copying some large text, I get random behavior when pasting: it either pastes some random binary, parts of the text, or the full text. Even just pressing CTRL+V consecutively a couple of times, without changing the contents of the clipboard, results in different things being pasted (either random binary, parts of the text, or full text).

When copyq is running, it seems to behave very similarly as it did with the previous fixes: right after copying some large text to the clipboard, there are several entries in copyq's clipboard history, some of them random binary, some parts of the original text, and there is also usually an entry with the full text. Pasting will always paste the latest entry.

Note that I am talking about copying some large text (I tested with 1.5 MB) from an xwayland-client app, and pasting it into a wayland-native app. With the previous code, copy/paste of large text was working just fine when not using copyq.

Are there any logs or similar that could help investigate why the new code is behaving the way it is?

EDIT: If it helps, I can provide a very thorough step-by-step guide to reproduce the issue with generally available tools, such as gedit started with X11-backend, and similar.

[Xyene]

Thanks for testing -- yes, just the Sway logs with -d -V flags would be helpful.

The new PR isn't a fix for copyq misbehaving (it's just some groundwork). However, the PR should be an improvement over the current behavior of causing a compositor crash; in the long-term I understand what's happening and ~how to fix it. (And it should be upstreamable, unlike my original take.)

If the new PR broke regular copy-paste for you (without copyq in the middle), then I'd definitely be interested in logs. I've been testing with copies from X11 CLion to Wayland gedit.

[zsolt-donca]

I've found that it actually matters what application I copy from and what application I paste into. When writing my previous comments, so far I've been pretty much always copying from IntelliJ IDEA (xwayland) and pasting into gedit (wayland). With the latest patch this worked fine, and also does work fine with the code from https://github.com/swaywm/wlroots/pull/2687. Yesterday, I was doing something different: copying from gedit started with GDK_BACKEND=x11 and pasting into Code - OSS, running in wayland-native mode. This produced me the behavior I was explaining yesterday, and it's possible that it would have been the same with the previous patch.

Today, I repeated my tests, with copyq not running:

1. copying from IntelliJ IDEA (xwayland) and pasting into gedit (wayland) works fine, 100% of the cases.
2. copying from IntelliJ IDEA (xwayland) and pasting into Code - OSS (wayland) works fine, 100% of the cases.
3. copying from IntelliJ IDEA (xwayland) and pasting into Chrome (v89, latest dev build) does not work fine: it produces some random binary garbage, though always the same for the same contents. When pasting several times, there is also a chance for Chrome to completely hang. This is only if the text I copy is large: if I copy something small enough, it works fine. If I keep CTRL+V pressed for a couple of seconds while it keeps pasting the binary garbage actually causes sway to segfault!
4. copying from gedit (xwayland) and pasting into Chrome produces the same thing as in step 3.
5. copying from gedit (xwayland) and pasting into Code - OSS (wayland) does not work fine: it produces the issue I explained yesterday, that is, pasting produces randomly one of the following: random binary garbage, large chunk of the original text with some binary garbage in it, full original text. Even pasting consecutively can produce different things.

You can find the logs here for step 3, step 5, and also from a sway session crashing as explained in step 3:

https://gist.github.com/zsolt-donca/c365cb2813ef6a5b0572036530700d32

[Xyene]

Thanks for this. From the logs this does look like a crash I'm aware of, that would've happened before and that I'm looking to fix :)

The cause is:

• clipboard transfers are asynchronous and you need an X11 window for one
• wlroots only has a single X11 window for X11 → Wayland transfers
• a Wayland application can request the clipboard simultaneously 2+ times
• wlroots tries to use the same window for two transfers
  
  
  
  • data gets corrupted (you have two streams of data coming down the same X11 property, mixing)
  
  
  • wlroots also loses track of some fds, and this leads to a use-after-free
  
  

Actually fixing this requires reworking wlroots to use a per-transfer X11 window. A temporary hack I have applied on my install is:

diff --git a/xwayland/selection/incoming.c b/xwayland/selection/incoming.c
index 3402d9a4..d7e81e8e 100644
--- a/xwayland/selection/incoming.c
+++ b/xwayland/selection/incoming.c
@@ -151,6 +151,16 @@ static void source_send(struct wlr_xwm_selection *selection,
        return;
    }

+   if (transfer->wl_client_fd > 0) {
+       wlr_log(WLR_ERROR, "source_send fd %d, but %d already in progress",
+               fd, transfer->wl_client_fd);
+       if (transfer->wl_client_fd != fd) {
+           close(fd);
+       }
+
+       return;
+   }
+
    xcb_convert_selection(xwm->xcb_conn,
        selection->window,
        selection->atom,

This nacks a Wayland app's clipboard request if there is a transfer already running, preventing the use-after-free. Depending on the application, you get lucky and the transfer that completes is the one the application actually cares about.

Here's some logs showing the issue:

$ WAYLAND_DEBUG=1 ./chromium-87.0.4280.66/out/Release/chrome --enable-features=UseOzonePlatform --ozone-platform=wayland --no-sandbox |& grep data
...
[2476354.821] [email protected](wl_data_offer@4278190080)
[2476354.826]  -> [email protected]()
--- pressed Ctrl+V ---
[2481241.501]  -> [email protected]("text/plain", fd 194)
[2481242.230]  -> [email protected]("text/plain", fd 194)

Note that Chrome calls wl_data_offer.receive twice. In principle, it shouldn't, but it's legal — and fine under wlroots' current implementation for small transfers that are (in practice, but not actually) ~atomic. This breaks for longer-running X11 incremental transfer operations, where the first transfer doesn't complete by the time Chrome sends the second wl_data_offer.receive request.

Compare this to gedit:

$ WAYLAND_DEBUG=1 gedit |& grep data --line-buffered
...
[2667233.495]  -> [email protected]()
--- pressed Ctrl+V ---
[2668576.004]  -> [email protected](56232, "text/plain;charset=utf-8")
[2668576.023]  -> [email protected]("text/plain;charset=utf-8", fd 20)

Note the single call to wl_data_offer.receive. This will work as long as your Ctrl+V repeat rate is < the time it takes for a transfer to complete :)

[Xyene]

I've just pushed another commit (https://github.com/swaywm/wlroots/pull/2687/commits/cd618d31732c08c855d08f3ec1ee22f724a07038) to the PR that solves the Chrome garbage / hang issue, at least on Chrome 87. Tested by copying a 100K-line file from CLion into https://ace.c9.io/ running in Chrome; logs now show a clean transfer. I wasn't able to reproduce the segfault in the first place, though. This was an existing bug as far as I can tell.

[zsolt-donca]

I have some great news: your latest commit (https://github.com/swaywm/wlroots/commit/cd618d31732c08c855d08f3ec1ee22f724a07038) seems to completely solve the issue for me! All those 5 tests that I outlined before now pass, flawlessly, 100% of the time, both with and without copyq running!

Also thanks for the detailed explanation, it does make sense.

[emersion]

Nice!

Is there another unresolved bug in this issue, or can we close it?

[zsolt-donca]

I can confirm that the issue is no longer present with the latest build on master, now that I see that @Xyene 's commits were merged, including https://github.com/swaywm/wlroots/commit/cd618d31732c08c855d08f3ec1ee22f724a07038.

Thank you, @Xyene ! :tada: :clap: