Commit Graph

33 Commits

Author SHA1 Message Date
Daniel Stone
dfda0ca9d2 xwayland: Use an array for CLOEXEC FDs
This gets us closer to the implementation of weston_client_launch, so we
can reuse that instead of open-coding it.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2023-04-19 08:28:21 +00:00
Daniel Stone
4e6a978e34 xwayland: Don't leak fds on failure
Trivial fixup to not leak socketpairs if we need to exit early in the
function.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2023-04-19 08:28:21 +00:00
Daniel Stone
17331a0c7d frontend: Add FDSTR_INIT macro
This initialises fdstr to 'safe' values so we can reliably deinit them.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2023-04-19 08:28:21 +00:00
Marius Vlad
8c4cdd782e compositor/shared: Suppress write(2) warnings
Fixes the following warnings when building with _FORTIFY_SOURCE
and optimizations enabled:

../shared/xalloc.h:49:9: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
   49 |         write(STDERR_FILENO, oommsg, strlen(oommsg));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

or
../compositor/main.c:427:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
  427 |                         write(STDERR_FILENO, fail_seteuid,
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  428 |                               strlen(fail_seteuid));
      |                               ~~~~~~~~~~~~~~~~~~~~~
../compositor/main.c:434:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
  434 |                         write(STDERR_FILENO, fail_cloexec,
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  435 |                               strlen(fail_cloexec));
      |                               ~~~~~~~~~~~~~~~~~~~~~
../compositor/main.c:442:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
  442 |                         write(STDERR_FILENO, fail_exec, strlen(fail_exec));
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
2022-09-23 11:32:19 +00:00
Derek Foreman
214d48bbab compositor: Fix use after free at shutdown
Another case of forgetting to remove a listener from a list when
signal_emit_mutable fires.

Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
2022-08-03 14:38:53 +03:00
Derek Foreman
c79e1126b6 xwayland: give Xwayland its own session
If we leave xwayland in weston's process group, it can receive
signals from the controlling TTY intended for weston.

The easiest way to see this is to launch weston under gdb, start an
X client, and hit ctrl-c in the gdb session. The Xwayland server
will also catch the SIGINT, and the X client will be disconnected.

Instead, let's call setsid() when launching Xwayland, like we do
for launched clients.

Suggested-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
2022-07-19 14:19:57 +00:00
Daniel Stone
8b238905d7 xwayland: Use os_socketpair_cloexec()
We already have a helper for this; use it.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2022-07-19 14:12:37 +00:00
Daniel Stone
5dbe001661 xwayland: Use custom-env arg handling
Use the arg handling added in the previous commit so that the
environment is completely encapsulated inside the custom env.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2022-07-19 14:12:37 +00:00
Daniel Stone
fafe5f0fc2 custom-env: Prepare for handling args as well as environment
Rename the bits handling environment variables (currently, all of it),
so we have room to handle args as well.

No functional changes.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2022-07-19 14:12:37 +00:00
Daniel Stone
b685e075cd process-util: Move Xwayland fork helpers to shared
We'll want to reuse these inside desktop-shell as well as the Weston
frontend.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2022-07-19 14:12:37 +00:00
Pekka Paalanen
764c2aff8f xwayland: do not check execve() return value
Simplifies the code a bit.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Pekka Paalanen
1bd92dac01 xwayland: do not use setenv() after fork()
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. Painfully, setenv() is not listed as such.

Therefore we must craft our own custom environment, and we get no help
from libc with that.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Pekka Paalanen
d1b01ffb9a xwayland: use execv()
Constructing argv before-hand is a little easier to look at, but this is
mostly just anticipating more changes to how Weston spawns processes in
general.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Pekka Paalanen
a3d7199bea xwayland: use pipe2()
We are already using pipe2() in many places, even in libweston, so let's
simplify the code here as well - not to mention avoid a theoretical
race.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Pekka Paalanen
4c0bdbfde9 xwayland: do not snprintf() after fork()
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. Surprisingly, snprintf() is not such
function. See e.g. https://stackoverflow.com/a/6771799 and how snprintf
is not listed in signal-safety(7) manual.

Therefore we must prepare the fd argument strings before fork(). That is
only possible if we also do not dup() fd in the child process. Hence we
remove the close-on-exec flag instead in the child process which has
copies of the parent's file descriptors. Fortunately fcntl() is safe.

struct fdstr is helping to reduce code clutter a bit.

Additionally, if fork() fails, we now clean up the fds we created.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Pekka Paalanen
77cf8cb006 xwayland: do not weston_log() after fork()
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. weston_log() definitely is not one, it
allocates memory and does whatnot.

weston_log() is also inappropriate for other reasons: the child process
has its own stream buffers and flight-recorder. No-one looks into the
child process' flight recorder, so messages would be lost there. The
logging machinery might also attempt to write into debug streams,
meaning both parent and child could be writing simultaneously.

It seems that the best we can do is to pre-bake an error message and
only write() it out if exec() fails. There is no mention that even
strerror_r() might be safe to call, so we don't.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Pekka Paalanen
71b40fc76b xwayland: move config reading up
Doing any kind of memory allocation calls between fork() and exec() in
the child process is prone to deadlocks and explosions. In general, only
async-signal-safe functions are safe there.

Move the config access to the parent process before fork() to avoid
problems.

See also:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/941#note_1457053

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Pekka Paalanen
e88a622434 Revert "xwayland: Don't dup() displayfd pipe"
This reverts commit 4aa885d4af.

Turns out the problem was not about dupping fds at all, but calling
non-async-signal-safe functions like strdup() between fork() and exec()
in the child process.

