Makes it possible to e.g. `call wl_client_get_credentials` with a `const
struct wl_client *` from a global filter callback.
Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
From cleanup commit 0cecde304:
assert()s can be compiled away by #defining NDEBUG. Some build systems
do this. Using wl_abort gives a human readable error message and it
isn't compiled away.
That commit missed one final assert, presumably due to missing it with
grep because of a coding style issue. Fix that up, and remove inclusion
of <assert.h> as appropriate.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
assert()s can be compiled away by #defining NDEBUG. Some build systems
do this. Using wl_abort gives a human readable error message and it
isn't compiled away. This commit closes issue #230.
Signed-off-by: meltq <tejasvipin76@gmail.com>
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>
With wl_shm_buffer_ref_pool(), it's possible for a wl_shm_pool
to outlive its wl_resource. We need to be careful not to access
wl_shm_pool.resource if it's been destroyed.
Reset resource to NULL in the resource destroy handler, and add
NULL checks.
Signed-off-by: Simon Ser <contact@emersion.fr>
Commit 5a981ee8 implemented a fallback path for platforms which do not
support mremap() such as FreeBSD.
To do so, the file descriptor for the mmap() is not closed immediately
but instead kept as long as the pool exists.
That induces more file descriptors kept open for longer, which in turn
may cause problems as wl_shm may be using a lot of file descriptors,
especially with Xwayland which can create a lot of pixmaps on behalf of
its X11 clients.
For platforms where mremap() is available, keeping those file
descriptors opened is a bit of a waste and may cause exhaustion of file
descriptors sooner that before commit 5a981ee8.
Only keep the mmap() file descriptor open on platforms which do not
implement mremap() and close it immediately as before on others.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1283
When allocating memory for structs, use zalloc instead of malloc.
This ensures the memory is zero-initialized, and reduces the risk
of forgetting to initialize all struct fields.
Signed-off-by: Simon Ser <contact@emersion.fr>
In 11623e8f, SIGBUS handlers aren't set if F_SEAL_SHRINK is configured on
the memory. This helps avoid setting up handlers with cooperative clients;
however, if an application gives an incorrect size, the compositor would
access it anyways, figuring SIGBUS is impossible, and crash.
This can be fixed by simply removing the seal-checking logic and always
setting the signal handler. However, it seems that fstat can give the size
of the memfd, so we can check that the size we were told is within the
region. Since it's sealed to shrinking, it must never be shrunk in future,
so we can really (hopefully) ignore SIGBUS.
I was worried that fstat wasn't supported for this, but shm_overview(7) does
mention that it is a possible use.
The best solution would likely be avoiding SIGBUS entirely with
MAP_NOSIGBUS, but that hasn't been merged yet and wouldn't help systems
without it (e.g. with older kernels).
A proof-of-concept of this crash is attached with the merge request. Running
it with this patch gives an invalid-shm error, which is correct.
Signed-off-by: Duncan McIntosh <duncan82013@live.ca>
shm_pool_create_buffer() can raise a false WL_SHM_ERROR_INVALID_STRIDE
error under some circumstances because of integer division.
Fix this by using a strict comparison operator instead of lower or
equal.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Reviewed-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Closes: https://gitlab.freedesktop.org/wayland/wayland/-/issues/235
Some operating systems (e.g. FreeBSD) do not implement mremap.
In that case we can grow the mapping by trying to map adjacent memory.
If that fails we can fall back to creating a new larger mapping and
moving the old memory contents there.
Co-authored-by: Koop Mast <kwm@rainbow-runner.nl>
Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
Catch any API mis-use with an assert. This should abort when the
user calls unreferences the pool more times than it's referenced.
Also change the refcount check to explicitly check for positive
counts. That makes the condition more readable.
Signed-off-by: Simon Ser <contact@emersion.fr>
wl_shm_buffer.pool is never set to NULL. The only time it's set is
in shm_pool_create_buffer, and the pool is guaranteed to be non-NULL
there.
Signed-off-by: Simon Ser <contact@emersion.fr>
While MAP_FAILED is generally defined to that value, we should not be
relying on implementation details of system headers.
Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
Reviewed-by: Simon Ser <contact@emersion.fr>
If a client set the F_SEAL_SHRINK seal on the fd before passing it to
the compositor, the kernel will ensure this fd won’t be able to shrink,
ever. This allows us to avoid setting up the SIGBUS handlers on such
file descriptors.
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
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 Wayland 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 Wayland code
by using directly the string returned by strerror(errno).
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
When an mmap() fails, a WL_SHM_ERROR_INVALID_FD is raised and the client
is killed.
However, there is no indication of the actual system error that caused
mmap() to fail, which makes such error harder to investigate.
Provide the actual error message that caused mmap() to fail.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Add static const modifiers to the shm_pool_interface definition,
making it consistent with the other wl_*_interface definitions and
mundanely safer.
Note that this does not affect the ABI, according to abi-dumper and
abi-compliance-checker[1]; and weston and its shm-backed clients still
run as expected.
[1]: http://lvc.github.io/abi-compliance-checker/
Signed-off-by: Yong Bakos <ybakos@humanoriented.com>
Tested-by: Yong Bakos <ybakos@humanoriented.com>
Acked-by: Bryce Harrington <bryce@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Some headers and source files have been using types such as uint32_t
without explicitly including stdint.h.
Explicitly include stdint.h where appropriate.
Signed-off-by: Yong Bakos <ybakos@humanoriented.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
wayland-shm.c uses WL_EXPORT and wl_array, which are defined in
wayland-util.h.
Include wayland-util.h explicitly, rather than transitively through
wayland-server.h.
Signed-off-by: Yong Bakos <ybakos@humanoriented.com>
Reviewed-by: Sam Spilsbury <smspillaz@gmail.com>
If wl_shm_buffer_get_data() is called on a shm pool that has an external
reference and a pending resize, then the buffer may be outside the pool's
current mapping.
Log a warning if this happens.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
If a compositor is rendering in one thread while dispatching wayland
events in another, a wl_shm_pool_resize() could change the memory
mappings it's rendering from and cause a crash.
Now we defer wl_shm_pool_resize() if the compositor has references on a
pool, and perform the actual resize when it drops those references.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
This is a preliminary step towards deferring shm resize operations until
after the compositor has released all external references on a pool.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
If the client passed a size <= 0 to shm_create_pool, it would
go to err_free, which wouldn't close the fd, and thus leave it opened.
We can also move the size check before the struct wl_shm_pool
malloc, so in case the client passes a wrong size, it won't
do an unnecessary malloc and then free.
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
There's no situation where a shm buffer without a pool makes sense,
so we enforce the pool's existence a little more rigidly.
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
In wayland-server.c we group the deprecated functions and
disable doxygen for them. Do that here too.
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
From irc:
<pq> it creates a wl_buffer object in a way that no client can ever
access the storage.
So, let's replace it with return NULL; and mark it with attribute
deprecated in the header.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
Sometimes the compositor wants to make sure a shm pool doesn't disappear
out from under it.
For example, in Enlightenment, rendering happens in a separate thread
while the main thread can still dispatch events. If a client is destroyed
during rendering, all its resources are cleaned up and its shm pools are
unmapped. This causes the rendering thread to segfault.
This patch adds a way for the compositor to increment the refcount of the
shm pool so it can't disappear, and decrement it when it's finished.
The ref/unref are asymmetrical (ref returns the pool) because it's
possible the buffer itself will be gone when you need to unref the pool.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
It would be possible to make the compositor leak file descriptors by
passing descriptors of open unmmapable files to it, such as /dev/null.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Linux will let you mmap a region of a file that is larger than the
size of the file. If you then try to read from that region the process
will get a SIGBUS signal. Currently the clients can use this to crash
a compositor because it can create a pool and lie about the size of
the file which will cause the compositor to try and read past the end
of it. The compositor can't simply check the size of the file to
verify that it is big enough because then there is a race condition
where the client may truncate the file after the check is performed.
This patch adds the following two public functions in the server API
which can be used wrap access to an SHM buffer:
void wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
void wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
The first time wl_shm_buffer_begin_access is called a signal handler
for SIGBUS will be installed. If the signal is caught then the buffer
for the current pool is remapped to an anonymous private buffer at the
same address which allows the compositor to continue without crashing.
The end_access function will then post an error to the buffer
resource.
The current pool is stored as part of some thread-local storage so
that multiple threads can safely independently access separate
buffers.
Eventually we may want to add some more API so that compositors can
hook into the signal handler or replace it entirely if they also want
to do some SIGBUS handling.
The wl_client_add/new_object() functions sends out an NO_MEMORY error if
the allocation fails. This was convenient in a couple of places where
that was all the error handling that was needed. Unfortunately that
looks like out-of-memory isn't handled at the call site and set a bad
precedent for not cleaning up properly or not handling at all.
As we're introducing wl_resource_create() as a replacement for those two
functions, let's remove the automatic error event posting and require
the caller to do that if necessary.
This commit also introduces a new helper, wl_client_post_no_memory() to
make it possible to send NO_MEMORY events from bind where we don't have
a wl_resource.
A new function, wl_resource_create(), lets the compositor create a
wl_resource for a given version of the interface. Passing 0 for the
object ID will allocate a new ID. The implementation, user data and
destructor can be set with wl_resource_set_implementation().
These two functions deprecates wl_client_add/new_object and the
main difference and motivation is the ability to provide a version number
for the resource. This lets the compositor track which version of the
interface a client has created and we'll use that to verify incoming requests.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
With the work to add wl_resource accessors and port weston to use them,
we're ready to make wl_resource and wl_object opaque structs. We keep
wl_buffer in the header for EGL stacks to use, but don't expose it by
default. In time we'll remove it completely, but for now it provides a
transition paths for code that still uses wl_buffer.
Reviewed-by: Jason Ekstrand<jason@jlekstrand.net>
This commit does not break ABI. It merely changes the types of some things
and adds a wl_shm_buffer_get function.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
This commit adds a wl_resource_init function for initializing wl_resource
structures similar to wl_client_add_object.
From this commit forward, wl_resource structures should not be initialized
manually, but should use wl_resource_init. In the event of a change to the
wl_resource structure, this allows us to protect against regressions by filling
in added fields with reasonable defaults. In this way, while changing
wl_object or wl_resource still constitutes an ABI break, compositors following
this rule will only need to be recompiled in order to properly link against the
new version.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
