This is less cryptic to read than letters, and allows the compiler
to check switch statements exhaustiveness.
Signed-off-by: Simon Ser <contact@emersion.fr>
There are situations in which a call into wl_client_destroy() can
result in a reentrant call into wl_client_destroy() - which
results in UAF / double free crashes.
For example, this can occur in the following scenario.
1. Server receives a message notifying it that a client has
disconnected (WL_EVENT_HANGUP [1])
2. This beings client destruction with a call to wl_client_destroy()
3. wl_client_destroy() kicks off callbacks as client-associated
resources are cleaned up and their destructors and destruction
signals are invoked.
4. These callbacks eventually lead to an explicit call to
wl_display_flush_clients() as the server attempts to flush
events to other connected clients.
5. Since the client has already begun destruction, when it is
reached in the iteration the flush fails wl_client_destroy()
is called again [2].
This patch guards against this reentrant condition by removing
the client from the display's client list when wl_client_destroy()
is first called. This prevents access / iteration over the client
after wl_client_destroy() is called.
In the example above, wl_display_flush_clients() will pass over
the client currently undergoing destruction and the reentrant
call is avoided.
[1] 8f499bf404/src/wayland-server.c (L342)
[2] 8f499bf404/src/wayland-server.c (L1512)
Signed-off-by: Thomas Lukaszewicz [thomaslukaszewicz@gmail.com](mailto:thomaslukaszewicz@gmail.com)
- wayland-egl-abi-check: try to use llvm-nm first instead of BSD nm (incompatible options)
- avoid forcing _POSIX_C_SOURCE=200809L (SOCK_CLOEXEC become available)
- epoll(7) is provided by a userspace wrapper around kqueue(2) as FreeBSD
- when using SO_PEERCRED, the struct to use is `struct sockpeercred` instead of `struct ucred` on OpenBSD
- provide a compatibility layer for count_open_fds() using sysctl(2) as FreeBSD
Signed-off-by: Sebastien Marie <semarie@online.fr>
The wl_output events should not be used anymore for guessing the
preferred scale and transform of a surface. We have explicit events
for that now.
Signed-off-by: Simon Ser <contact@emersion.fr>
The only way to attach some data to a wl_client seems to be setting up a
destroy listener and use wl_container_of. Let's make it straight forward
to attach some data.
Having an explicit destroy callback for the user data makes managing the
user data lifetime much more convenient. All other callbacks, be they
wl_resource request listeners, destroy listeners or destructors, or
wl_client destroy listeners, can assume that the wl_client user data
still exists if it was set. Otherwise making that guarantee would be
complicated.
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Sebastian Wick <sebastian@sebastianwick.net>
Currently it is possible to iterate over client-owned resources
during client destruction that have had their associated memory
released.
This can occur when client code calls wl_client_destroy(). The
following sequence illustrates how this may occur.
1. The server initiates destruction of the connected client via
call to wl_client_destroy().
2. Resource destroy listeners / destructors are invoked and
resource memory is freed one resource at a time [1].
3. If a listener / destructor for a resource results in a call
to wl_client_for_each_resource(), the iteration will proceed
over resources that have been previously freed in step 2,
resulting in UAFs / crashes.
The issue is that resources remain in the client's object map
even after they have had their memory freed, and are removed
from the map only after each individual resource has had its
memory released.
This patch corrects this by ensuring resource destruction first
invokes listeners / destructors and then removing them from the
client's object map before releasing the associated memory.
[1] https://gitlab.freedesktop.org/wayland/wayland/-/blob/main/src/wayland-server.c?ref_type=heads#L928
Signed-off-by: Thomas Lukaszewicz thomaslukaszewicz@gmail.com
"is incompatible with the implementation in libwayland" is a common
source of confusion as evidenced by repeated discussions in IRC
channel.
Improve the wording by making clear that
- packing IDs is a protocol requirement
- there are implementations (including libwayland) that enforce it
Signed-off-by: Mikhail Gusarov <dottedmag@dottedmag.net>
Allow setting a name for an event queue. The queue is used only for
printing additional debug information.
Debug output can now show the name of the event queue an event is
dispatched from, or the event queue of a proxy when a request is made.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Some code paths that lead to a client error and connection termination
have no associated logging, or insufficient logging. This makes it
difficult to understand what went wrong. This commit adds or supplements
logging for all these code paths.
Signed-off-by: Erik Chen <erikchen@chromium.org>
wl_connection_write() contained an exact copy of the logic in
wl_connection_queue(). Simplify things by just calling
wl_connection_queue() from wl_connection_write().
Signed-off-by: John Lindgren <john@jlindgren.net>
For libs/cflags this is done automatically, but not for manually accessed
variables. This matches what wayland-protocols does.
Signed-off-by: Andreas Cord-Landwehr <cordlandwehr@kde.org>
Allows clients to cleanly release wl_shm objects. Useful for clients
using multiple wl_registry objects (e.g. via libraries).
Signed-off-by: Simon Ser <contact@emersion.fr>
This was hardcoded to 1 regardless of the version passed to the
callback or the version of the parent resource.
Signed-off-by: Simon Ser <contact@emersion.fr>
Since the positivity of zero is debatable, and, in some cases scale was simply
underspecified, clarify the situation.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
The cursor name spec [1] describes how cursors should be named,
and is widely used. Add aliases so that users can pass these
names to libwayland-cursor without having to add fallbacks for
X11 cursor names.
[1]: https://www.freedesktop.org/wiki/Specifications/cursor-spec/
Signed-off-by: Simon Ser <contact@emersion.fr>
This adds a command to re-generate the test data. This needs to be
done when either an XML source file or the scanner's output is
changed.
Signed-off-by: Simon Ser <contact@emersion.fr>
The spec does not describe which actions cause the compositor to assign
keyboard focus to a surface, leaving this up to the compositor.
Compositors differ in their behavior when the user clicks on a
sub-surface. Some will move the keyboard focus to the subsurface whereas
others will only ever assign the keyboard focus to toplevel surfaces.
Some applications (e.g. firefox) seem to require the second behavior.
This patch specifies that sub-surfaces never get the keyboard focus.
Signed-off-by: Julian Orth <ju.orth@gmail.com>
Don't mention when the parent surface state is applied; the parent
surface isn't necessarily a sub-surface.
Signed-off-by: Kirill Primak <vyivel@eclair.cafe>
This should be sufficient for clients to not decide to fallback to
output based logic to determine scaling/transform when compositor
doesn't send any of the v6 events.
Signed-off-by: Kirill Chibisov <contact@kchibisov.com>
The issue template is hard to notice because it's not the default.
Users have to explicitly select it from the easy-to-miss dropdown
to get the warning.
Make the template the default one, so that new users are less likely
to miss it.
Signed-off-by: Simon Ser <contact@emersion.fr>
Due to what is arguably a mistake in the C language specification,
passing NULL to memcpy and friends is undefined behavior (UB) even when
the count is 0. C additionally mistakenly leaves NULL + 0 and NULL -
NULL undefined. (C++ fixes this mistake.) These are very problematic
because (NULL, 0) is a natural representation of the empty slice.
Some details:
https://github.com/llvm/llvm-project/issues/49459https://www.imperialviolet.org/2016/06/26/nonnull.html
Unfortunately, despite how clearly this is a mistake, glibc headers and
GCC now try to exploit this specification mistake and will miscompile
code, so C projects need to workaround this. In particular, UBSan from
Clang will flag this as a bug (although Clang itself has the good sense
to never lean on this bug). We've run into a few UBSan errors in
Chromium stemming from Wayland's memcpy calls. Add runtime guards as
needed to avoid these cases.
Note: Chromium's copy of wayland has
https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/188
applied. It is possible the ring_buffer_copy UB cases are only reachable
with that MR applied, I'm not sure. But it seemed simplest to just add
the fix to wayland as-is. Then when/if that MR lands, it will pick this
up.
Signed-off-by: David Benjamin <davidben@google.com>
This can be useful for additional validation purposes when handling
proxies. This is similar to existing server side API
wl_global_get_display.
Signed-off-by: David Edmundson <david@davidedmundson.co.uk>
Newer ci-templates contains bugfixes.
While at it, stop using a GitLab YAML reference, because we only
use this value in one spot.
Signed-off-by: Simon Ser <contact@emersion.fr>
Wayland debug logs resemble email addresses. This is a problem when
anonymizing logs from users. For example:
[2512874.343] xdg_surface@700.configure(333)
In the above log line, the substring "surface@700.config" can be
mistaken for an email address and redacted during anonymization.
Signed-off-by: Alex Yang <aycyang@google.com>
The way we're wrapping libc functions via dlsym() is pretty fragile
and breaks on FreeBSD. The failures happen in our CI and are pretty
random, see e.g. [1].
Use a more manual way to wrap via a function pointer.
[1]: https://gitlab.freedesktop.org/wayland/wayland/-/jobs/44204010
Signed-off-by: Simon Ser <contact@emersion.fr>
Use bool instead of int for boolean values, to make it more
explicit what the field contains. For instance "error" is not to
be confused with an error code.
This is all private API.
Signed-off-by: Simon Ser <contact@emersion.fr>
The offset in wl_surface.attach has been superseded by
wl_surface.offset. Refer to the new request instead of using the
deprecated one.
Signed-off-by: Simon Ser <contact@emersion.fr>
There are two ways to do pre-multiplication of the alpha channel into
the color channels: on optical values or on electrical values. While
pre-multiplication with optical values is arguably more correct, because
operations like blending or scaling require pre-multiplied, optical
color channels, wayland and compositors by default work with
pre-multiplied electrical values. This is most likely a convention that
Wayland took from Cairo.
This commit makes sure that the expectation of pre-multiplied electrical
values is properly documented.
Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
If wl_event_loop_dispatch() fails, we could enter an infinite loop,
repeatedly calling a failing wl_event_loop_dispatch() forever.
Signed-off-by: Simon Ser <contact@emersion.fr>
the 'has_timers' flag can be returned directly without having to track all the ready events
when a timer is found ready.
Signed-off-by: Yang Wang <KevinYang.Wang@amd.com>
