The SProcXSendExtensionEvent must not attempt to swap GenericEvent because
it is assuming that the event has fixed size and gives the swapping function
xEvent-sized buffer.
A GenericEvent would be later rejected by ProcXSendExtensionEvent anyway.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The requirement is that events have type in range
EXTENSION_EVENT_BASE..lastEvent, but it was tested
only for first event of all.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The SendEvent request holds xEvent which is exactly 32 bytes long, no more,
no less. Both ProcSendEvent and SProcSendEvent verify that the received data
exactly match the request size. However nothing stops the client from passing
in event with xEvent::type = GenericEvent and any value of
xGenericEvent::length.
In the case of ProcSendEvent, the event will be eventually passed to
WriteEventsToClient which will see that it is Generic event and copy the
arbitrary length from the receive buffer (and possibly past it) and send it to
the other client. This allows clients to copy unitialized heap memory out of X
server or to crash it.
In case of SProcSendEvent, it will attempt to swap the incoming event by
calling a swapping function from the EventSwapVector array. The swapped event
is written to target buffer, which in this case is local xEvent variable. The
xEvent variable is 32 bytes long, but the swapping functions for GenericEvents
expect that the target buffer has size matching the size of the source
GenericEvent. This allows clients to cause stack buffer overflows.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Make sure that the xEvent eventT is initialized with zeros, the same way as
in SProcSendEvent.
Some event swapping functions do not overwrite all 32 bytes of xEvent
structure, for example XSecurityAuthorizationRevoked. Two cooperating
clients, one swapped and the other not, can send
XSecurityAuthorizationRevoked event to each other to retrieve old stack data
from X server. This can be potentialy misused to go around ASLR or
stack-protector.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This currently does a meson build using a docker image I've prepared.
The Dockerfile source is at:
https://github.com/anholt/xserver-travis
Docker proved to be necessary to cut the build time per Travis push.
If some day we end up using meson in more of the X stack, we may be
able to move more dependencies out of the docker image and into the CI
build (putting the I in CI). Until then, we'll have to do docker
image rebuilds when dependencies are added/updated.
To enable Travis CI on your github repository, see the first two steps
of the docs at:
https://docs.travis-ci.com/user/getting-started/
Once you do that, pushing branches to your github repo will trigger
builds, which will send you email if they fail. Current build status
can be veiewed your account on travis-ci.org:
https://travis-ci.org/anholt/xserver
Our top-level glx.h include already provides all of the tokens we use,
and fixes redefinition warnings in the meson build.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
This ensures that we don't use the now-closed file descriptor in the
future.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
oc->trans_conn is set to NULL when the connection is closed. At this
point, oc->fd is no longer valid and shouldn't be used. Move
dereference of oc->fd up into YieldControlNoInput where the state of
oc->trans_conn can be checked in a single place.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
In set_poll_client, check oc->trans_conn to make sure the connection
is still running before changing the ospoll configuration of the file
descriptor in case some other bit of the server is now using this file
descriptor.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
AbortClient performs most of the same operations as
CloseDownFileDescriptor except that it doesn't call ospoll_remove,
leaving that unaware that the file descriptor has been closed.
If the file descriptor is re-used before the server comes back around
to clean up, and that new file descriptor is passed to SetNotifyFd,
then that function will mistakenly re-interpret the stale ClientPtr
returned by ospoll_data as a struct notify * instead and mangle data
badly.
To fix this, the patch does:
1) Change CloseDownFileDescriptor so that it can be called multiple
times on the same OsCommPtr. The calls related to the file
descriptor are moved inside the check for trans_conn and
oc->trans_conn is set to NULL after cleaning up.
2) Move the XdmcpCloseDisplay call into CloseDownFileDescriptor. I
don't think the actually matters as we just need to know at some
point that the session client has exited. Moving it avoids the
possibility of having this accidentally trigger from another client
with the same fd which closes down at around the same time.
3) Change AbortClient to call CloseDownFileDescriptor. This makes sure
that all of the fd-related clean up happens in the same way
everywhere, in particular ensures that ospoll is notified about the
closed file descriptor at the time it is closed and not some time later.
Debian-bug: https://bugs.debian.org/862824
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
This infrastructure is no longer read, only written; the mapping
from fd to client is now handled by ospoll.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
This function creates a temporary pixmap to hold data being moved from
the source to the destination. However, it labeled all of the
variables associated with this as src_, which makes it confusing to
read the code. Rename them tmp_ instead. Also fix the comment
describing the function to note that it copies from CPU to GPU, not
GPU to GPU.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
The previous values happened to work in basic cases, but not in general
if the destination is a subwindow or has a border.
Fixes crash with xli, which moves a large subwindow inside a smaller
parent window for scrolling.
No regressions with xterm, x11perf -copyplane or the xscreensaver
phosphor hack.
Bug: https://bugs.debian.org/857983
Reviewed-by: Keith Packard <keithp@keithp.com>
The client could have said anything here, and if what they said doesn't
actually name an atom NameForAtom() will return NULL, and strcmp() will
be unhappy about that.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The 'tablet_tool_wheel' function for tablet scrolling was added back in
8a1defcc63 but left unimplemented. This commit fills in the necessary
details, using the "clicks" count as the number of discrete scroll up/down
events to send.
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The 'tablet_tool_frame' function treats the button masks as though they
are zero-indexed, but 'tablet_tool_button_state' treats them as one-
indexed. The result is that an e.g. middle click event recieved from
Wayland will be sent from the X server as a right-click instead.
Fixes: 773b04748d ("xwayland: handle button events after motion events")
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Xwayland doesn't override these, so we don't need defining those
in the xwl_screen struct.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Just like we do with XWarpPointer's.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
In the typical pattern in games of "hide cursor, grab with a confineTo,
warp constantly the pointer to the middle of the window" the last warping
step is actually rather optional. Some games may choose to just set up a
grab with confineTo argument, and trust that they'll get correct relative
X/Y axis values despite the hidden cursor hitting the confinement window
edge.
To cater for these cases, lock the pointer whenever there is a pointer
confinement and the cursor is hidden. This ensures the pointer position
is in sync with the compositor's when it's next shown again, and more
importantly resorts to the relative pointer for event delivery.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This fixes grabs on InputOnly windows whose parent is the root window
failing with GrabNotViewable. This is due to window->borderSize/windowSize
being computed as clipped by its parent, resulting in a null region.
Setting up the right size on the root window makes the InputOnly size
correct too, so the GrabNotViewable paths aren't hit anymore.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Of sorts, actually make it confine to the pointer focus, as the
InputOnly window is entirely invisible to xwayland accounting,
we don't have a xwl_window for it.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Of sorts, as we can't honor pointer warping across the whole root window
coordinates, peek the pointer focus in these cases.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If the root window borderClip region is null, the PointInWindowIsVisible()
check fails if pointer warping is attempted on the root window, making
the warping operation bail out early.
Assume coordinates always lay inside the root window for this case,
the actual position will be clamped later within screen coordinates anyway.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Fixes subtle breakage which could sometimes trigger after a server reset
with multiple screens using glamor:
Screen A enters glamor_close_screen last and calls various cleanup
functions, which at some point call glamor_make_current to make sure
screen A's GL context is current. This sets lastGLContext to screen A's
&glamor_priv->ctx. Finally, glamor_close_screen calls
glamor_release_screen_priv, which calls free(glamor_priv).
Later, screen B enters glamor_init, which allocates a new glamor_priv.
With bad luck, this can return the same pointer which was previously
used for screen A's glamor_priv. So when screen B's glamor_init calls
glamor_make_current, lastGLContext == &glamor_priv->ctx, so MakeCurrent
isn't called for screen B's GL context, and the following OpenGL API
calls triggered by glamor_init mess up screen A's GL context.
The observed end result of this was a crash in glamor_get_vbo_space
because glamor_priv->vbo didn't match the GL context, though there might
be other possible outcomes.
Assigning the actual GL context pointer to lastGLContext prevents this
by preventing the false negative test in glamor_make_current.
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
When the default behaviour was changed from clone mode to horizontal
extended layout, a boolean ScrnInfoRec member preferClone was introduced
to choose the old default behaviour. Option "PreferCloneMode" allows
setting this preferClone member.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
All that was left here was updating the FBO's size. However, the FBO
size was always set correctly already through
glamor_set_pixmap_texture() from whoever had attached a new BO to the
pixmap.
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
This means we no longer get "s" for on-screen drawing in glamor_debug,
and there's only "m" (CPU memory) or "f" (Any GPU memory, aka FBOs).
That seems fine to me.
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
This needs a meson with PRs #1784, #1792 and #1794
Future work: remove conditionals which are always on, and simplify redundant
CYGDEBUG conditionals
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
Reviewed-by: Eric Anholt <eric@anholt.net>
Don't unconditionally include rootless.h, and so we don't need to add
rootless to the include path unless building MWEXTWM.
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
Reviewed-by: Eric Anholt <eric@anholt.net>
I misspelled the enable flag, so DRI3 would throw BadImplementation
when you tried to start any GL app. Same as in
c7be7a688a, we also convert it to #ifdef
for consistency.
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
When running an Xwayland server from the command line, we end up
resetting the server every time all of the clients connected to the
server leave. This would be fine, except that xwayland makes the mistake
of unconditionally calling LoadExtensionList(). This causes us to setup
the glxExtension twice in a row which means that when we lose our last
client on the second server generation, we end up trying to call the glx
destructors twice in a row resulting in a segfault:
(EE)
(EE) Backtrace:
(EE) 0: Xwayland (OsSigHandler+0x3b) [0x4982f9]
(EE) 1: /lib64/libpthread.so.0 (__restore_rt+0x0) [0x70845bf]
(EE) 2: /usr/lib64/dri/swrast_dri.so (__driDriverGetExtensions_virtio_gpu+0x32897d) [0x1196e5bd]
(EE) 3: /usr/lib64/dri/swrast_dri.so (__driDriverGetExtensions_virtio_gpu+0x328a45) [0x1196e745]
(EE) 4: /usr/lib64/dri/swrast_dri.so (__driDriverGetExtensions_virtio_gpu+0x32665f) [0x11969f7f]
(EE) 5: Xwayland (__glXDRIscreenDestroy+0x30) [0x54686e]
(EE) 6: Xwayland (glxCloseScreen+0x3f) [0x5473db]
(EE) 7: Xwayland (glxCloseScreen+0x53) [0x5473ef]
(EE) 8: Xwayland (dix_main+0x7b6) [0x44c8c9]
(EE) 9: Xwayland (main+0x28) [0x61c503]
(EE) 10: /lib64/libc.so.6 (__libc_start_main+0xf1) [0x72b1401]
(EE) 11: Xwayland (_start+0x2a) [0x4208fa]
(EE) 12: ? (?+0x2a) [0x2a]
(EE)
(EE) Segmentation fault at address 0x18
(EE)
Fatal server error:
(EE) Caught signal 11 (Segmentation fault). Server aborting
(EE)
Easy reproduction recipe:
- Start an Xwayland session with the default settings
- Open a window
- Close that window
- Open another window
- Close that window
- Total annihilation occurs
Signed-off-by: Lyude <lyude@redhat.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Commit aa6717ce2 switched xf86WaitForInput from using select(2) to using
poll(2). Before this change, the timeout was interpreted as being in
microseconds; afterwards it is fed directly to xorg_poll which interprets
it as being in milliseconds. This results in the function potentially
blocking 1000x longer than intended. This commit scales down the timeout
argument before passing it to xorg_poll, being careful to ensure the result
is not rounded down due to integer division.
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
./hw/xfree86/common/xf86pciBus.c: In function ‘xf86MatchDriverFromFiles’:
../hw/xfree86/common/xf86pciBus.c:1330:52: warning: ‘snprintf’ output may be
truncated before the last format character [-Wformat-truncation=]
snprintf(path_name, sizeof(path_name), "%s/%s", ^~~~~~~
../hw/xfree86/common/xf86pciBus.c:1330:13: note: ‘snprintf’ output between 2
dirent->d_name is 256, so sprintf("%s/%s") into a 256 buffer gives us:
and 257 bytes into a destination of size 256
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Unused since Keith ported misprite to damage in '04.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
The function hasn't been doing anything useful since keithp's resource
freeing fixes in 2014.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
GLES2 support has been requested multiple times, and we've had this
code laying around trying to implement it. The GLES2 implementation
is not quite there yet (some pixel transfer failures), but it
shouldn't take much fixing at this point.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Those extensions don't exist. There's only surfaceless_context.
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Indirect GLX uses its own context and doesn't care what glamor is
using.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Using flink is banned on render nodes, and they needlessly expose our
screen pixmap contents to any authenticated client. This also
incidentally drops the dependency on EGL_MESA_drm_image.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
I couldn't find it being used anywhere in the history of the code.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
This has been associated with dri3 for now, but we need to use it
elsewhere in order to avoid flink. The extensions have been
implemented for long enough that I couldn't find when it was that we
turned them on. Oddly, we already required renderbuffer import
support, which is basically as hard to implement as texture import.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Daniel Stone <daniels@collabora.com>
