Both ProcXChangeDeviceProperty and ProcXIChangeProperty checked the
property for validity but didn't actually return the potential error.

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

This fixes a use-after-free bug:

When a client first calls ScreenSaverSetAttributes(), a struct
ScreenSaverAttrRec is allocated and added to the client's
resources.

When the same client calls ScreenSaverSetAttributes() again, a new
struct ScreenSaverAttrRec is allocated, replacing the old struct. The
old struct was freed but not removed from the clients resources.

Later, when the client is destroyed the resource system invokes
ScreenSaverFreeAttr and attempts to clean up the already freed struct.

Fix this by letting the resource system free the old attrs instead.

CVE-2022-46343, ZDI-CAN 19404

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>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>

This fixes a use-after-free bug:

When a client first calls XvdiSelectVideoNotify() on a drawable with a
TRUE onoff argument, a struct XvVideoNotifyRec is allocated. This struct
is added twice to the resources:
  - as the drawable's XvRTVideoNotifyList. This happens only once per
    drawable, subsequent calls append to this list.
  - as the client's XvRTVideoNotify. This happens for every client.

The struct keeps the ClientPtr around once it has been added for a
client. The idea, presumably, is that if the client disconnects we can remove
all structs from the drawable's list that match the client (by resetting
the ClientPtr to NULL), but if the drawable is destroyed we can remove
and free the whole list.

However, if the same client then calls XvdiSelectVideoNotify() on the
same drawable with a FALSE onoff argument, only the ClientPtr on the
existing struct was set to NULL. The struct itself remained in the
client's resources.

If the drawable is now destroyed, the resource system invokes
XvdiDestroyVideoNotifyList which frees the whole list for this drawable
- including our struct. This function however does not free the resource
for the client since our ClientPtr is NULL.

Later, when the client is destroyed and the resource system invokes
XvdiDestroyVideoNotify, we unconditionally set the ClientPtr to NULL. On
a struct that has been freed previously. This is generally frowned upon.

Fix this by calling FreeResource() on the second call instead of merely
setting the ClientPtr to NULL. This removes the struct from the client
resources (but not from the list), ensuring that it won't be accessed
again when the client quits.

Note that the assignment tpn->client = NULL; is superfluous since the
XvdiDestroyVideoNotify function will do this anyway. But it's left for
clarity and to match a similar invocation in XvdiSelectPortNotify.

CVE-2022-46342, ZDI-CAN 19400

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>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>

The XKB protocol effectively prevents us from ever using keycodes above
255. For buttons it's theoretically possible but realistically too niche
to worry about. For all other passive grabs, the detail must be zero
anyway.

This fixes an OOB write:

ProcXIPassiveUngrabDevice() calls DeletePassiveGrabFromList with a
temporary grab struct which contains tempGrab->detail.exact = stuff->detail.
For matching existing grabs, DeleteDetailFromMask is called with the
stuff->detail value. This function creates a new mask with the one bit
representing stuff->detail cleared.

However, the array size for the new mask is 8 * sizeof(CARD32) bits,
thus any detail above 255 results in an OOB array write.

CVE-2022-46341, ZDI-CAN 19381

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>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>

XTestSwapFakeInput assumes all events in this request are
sizeof(xEvent) and iterates through these in 32-byte increments.
However, a GenericEvent may be of arbitrary length longer than 32 bytes,
so any GenericEvent in this list would result in subsequent events to be
misparsed.

Additional, the swapped event is written into a stack-allocated struct
xEvent (size 32 bytes). For any GenericEvent longer than 32 bytes,
swapping the event may thus smash the stack like an avocado on toast.

Catch this case early and return BadValue for any GenericEvent.
Which is what would happen in unswapped setups anyway since XTest
doesn't support GenericEvent.

CVE-2022-46340, ZDI-CAN 19265

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>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>

Doğukan Korkmaztürk authored 2 years ago and Olivier Fourdan committed 2 years ago

In CommonMakeCurrent() function, the tag of the old context is freed
before the new context is made current. This is problematic because if
the CommonMakeNewCurrent() function fails, the tag of the old context
ends up being removed, even though it is still active. This causes
subsequent glXMakeCurrent() or glXMakeContextCurrent() requests to
generate a GLXBadContextTag error.

This change moves the function call that frees the old tag to a location
where the result of CommonMakeNewCurrent() call is known and it is safe
to free it.

Signed-off-by: Doğukan Korkmaztürk <dkorkmazturk@nvidia.com>

Updated the for-loop that iterates over the received EGLConfigs to
include the very first EGLConfig with index 0.

Signed-off-by: Doğukan Korkmaztürk <dkorkmazturk@nvidia.com>
Fixes: 84692415 - xwayland: Add EGL-backed GLX provider

For now, it sets .version=120, which prevents shader from compiling on ES.
We just force version of shaders to be always 100 on ES, because we use
only 120 shaders on ES anyway, and all shaders works.

Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>

Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Emma Anholt <emma@anholt.net>

Vasily Khoruzhick authored 2 years ago and Konstantin committed 2 years ago

ARB_blend_func_extended may be exposed even without GLSL 1.30.
In order to use it we need GLES2 shaders that are available if
ARB_ES2_compatibility is exposed.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Emma Anholt <emma@anholt.net>

Yuriy Vasilev authored 3 years ago and Konstantin committed 2 years ago

In GLES2, we cannot do GL_RED or GL_RG without GL_EXT_texture_rg.
So, add check for GL_EXT_texture_rg to make it working. Also add
a yuv2 pixman format into render.h to make Xv yuv rendering works.

