Commit Graph

36 Commits

Author SHA1 Message Date
Tudor Brindus aa86a022fa xwayland/selection: make xwm_selection_finish take a wlr_xwm_selection *
Previously it took a wlr_xwm *, which was a bit surprising in that it
freed members of wlr_xwm *, not just its respective selections.
2021-01-31 19:17:04 +01:00
Tudor Brindus b6ba595862 xwayland/selection: destroy all selections on Xwayland restart
Previously, Xwayland could restart, and we'd get events for transfers
pointing to the previous (now freed) xwm instance. This led to
use-after-free segfaults.

Closes #2565.
2021-01-31 10:24:59 +01:00
Tudor Brindus 3417fc0cca xwayland/selection: don't leak Wayland fd if ConvertSelection fails
If our ConvertSelection failed, we would previously leak the pending
Wayland client fd.

Refs swaywm/sway#5946.
2021-01-31 10:24:53 +01:00
Tudor Brindus e0dfc14983 xwayland/selection: don't request another selection while one is pending
This will hopefully be fixed in the future by having separate windows
for each X11-to-Wayland transfer, but until then, let's avoid a
compositor crash.
2021-01-31 10:24:47 +01:00
Tudor Brindus 211c1e23be xwayland/selection: end incr transfer on empty prop, not next selection
Previously, `transfer->incr` was being cleared on the next selection.
However, if the next selection was *also* incremental, it's possible
that `xwm_handle_selection_property_notify` would route us to
`xwm_get_incr_chunk` instead of `xwm_selection_get_data`.
2021-01-29 10:18:03 +01:00
Tudor Brindus 703c17ae41 xwayland/selection: refactor remaining incremental transfer code 2021-01-29 10:18:03 +01:00
Tudor Brindus 23148d283f xwayland/selection: extract out property requests
Apart from reducing duplication, this has the positive side-effect of
allowing all deallocs to use
`xwm_selection_transfer_destroy_property_reply`, as opposed to the
latter and a mix of ad-hoc `free`s.
2021-01-29 10:18:03 +01:00
Tudor Brindus dea94f2bad xwayland/selection: simplify incremental transfer control flow
Previously, if the Wayland client died before an incremental transfer
was complete, the logs would be spammed by "write error to target fd" as
wlroots entered some control flow wherein it'd continually try
scheduling further writes to the already-dead pipe.

This commit contains no behavioral changes, but introduces explicit
handling for draining the X11 selection in case of Wayland client death.
2021-01-29 10:18:03 +01:00
Tudor Brindus 10a2d57055 xwayland/selection: explicitly bail if first write to Wayland fd fails
If `xwm_data_source_write` failed, it's failed permanently. In fact, a
failing `xwm_data_source_write` sets `transfer->property_reply` to
null as part of its error handling.

Instead of relying on an indirect check (whether
`transfer->property_reply` is still non-null), explicitly use the return
value from `xwm_data_source_write`.
2021-01-29 10:18:03 +01:00
Tudor Brindus 40b2e7669a xwayland/selection: make `xwm_data_source_write` return 0 on failure
The `fd` is marked `O_NONBLOCK`, so `write` will never spuriously return
`EINTR`. Therefore, `write` failing is permanent, and we can return 0 to
make the return value meaningful.
2021-01-29 10:18:03 +01:00
Tudor Brindus e75f483aeb xwayland/selection: rename Wayland-facing data and helpers
Previously, wlr_xwm_selection_transfer.source_fd meant:

- the source of data in a Wayland -> X11 copy (good)
- the destination of data in a X11 -> Wayland copy (confusing)

This made reading through xwayland/selection/incoming.c difficult: in
many places, "source" actually means "destination".
2021-01-25 21:02:55 +01:00
Tudor Brindus 0db191d3bf xwayland/selection: prevent fd leak on unsupported MIME type
Since we never end up calling xcb_convert_selection, the file descriptor
ends up getting leaked (i.e., not cleaned up within
xwm_data_source_write).
2021-01-25 09:46:20 +01:00
Tudor Brindus abb56152ff xwayland: use wlr_log_errno instead of %m
Previously, any error would be masked by an internal isatty call:

  24:31:48.174 [DEBUG] [wlr] [xwayland/selection/incoming.c:386] XCB_SELECTION_NOTIFY (selection=277, property=278, target=256)
  24:31:48.174 [ERROR] [wlr] [xwayland/selection/incoming.c:30] write error to target fd: Inappropriate ioctl for device
2021-01-25 09:22:04 +01:00
Tudor Brindus afeb941ca0 xwayland: notify requestor when we fail to respond to their request
We already mostly did this, but there were a couple of branches
(`calloc` failures) where we'd bail without letting the other side know.

