Lucas Stach authored 2 years ago and Michel Dänzer committed 2 years ago

Check the fd for validity before giving a success return code.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Simon Ser <contact@emersion.fr>
Tested-by: Guido Günther <agx@sigxcpu.org>

Olivier Fourdan authored 2 years ago and Olivier Fourdan committed 2 years ago

Xwayland uses API such as wl_proxy_set_tag()/wl_proxy_get_tag() which
appeared in Wayland 1.18, but the build system still requires Wayland
1.5 at least.

Bump the Wayland version to match the requirements.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>

Olivier Fourdan authored 2 years ago and Olivier Fourdan committed 2 years ago

Now that we keep the Wayland surface around for longer than the
xwl_window, we might get events for that surface after the X11 window
is unrealized.

Make sure we untag the Wayland surface when the Wayland surface is
delayed, to break the wl_surface/xwl_window relationship, so that events
for that surface are discarded by Xwayland.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Fixes: e37f18ee - xwayland: Delay wl_surface destruction

Demi Marie Obenour authored 2 years ago and Olivier Fourdan committed 2 years ago

a77d95af intended to do this, but the
check for “is this rootless or rootful XWayland” was inverted.

Fixes: a77d95af ("xwayland: Prevent Xserver grabs with rootless")
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>

Jan Beich authored 2 years ago and Matt Turner committed 2 years ago

hw/xwayland/xwayland.c:306:10: fatal error: 'X11/extensions/xwaylandproto.h' file not found
 #include <X11/extensions/xwaylandproto.h>
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 2700bc60 ("xwayland: add support for the XWAYLAND extension")

X11 and Wayland requests are unordered, causing a race in the X11 window
and wl_surface association.

To mitigate that race, delay the wl_surface destruction by 1 second,
so that the compositor has time to establish the association before the
wl_surface is destroyed: to see both the wl_surface created and the
WL_SURFACE_ID X11 property set.

This is only a mitigation though, a more robust solution requires a
future dedicated Wayland protocol.

v2: Clean up pending wl_surface destroy on exit as well.

Closes: xorg/xserver#1157


Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Tested-by: Joshua Ashton <joshua@froggi.es>
Tested-by: Sterophonick <sterophonick@gmail.com>
See-also: wayland/wayland-protocols!163

Changes check for trying modesetting driver from if defined(__linux__)
to use meson check for if we built the driver for this platform.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Olivier Fourdan authored 2 years ago and Olivier Fourdan committed 2 years ago

Because of the design of most Wayland compositors, where the compositor
is both a Wayland server and an X11 window manager, any X11 client
issuing a server grab (i.e. XGrabServer()) can possibly hang the whole
desktop when Xwayland is running rootless.

This can happen with e.g. ImageMagick's import command with mutter.