Signed-off-by: Yuriy Vasilev <uuvasiliev@yandex.ru>

Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Emma Anholt <emma@anholt.net>

glUniformMatrix3fv is used with argument transpose set to GL_TRUE.
According to the Khronos OpenGL ES 2.0 pages transpose must be GL_FALSE.
Actually we can just return transformed matrix from
_glamor_gradient_convert_trans_matrix (@anholt suggest),
so @uvas workaround is not required

Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>

Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Emma Anholt <emma@anholt.net>

For 24 and 32 bit depth pictures xserver uses PICT_x8r8g8b8 and PICT_a8r8g8b8 formats,
which must be backed with GL_BGRA format. It is present in OpenGL ES 2.0 only with
GL_EXT_texture_format_BGRA8888 extension. We require such extension in glamor_init,
so, why not to make use of it?
Fixes #1208
Fixes #1354

Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>

Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Emma Anholt <emma@anholt.net>

Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>

Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Emma Anholt <emma@anholt.net>

Corentin Noël authored 2 years ago and Emma Anholt committed 2 years ago

The virgl driver exposes the name of the host renderer which might be llvmpipe.
In this case we still need glamor to be initialized.

Only check if the renderer starts with llvmpipe (which is what llvmpipe exposes).

Signed-off-by: Corentin Noël <corentin.noel@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>

The only reason we still depend on xorg/font/utils is because we pull a
pkgconfig variable from that .pc file. Let's drop that dependency by
providing that option ourselves.

And where the option isn't specified and font-utils isn't found, default
to $datadir/fonts/X11, same path it's always been.

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

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>

This will allow side-wide customization.

Fixes: https://github.com/XQuartz/XQuartz/issues/274


Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>

Shashank Sharma authored 2 years ago and Shashank Sharma committed 2 years ago

The current X server infrastructure sets modesetting driver as default driver
to handle PCI-hotplug of a GPU device. This prevents the respective DDX driver
(like AMDGPU DDX driver) to take control of the card.

This patch:
- Adds a few functions and fine-tunes the GPU hotplug infrastructure to allow
  the DDX driver to be loaded, if it is configured in the X config file
  options as "hotplug-driver".
- Scans and updates the PCI device list before adding the new GPU device
  in platform, so that the association of the platform device and PCI device
  is in place (dev->pdev).
- Adds documentation of this new option

An example usage in the config file would look like:

Section "OutputClass"
        Identifier "AMDgpu"
        MatchDriver "amdgpu"
        Driver "amdgpu"
	HotplugDriver "amdgpu"
EndSection

V2:
Fixed typo in commit message (Martin)
Added R-B from Adam.
Added ACK from Alex and Martin.

V3:
Added an output class based approach for finding the DDX driver (Aaron)
Rebase

V4:
Addressed review comment from Aaron:
GPU hot-plug handling driver's name to be read from the DDX config file options.
In this way only the DDX drivers interested in handling GPU hot-plug will be
picked and loaded, for others modesetting driver will be used as usual.

V5:
Addressed review comments from Aaron:
- X config option to be listed in CamelCase.
- Indentation fix at one place.
- Code readability related optimization.

V6:
Addressed review comments from Aaron:
- Squash the doc in the same patch
- Doc formatting changes

Cc: Alex Deucher <alexander.deucher@amd.com>
Suggested-by: Aaron Plattner aplattner@nvidia.com (v3)
Acked-by: Martin Roukala <martin.roukala@mupuf.org(v1)>
Acked-by: Alex Deucher alexander.deucher@amd.com (v1)
Reviewed-by: Adam Jackson <ajax@redhat.com(v1)>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>

pkubaj authored 2 years ago and Alan Coopersmith committed 2 years ago

Current error:
ld: error: undefined symbol: xf86EnableIO
>>> referenced by xf86Configure.c
>>>               libxorg_common.a.p/xf86Configure.c.o:(DoConfigure) in archive hw/xfree86/common/libxorg_common.a
>>> referenced by xf86Events.c
>>>               libxorg_common.a.p/xf86Events.c.o:(xf86VTEnter) in archive hw/xfree86/common/libxorg_common.a
>>> referenced by xf86Init.c
>>>               libxorg_common.a.p/xf86Init.c.o:(InitOutput) in archive hw/xfree86/common/libxorg_common.a
>>> referenced 1 more times

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

Commit 8a5f3ddb ("set tag on our surface") introduced the use of tags
to differentiate our own surfaces, and commit a1d14aa8 ("Clear the
"xwl-window" tag on unrealize") removed the tags before the surfaces are
actually destroyed.

Xwayland would then rely on these tags on the surface to decide whether
to ignore or to process the Wayland event in various places.

However, in doing so, it also checked for the tag on keyboard leave
events.

As a result, if the keyboard leave events is received after the X11
window is unrealized, keyboard_handle_leave() would not queue the
LeaveNotify events for the DIX to proceed, and the key repeat would
kick in and repeat the key event indefinitely.

To avoid the issue, process events regardless of the tag as before
in keyboard_handle_leave().

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Fixes: 8a5f3ddb - "xwayland: set tag on our surface"
Closes: #1395


Tested-by: Renan Guilherme Lebre Ramos <japareaggae@gmail.com>
Tested-by: Stefan Dirsch <sndirsch@suse.de>
Acked-by: Michel Dänzer <mdaenzer@redhat.com>

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

Multiplanar GBM buffers can point to different objects from each plane.
Use the _for_plane API when possible to retrieve the correct prime FD
for each plane.

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

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>

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

Multiplanar GBM buffers can point to different objects from each plane.
Use the _for_plane API when possible to retrieve the correct prime FD
for each plane.

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

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.