Refs swaywm/sway#4007. Likely not going to be a real improvement there
(if `calloc` fails you're already pretty screwed), but it does address a
theoretical possibility.
2020-10-13 09:02:20 +02:00
Tudor Brindus 7bb9d48dd1 xwayland: remove stale transfers from the same requestor
It seems that if we ever try to reply to a selection request after
another has been sent by the same requestor (we reply in FIFO order),
the requestor never reads from it, and we end up stalling forever on a
transfer that will never complete.

It appears that `XCB_SELECTION_REQUEST` has some sort of singleton
semantics, and new requests for the same selection are meant to replace
outstanding older ones. I couldn't find a reference for this, but
empirically this does seem to be the case.

Real (contrived) case where we don't currently do this, and things break:

* run fcitx
* run Slack
* wl-copy < <(base64 /opt/firefox/libxul.so)  # or some other large file
* focus Slack (no need to paste)

fcitx will send in an `XCB_SELECTION_REQUEST`, and we'll start
processing it. Immediately after, Slack sends its own. fcitx hangs for a
long, long time. In the meantime, Slack retries and sends another
selection request. We now have two pending requests from Slack.

Eventually fcitx gives up (or it can be `pkill`'d), and we start
processing the first request Slack gave us (FIFO). Slack (Electron?)
isn't listening on the other end anymore, and this transfer never
completes. The X11 clipboard becomes unusable until Slack is killed.

After this patch, the clipboard is immediately usable again after fcitx
bails. Also added a bunch of debug-level logging that makes diagnosing
this sort of issue easier.

Refs swaywm/sway#4007.
2020-10-12 10:53:42 +02:00
Tudor Brindus feb0e1c74d xwayland: fix use-after-free in selection handling
Fixes #2425.

wlroots can only handle one outgoing transfer at a time, so it keeps a
list of pending selections. The head of the list is the currently-active
selection, and when that transfer completes and is destroyed, the next
one is started.

The trouble is when you have a transfer to some app that is misbehaving.
fcitx is one such application. With really large transfers, fcitx will
hang and never wake up again. So, you can end up with a transfer list
that looks like this:

| T1: started | T2: pending | T3: pending | T4: pending |

The file descriptor for transfer T1 is registered in libwayland's epoll
loop. The rest are waiting in wlroots' list.

As a user, you want your clipboard back, so you `pkill fcitx`. Now
Xwayland sends `XCB_DESTROY_NOTIFY` to let us know to give up. We clean
up T4 first.

Due to a bug in wlroots code, we register the (fd, transfer data
pointer) pair for T1 with libwayland *again*, despite it already being
registered. We do this 2 more times as we remove T3 and T2.

Finally, we remove T1 and `free` all the memory associated with it,
before `close`-ing its transfer file descriptor.

However, we still have 3 copies of T1's file descriptor left in the
epoll loop, since we erroneously added them as part of removing T2/3/4.
When we `close` the file descriptor as part of T1's teardown, we
actually cause the epoll loop to wake up the next time around, saying
"this file descriptor has activity!" (it was closed, so `read`-ing would
normally return 0 to let us know of EOF).

But instead of returning 0, it returns -1 with `EBADF`, because the file
descriptor has already been closed. And finally, as part of error-handling
this, we access the transfer pointer, which was `free`'d. And we crash.
2020-10-11 08:59:08 +02:00
Tudor Brindus ab80ad902e xwayland: using %m in `wlr_log` is broken, use `wlr_log_errno` instead
This one was awful to track down, but calls to `wlr_log` with %m have
the errno masked by the `isatty` call in `log_stderr`. Switch them to
`wlr_log_errno` instead.

Cue quality "how can read(2) POSSIBLY be returning ENOTTY?" moments.
2020-10-11 06:36:23 +02:00
Antonin Décimo d9bb792794 Fix incorrect format parameters 2020-07-27 10:49:19 +02:00
John Chadwick 58bcec9d94 xwm: end transfers when the requestor is destroyed
This improves the failure cases when incremental transfers fail to
complete successfully for one reason or another.
2020-07-03 09:42:36 +02:00
Manuel Stoeckl edb30a6828 Implement serial validation for selection requests
This change tracks, for each wlr_seat_client, the most recent serial
numbers which were sent to the client. When the client makes a
selection request, wlroots now verifies that the serial number
associated with the selection request was actually provided to that
specific client. This ensures that the client that was most
recently interacted with always has priority for its copy selection
requests, and that no other clients can incorrectly use a larger serial
value and "steal" the role of having the copy selection.

Also, the code used to determine when a given selection is superseded
by a newer request uses < instead of <= to allow clients to make
multiple selection requests with the same serial number and have the
last one hold.

To limit memory use, a ring buffer is used to store runs of sequential
serial numbers, and all serial numbers earlier than the start of the
ring buffer are assumed to be valid. Faking very old serials is
unlikely to be disruptive.

Assuming all clients are correctly written, the only additional
constraint which this patch should impose is that serial numbers
are now bound to seats: clients may not receive a serial number
from an input event on one seat and then use that to request
copy-selection on another seat.
2019-06-30 15:01:05 -04:00
emersion 556bf3ac31
data-device: destroy previous source when starting drag
This supersedes f24e17259e and
04c9ca4198. These commits were manually removing
wlr_data_source destroy handlers when starting a new drag. This is error-prone.

Instead, this commit destroys the previous source whenever we start a new drag.
2019-02-05 18:43:06 +01:00
John Chen 819bd3e344 Fix another instance of swaywm/sway#3545. 2019-02-03 15:11:21 +08:00
emersion 1150ff13ce
data-device: make sources inert, rename cancel to destroy 2019-01-24 12:12:55 +01:00
emersion 4cb0697e57 data-device, primary-selection: add request_set_selection
This makes compositors able to block and/or customize set_selection requests
coming from clients. For instance, it's possible for a compositor to disable
rich selection content (by removing all MIME types except text/plain). This
commit implements the design proposed in [1].

Two new events are added to wlr_seat: request_set_selection and
request_set_primary_selection. Compositors need to listen to these events and
either destroy the source or effectively set the selection.

Fixes https://github.com/swaywm/wlroots/issues/1138

[1]: https://github.com/swaywm/wlroots/issues/1367#issuecomment-442403454
2019-01-24 11:38:23 +01:00
emersion 06467d2e12
primary-selection: add a serial argument
The serial needs to be bumped when X11 clients set the selection, otherwise
some Wayland clients (e.g. GTK) will overwrite it when they gain focus.
2019-01-21 19:23:40 +01:00
emersion 9f0720c03a
primary-selection: introduce wlr_primary_selection_source
This is a common interface that can be used for all primary selection
protocols, as discussed in [1]. A new function wlr_seat_set_primary_selection
is added to set the primary selection for all protocols.

The seat now owns again the source, and resets the selection to NULL when
destroyed.

[1]: https://github.com/swaywm/wlroots/issues/1367#issuecomment-442403454
2018-11-29 19:40:28 +01:00
emersion bfa7f4ee0d
gtk-primary-selection: use impl pattern for sources 2018-11-27 20:16:55 +01:00
emersion f001f98cef
gtk-primary-selection: refactor everything, untie from seat
This commits completely refactors wlr_gtk_primary_selection. The goal is to
remove gtk-primary-selection state from the seat and better handle inert
resources where it makes sense.

wlr_seat_client.primary_selection_devices has been removed and replaced by
wlr_gtk_primary_selection_device. This allows us to make offers inert when the
current selection is replaced.

wlr_seat_set_primary_selection has been removed because it relied on wlr_seat
instead of wlr_gtk_primary_selection_device_manager. A new function,
wlr_gtk_primary_selection_device_manager_set_selection (candidate for the
longest function name in wlroots) has been added. It doesn't take a serial
anymore as serial checking only makes sense for set_selection requests coming
from Wayland clients (serial checking is now done in the Wayland interface
implementation).

Since wlr_gtk_primary_selection_device_manager is now required to set the
selection, a new function wlr_xwayland_set_gtk_primary_selection_device_manager
(candidate number two for longest function name) has been added.

Devices are now made inert when the seat goes away.

Future work includes removing the last primary selection bits from the seat,
mainly wlr_seat.primary_selection_source and wlr_seat.events.primary_selection,
replacing those with new fields in wlr_gtk_primary_selection_device. Or maybe
we could keep those in the seat and replace them with a re-usable interface
(for future zwp_primary_selection_v1 support). We need to think how we'll sync
these three protocols (GTK, X11 and wayland-protocols).

See https://github.com/swaywm/wlroots/issues/1388
2018-11-27 18:57:26 +01:00
emersion 811a4d997b
Rename wlr_primary_selection to wlr_gtk_primary_selection 2018-11-23 11:58:56 +01:00
emersion 2d0c5ec78e
Use _POSIX_C_SOURCE, use shm_open 2018-11-06 08:29:23 +01:00
emersion 7cbef15206
util: add wlr_ prefix to log symbols 2018-07-09 22:49:54 +01:00
emersion c119c43b48
xwayland/selection: fix little memory leak on error 2018-04-26 15:59:58 +01:00
Drew DeVault fac2c3e25f
Merge pull request #882 from emersion/unprefix-local-symbols
Remove wlr_ prefix from local symbols
2018-04-26 11:18:01 +02:00
Guido Guenther 085452f9d9 Use correct printf format specifiers for ssize_t
This unbreaks the build on armhf that otherwise fails like

    ../xwayland/selection/incoming.c: In function 'xwm_data_source_write':
    ../include/wlr/util/log.h:34:17: error: format '%ld' expects argument of type 'long int', but argument 6 has type 'ssize_t {aka int}' [-Werror=format=]
      _wlr_log(verb, "[%s:%d] " fmt, wlr_strip_path(__FILE__), __LINE__, ##__VA_ARGS__)
                     ^
    ../xwayland/selection/incoming.c:34:2: note: in expansion of macro 'wlr_log'
      wlr_log(L_DEBUG, "wrote %zd (chunk size %ld) of %d bytes",
      ^~~~~~~
    ../xwayland/selection/incoming.c:34:44: note: format string is defined here
      wlr_log(L_DEBUG, "wrote %zd (chunk size %ld) of %d bytes",
                                              ~~^
                                              %d
2018-04-26 10:46:11 +02:00
emersion 71ca45e2c0
Make sure we don't use others' prefixes 2018-04-25 23:24:58 +01:00
emersion 591ea27cf9
xwayland: refactor selection code 2018-04-03 12:56:54 -04:00