1. "import" is launched and issues an XServerGrab(),
2. Xwayland restricts access to that "import" X11 client alone,
3. mutter continues to process events until it needs to sync with
   Xwayland (there's variability in time before the hang occurs),
4. When mutter does an XSync() (explicitly or implicitly through some
   other Xlib call), it will stop waiting for Xwayland to reply,
5. Xwayland waits for the XServerGrab() to be released by import,
6. "import" waits for a user input to release the XServerGrab(),
7. mutter is stuck waiting on Xwayland and does not process input
   events...

To prevent this, re-route the GrabServer/UngrabServer requests and
pretend the grab works but actually does nothing at all for all clients
but the X11 window manager (which can still issue X11 server grabs, at
its own risks).

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Closes: https://bugzilla.redhat.com/1914021

Despite e957a2e5 ("dix: Add hybrid full-size/empty-clip mode to
SetRootClip"), I was still seeing all X11 client windows flashing when
the root window size changes with rootless Xwayland (e.g. due to
hotplugging a monitor).

Skipping this code for ROOT_CLIP_INPUT_ONLY fixes the issue for me.

Luc Ma authored 2 years ago and Luc Ma committed 2 years ago

Fixes: 84897891 ("ci: Install libxcvt from git")
Signed-off-by: Luc Ma <luc@sietium.com>

Peter Harris authored 4 years ago and Alan Coopersmith committed 2 years ago

The commit 9bf46610 "os: Immediately
queue initial WriteToClient" effectively disables buffering (of all
writes, not just the "initial" write), since the OS's network buffers
will usually be large enough to hold whatever replies we have sent.

This does improve performance when drawing over a Unix socket (I measure
approximtely 10%, not the ~5x mentioned in that commit message, probably
due to the large changes in this area since that commit), but it
decreases performance when drawing over a network due to the additional
TCP packets. This decrease is small (~10%) in most cases, but if the two
machines have mismatched Nagle / tcp_delay settings it can cause
XGetWindowAttributes to take 200ms (because it's composed of two
requests, the 2nd of which might wait for the ack which is delayed).

Avoid network slowdowns by making the immediate flush conditional on
who->local.

Signed-off-by: Peter Harris <pharris@opentext.com>

Eli Schwartz authored 2 years ago and Alan Coopersmith committed 2 years ago

Booleans are supposed to be actual booleans, not strings describing
their value, but Meson has a bug where it silently accepts either one.
It's still wrong to do it though, and other implementations of Meson
such as muon choke on it.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>

Maya Rashish authored 5 years ago and Alan Coopersmith committed 2 years ago

I wanted to simplify the logic, and thought this is a good opportunity
to eliminate local diffs.

I don't want to list OSes without wsfb, because I understand that is a
netbsd/openbsd driver, and always have it as a fallback for us.

Additionally, I understand "fbdev" is linux-specific, so have the logic
match this intent.

Without these, the build jobs would spuriously pass if some of the
expected piglit tests didn't run at all.

v2:
* Use local variables instead of starting their names with underscores
  (Peter Hutterer)

Will make it easier to do more complex shell stuff.

No functional change intended.

v2:
* Use /bin/bash instead of /bin/sh (Peter Hutterer)
* Export environment variables on a separate line (Peter)
* Use "set" command instead of shell command line arguments, for
  consistency with debian-install.sh.

Without this, building a new docker image may pull in new changes from
those repositories, which may affect the CI results.

The commit below was meant to do this, but it slipped through the
cracks.

Fixes: e2dcf310 ("CI: Update gitlab CI to use debian bullseye")

They start failing after rebuilding the docker image. It's not clear
why, but looks like it's triggered by changes in Debian bullseye.

They can be useful also when the job passes.

Can be useful for diagnosing CI issues.

FeepingCreature authored 2 years ago and FeepingCreature committed 2 years ago

_XkbCheckRequestBounds assumes that from..to is at least one byte.
However, request strings can be empty, causing spurious failures in
XkbGetKbdByName calls. To avoid this, before checking bounds make
sure that the length is nonzero.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Most of these came from a mass bcopy() -> memmove() substitution in 1993
with a commit comment of "Ansification (changed bfuncs -> mfuncs)"

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Most of these came from a mass bcopy() -> memmove() substitution in 1993
with a commit comment of "Ansification (changed bfuncs -> mfuncs)"

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Uses ci-fairy from freedesktop/ci-templates

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Fixes accidentally taking the WL_POINTER_AXIS_HORIZONTAL_SCROLL case as
well after the WL_POINTER_AXIS_VERTICAL_SCROLL case, which resulted in
vertical wheel events triggering both vertical and horizontal scrolling.

Fixes: e37eeb7a ("xwayland: Aggregate scroll axis events to fix kinetic scrolling")
Closes: xorg/xserver#1367

This extension exists to serve one purpose: reliably identifying
Xwayland. Previous attempts at doing so included querying root window
properties, output names or input device names. All these attempts are
somewhat unreliable. Instead, let's use an extension - where that
extension is present we have an Xwayland server.

Clients should never need to do anything but check whether the extension
exists through XQueryExtension or search through XListExtensions.

This extension provides a single QueryVersion request only, and
that is only to provide future compatibility if we ever need anything
other than "this extension exists" functionality.

xorg/proto/xorgproto!54



Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Not that it actually matters since the typedef is int32_t anyway, but
this theoretically avoids an erroneous call to wl_fixed_to_double() on
that value.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Commit c7311654 cached the value of ResourceClientBits(), but that value
depends on the `MaxClients` value set either from the command line or
from the configuration file.

For the latter, a call to ResourceClientBits() is issued before the
configuration file is read, meaning that the cached value is from the
default, not from the maximum number of clients set in the configuration
file.

That obviously causes all sort of issues, including memory corruption
and crashes of the Xserver when reaching the default limit value.

To avoid that issue, also keep the LimitClient value, and recompute the
ilog2() value if that changes, as on startup when the value is set from
the the xorg.conf ServerFlags section.

v2: Drop the `cache == 0` test
    Rename cache vars

Fixes: c7311654 - dix: cache ResourceClientBits() value
Closes: xorg/xserver#1310


Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>

Commit 7cdcdfea introduced a new command line option
"-force-xrandr-emulation", however it is missing from the
ddxProcessArgument().

As a result, trying to use that command option would result in a error:

(EE) Unrecognized option: -force-xrandr-emulation

Make sure "-force-xrandr-emulation" is accounted for in Xwayland's
ddxProcessArgument().

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Fixes: 7cdcdfea - xwayland: Add -force-xrandr-emulation switch

GetCountedString did a check for the whole string to be within the
request buffer but not for the initial 2 bytes that contain the length
field. A swapped client could send a malformed request to trigger a
swaps() on those bytes, writing into random memory.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Each string length field was accessed before checking whether that byte
was actually part of the client request. No real harm here since it
would immediately fail with BadLength anyway, but let's be correct here.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

This request accessed &stuff[1] before length-checking everything. The
check was performed afterwards so invalid requests would return
BadLength anyway, but let's do this before we actually access the
memory.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

GetComponentByName returns an allocated string, so let's free that if we
fail somewhere.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Peter Hutterer authored 2 years ago and Povilas Kanapickas committed 2 years ago

No validation of the various fields on that report were done, so a
malicious client could send a short request that claims it had N
sections, or rows, or keys, and the server would process the request for
N sections, running out of bounds of the actual request data.

Fix this by adding size checks to ensure our data is valid.

ZDI-CAN 16062, CVE-2022-2319.

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Peter Hutterer authored 2 years ago and Povilas Kanapickas committed 2 years ago

XKB often uses a FooCheck and Foo function pair, the former is supposed
to check all values in the request and error out on BadLength,
BadValue, etc. The latter is then called once we're confident the values
are good (they may still fail on an individual device, but that's a
different topic).

In the case of XkbSetDeviceInfo, those functions were incorrectly
named, with XkbSetDeviceInfo ending up as the checker function and
XkbSetDeviceInfoCheck as the setter function. As a result, the setter
function was called before the checker function, accessing request
data and modifying device state before we ensured that the data is
valid.

In particular, the setter function relied on values being already
byte-swapped. This in turn could lead to potential OOB memory access.

Fix this by correctly naming the functions and moving the length checks
over to the checker function. These were added in 87c64fc5 to the
wrong function, probably due to the incorrect naming.

Fixes ZDI-CAN 16070, CVE-2022-2320.

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Introduced in c06e27b2

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Peter Hutterer authored 2 years ago and Olivier Fourdan committed 2 years ago

Most similar loops here use a pointer that advances with each loop
iteration, let's do the same here for consistency.

No functional changes.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>