For more discussion, see:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/941#note_1457053

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2022-07-12 08:30:22 +00:00
Daniel Stone
18897253d4 xwayland: Add compositor destroy listener to free allocation
Otherwise we just leak this into the void. Not good.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2022-07-05 13:25:37 +01:00
Daniel Stone
4aa885d4af xwayland: Don't dup() displayfd pipe
For some reason, this causes the reads to get completely lost sometimes
in CI.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2022-07-05 13:25:37 +01:00
Daniel Stone
b923802113 xwayland: Refactor argument string construction
Replace an oft-duplicated pattern with a trivial helper function. In
doing so, we observe that the one special case (displayfd 'didn't need
to be CLOEXEC') was wrong, because the X server does fork itself
internally, so there is nothing wrong with setting CLOEXEC.

Signed-off-by: Daniel Stone <daniels@collabora.com>
2022-07-04 11:58:37 +00:00
Dominique Martinet
c2f4201ed2 xwayland: use -displayfd instead of USR1 to signal readiness
We want to wait for Xwayland to be ready before issuing it blocking
requests, but relying on USR1 is a bit unsafe:
 - we can't ascertain the signal originated from Xwayland
 - if weston is started as PID1 (e.g. in its own container), then
   Xwayland will not send SIGUSR1 and X11 connections will be stuck
   forever: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1312

Creating a pipe and using -displayfd, even if we don't care about the
display value itself, is safe and works for all cases

Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
2022-04-18 12:46:04 +03:00
Derek Foreman
869cab4938 xwayland: Simplify HAVE_XWAYLAND_LISTENFD usage
We can use it just once to define a string instead of having preprocessor
conditionals sprinkled about the code.

Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
2022-04-11 08:45:19 +03:00
Emmanuel Gil Peyrot
eff793ab46 Fix indentation all through the project
Fixes a “regression” from 04918f3b0b, but
also other missed pieces.
2021-07-31 15:28:20 +00:00
Alvarito050506
1f57a1f998 compositor: Move child_process_list from global to wet_compositor.
Removes a global variable and makes it easier to avoid adding more
global variables in the next commit.

See https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/652#note_975222

Signed-off-by: Alvarito050506 <donfrutosgomez@gmail.com>
2021-07-06 18:46:09 +00:00
Vlad Zahorodnii
022ea43f9b xwayland: Pass -listenfd instead of -listen <fd>
The -listen <fd> option has been deprecated. Its replacement is -listenfd.

Signed-off-by: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
2021-03-04 09:35:55 +00:00
Antonio Borneo
3957863667 log: remove "%m" from format strings by using strerror(errno)
The printf() format specifier "%m" is a glibc extension to print
the string returned by strerror(errno). While supported by other
libraries (e.g. uClibc and musl), it is not widely portable.

In Weston code the format string is often passed to a logging
function that calls other syscalls before the conversion of "%m"
takes place. If one of such syscall modifies the value in errno,
the conversion of "%m" will incorrectly report the error string
corresponding to the new value of errno.

Remove all the occurrences of the specifier "%m" in Weston code
by using directly the string returned by strerror(errno).
While there, fix some minor indentation issue.

Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
2019-05-02 22:10:30 +02:00
Pekka Paalanen
eebb7dc9ce Rename xwayland-api.h to libweston/xwayland-api.h
See "Rename compositor.h to libweston/libweston.h" for rationale.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2019-04-18 12:31:46 +03:00
Pekka Paalanen
3d5d9476e3 Rename compositor.h to libweston/libweston.h
The main idea is to make libweston users use the form

 #include <libweston/libweston.h>

instead of the plain

 #include <compositor.h>

which is prone to name conflicts. This is reflected both in the installed
files, and the internal header search paths so that Weston would use the exact
same form as an external project using libweston would.

The public headers are moved under a new top-level directory include/ to make
them clearly stand out as special (public API).

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
2019-04-18 12:31:46 +03:00
Alexandros Frantzis
7e2d4beb08 xwayland: Silence format-truncation compilation warnings
We are currently formatting 32-bit signed integers into 8 byte buffers,
which are too small, causing the compiler to complain. Update the buffer
size to the minimum required value of 12 bytes: 1 for the sign, 10 for
the number, and 1 for the null byte terminator.

Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
2018-10-12 13:00:08 +03:00
Bryce Harrington
cff0b1d80f xwayland: Cleanup error message on spawn failure Signed-off-by: Bryce Harrington <bryce@osg.samsung.com>
Signed-off-by: Bryce Harrington <bryce@osg.samsung.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
2016-07-07 10:48:34 -07:00
Bryce Harrington
9335ca5c02 xwayland: Include missing config.h
Signed-off-by: Bryce Harrington <bryce@osg.samsung.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
2016-07-07 10:48:19 -07:00
Giulio Camuffo
9c764df043 xwayland: make the plugin usable by libweston compositors
This patch follows a similar approach taken to detach the backends from
weston. But instead of passing a configuration struct when loading the
plugin, we use the plugin API registry to register an API, and to get it
in the compositor side.  This API allows to spawn the Xwayland process
in the compositor side, and to deal with signal handling.  A new
function is added in compositor.c to load and init the xwayland.so
plugin.

Also make sure to re-arm the SIGUSR1 when the X server quits.

Signed-off-by: Giulio Camuffo <giuliocamuffo@gmail.com>
[Pekka: moved xwayland/weston-xwayland.c -> compositor/xwayland.c]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
2016-07-01 14:34:33 +03:00