Callers may check errno when wl_map_insert_* functions return an
error (since [1]). Make sure it's always set to a meaningful value
when returning an error, otherwise callers might end up checking an
errno coming from a completely different function.
[1]: https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/205
Signed-off-by: Simon Ser <contact@emersion.fr>
Fixes: b19488c715 ("util: Limit size of wl_map")
Since server IDs are basically indistinguishable from really big client
IDs at many points in the source, it's theoretically possible to overflow
a map and either overflow server IDs into the client ID space, or grow
client IDs into the server ID space. This would currently take a massive
amount of RAM, but the definition of massive changes yearly.
Prevent this by placing a ridiculous but arbitrary upper bound on the
number of items we can put in a map: 0xF00000, somewhere over 15 million.
This should satisfy pathological clients without restriction, but stays
well clear of the 0xFF000000 transition point between server and client
IDs. It will still take an improbable amount of RAM to hit this, and a
client could still exhaust all RAM in this way, but our goal is to prevent
overflow and undefined behaviour.
Fixes#224
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Not checking the result of wl_array_add() can cause writes past the end of the
allocated buffer if realloc fails.
Signed-off-by: Aleksandr Mezin <mezin.alexander@gmail.com>
for_each_helper tries to calculate a one-past-the-end pointer for its
wl_array input. This is fine when the array has one or more entries, but we
initialize arrays by setting wl_array.data to NULL. Pointer arithmetic is
only defined when both the pointer operand and the result point to the same
allocation, or one-past-the-end of that allocation. As NULL points to no
allocation, no pointer arithmetic can be performed on it, not even adding 0,
even if the result is never dereferenced.
This is caught by clang's ubsan from version 10.
Many tests already hit this case, but I added an explicit test for iterating
over an empty wl_map.
Signed-off-by: Fergus Dall <sidereal@google.com>
The problem was found running Weston, with both Weston and Wayland built
with ASan:
../../git/wayland/src/wayland-util.c:150:2: runtime error: null pointer passed as argument 1, which is declared to never be null
../../git/wayland/src/wayland-util.c:150:2: runtime error: null pointer passed as argument 2, which is declared to never be null
This turns out to be caused by copying an empty array into an empty
array.
That seems to be completely valid thing to do, and wl_array_init()
initializes the pointers to NULL and size to zero. Copying initialized
arrays must always be valid.
The error are caused by calling memcpy() with NULL pointers. It doesn't
explode, because also the size is zero.
Fix the problem by calling memcpy() only if size is not zero. This
should keep things like copying an empty array into a non-empty array
work.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The pointer operand to the binary `+` operator must be to a complete
object type. Since we are working with byte sizes, use `char *` for
arithmetic instead.
Signed-off-by: Michael Forney <mforney@mforney.org>
Since we now have the WL_MAP_ENTRY_ZOMBIE flag to determine whether or
not a client-side object is a zombie, we can remove the faux object.
[daniels: Extracted from Derek's bespoke-zombie patch as an intermediate
step.]
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
On the client side we're going to need to know if an object from the
map is a zombie before we attempt to dereference it, so we need to
pass this to the iterator.
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Used only internally and explicitly marked as such with commit
cf04b0a18f ("Move private definitions and prototypes to new
zwayland-private.h")
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Explicitly set the data member to an invalid memory address during
wl_array_release, such that re-using a freed wl_array without re-initializing
causes a crash. In addition, this pointer assignment makes wl_array_release
testable.
Define a constant for the invalid memory address, and add documentation about
this behavior, starting at libwayland version 1.13.
See https://lists.freedesktop.org/archives/wayland-devel/2016-September/031116.html
Signed-off-by: Yong Bakos <ybakos@humanoriented.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
[Pekka: remove the doc about crashing]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
To complement on the new resource created signal, this allows to
iterate over the existing resources of a client.
Signed-off-by: Giulio Camuffo <giulio.camuffo@kdab.com>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
[Pekka: added empty lines, init ret in for_each_helper()]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Both global_zombie_object and wl_interface_equal are private, yet were
part of public documentation despite not being part of the public API.
Move these two definitions to the top of an existing doxygen \cond block,
which removes them from the public documentation.
Signed-off-by: Yong Bakos <ybakos@humanoriented.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
The public documentation included descriptions of wl_log_stderr_handler,
wl_log_func_t wl_log_handler, wl_log and wl_abort. These are not accessible
via the public API.
Move the doxygen \endcond command to wrap these definitions, removing them
from publication.
Signed-off-by: Yong Bakos <ybakos@humanoriented.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
Move the wl_interface_equal prototype to the top of wayland-private, where
it is not buried in the middle of map, connection and closure functions.
Move the implementation out of connection and into util. This is a utility
function, not specific to connections, and has call sites within connection,
wayland-client and wayland-server.
Signed-off-by: Yong Bakos <ybakos@humanoriented.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
On many places in the code we use wl_log + abort or wl_log + assert(0).
Replace these with one call to wl_abort, so that we don't mix abort(),
assert(0) and we'll save few lines
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
This object is only in wayland-private.h so it's methods should not
be in the documentation.
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
I got a little over-eager with my sanity checks and didn't realize that the
client uses wl_map_insert_at to mark objects as zombies when they come from
the server-side.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
In order to use the second-lowest bit of each pointer in wl_map for the
WL_MAP_ENTRY_LEGACY flag, every pointer has to be a multiple of 4. This
was a good assumption, except with WL_ZOMBIE_OBJECT. This commit creates
an actual static variable to which WL_ZOMBIE_OBJECT now points. Since
things are only every compared to WL_ZOMBIE_OBJECT with "==" or "!=", the
only thing that matters is that it is unique.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
The implementation in this commit allows for one bit worth of flags. If
more flags are desired at a future date, then the wl_map implementation
will have to change but the wl_map API will not.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
The original wl_map implementation did no checking to ensures that ids fell
on the correct side of the WL_SERVER_ID_START line. This meant that a
client could send the server a server ID and it would happily try to use
it. Also, there was no distinction between server-side and client-side in
wl_map_remove. Because wl_map_remove added the entry to the free list
regardless of which side it came from, the following set of actions would
break the map:
1. Client creates a bunch of objects
2. Client deletes one or more of those objects
3. Client does something that causes the server to create an object
Because of the problem in wl_map_remove, the server would take an old
client-side id, apply the WL_SERVER_ID_START offset, and try to use it as a
server-side id regardless of whether or not it was valid.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
If we cannot increase the array for new entries, we now return 0 instead
of accessing invalid memory.
krh: Edited to return 0 on failure instead. In the initialization path,
we call wl_map_insert_new() to insert NULL at index 0, which also returns
0 but not as an error. Since we do that up front, every other case of
returning 0 is an unambiguous error.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
We might have to perform memory allocations in wl_array_copy(), so catch
out-of-memory errors in wl_array_add() and return -1 before changing any
state.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
The core libwayland libraries should not handle logging, only passing
the error messages to subscribed functions.
An application linked to libwayland-server or libwayland-client
will be able to set own functions (one per library) to handle error
messages.
Change in this series: make the wl_log return int, because
of compatibility with printf. It will return the number of bytes logged.
Set the next and prev pointers of the removed list element to NULL. This
will catch programming errors that would use invalid list pointers,
double-remove for instance.
It also helps debugging, making it easy to see in gdb if an object is
not in a list.
Signed-off-by: Pekka Paalanen <ppaalanen@gmail.com>
We set aside a range of the object ID space for use by the server. This
allows the server to bind an object to an ID for a client and pass that
object to the client. The client can use the object immediately and the
server can emit events to the object immdiately.
