the code is something like:
if (object == NULL && ...) {
object = NULL;
return;
}
first, the object is already NULL, second, the assignment has no effect
since we return from the function right away
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
When we are adding padding bytes making our wl_buffer buffer content 4
byte aligned, we are just moving the pointer. Since the buffer is
allocated using plain malloc(), this means our padding bytes are
effectively uninitialized data, which could be anything previously
allocated in the server process. As we'll be sharing this buffer
content with arbitrary clients, we are effectively sharing private
memory with every client, and even though a well behaving client will
discard any such memory, a malicious client may not.
Therefor, to avoid any potential missuse of the uninitialized padding
memory shared between the server and client, initialize the buffer
content to 0, making the padding bytes always 0.
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
This adds an API to get the file descriptor for a client.
The client file descriptor can be used for a wayland compositor to validate
a request from a client if there are any additional information provided from
the client's file descriptor.
For instance, this will be helpful in some linux distributions, in which SELinux
or SMACK is enabled. In those environments, each file (including socket) will have
each security contexts in its inode as xattr member variable. A wayland compositor
can validate a client request by getting the file descriptor of the client and
by checking the security contexts associated with the file descriptor.
Signed-off-by: Sung-Jin Park <input.hacker@gmail.com>
When processing a closure, data in the connection can be consumed again
if the closure itself invokes extra event dispatch. In that case the
remaining data size is also altered, so the variable len should be
updated after the closure is processed.
Signed-off-by: Jaeyoon Jung <jaeyoon.jung@lge.com>
Reviewed-by: Jonas Ådahl <jadahl@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>
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>
It expects ((msg_controllen == 0) == (msg_control == NULL)), and returns
EINVAL otherwise. It can't hurt to be tidy about things on other platforms
either though.
See: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=99356#c5
Signed-off-by: Philip Withnall <philip at tecnocode.co.uk>
Signed-off-by: Karsten Otto <ottoka at posteo.de>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Calling close() on the same file descriptor that a previous call to
close() already closed is wrong, and racy if another thread received
that same file descriptor as a eg. new socket or actual file.
There are two situations where wl_connection_destroy() would close its
file descriptor and then another function up in the call chain would
close the same file descriptor:
* When wl_client_create() fails after calling wl_connection_create(),
it will call wl_connection_destroy() before returning. However, its
caller will always close the file descriptor if wl_client_create()
fails.
* wl_display_disconnect() unconditionally closes the display file
descriptor and also calls wl_connection_destroy().
So these two seem to expect wl_connection_destroy() to leave the file
descriptor open. The other caller of wl_connection_destroy(),
wl_client_destroy(), does however expect wl_connection_destroy() to
close its file descriptor, alas.
This patch changes wl_connection_destroy() to indulge this majority of
two callers by simply not closing the file descriptor. For the benefit
of wl_client_destroy(), wl_connection_destroy() then returns the
unclosed file descriptor so that wl_client_destroy() can close it
itself.
Since wl_connection_destroy() is a private function called from few
places, changing its semantics seemed like the more expedient way to
address the double-close() problem than shuffling around the logic in
wl_client_create() to somehow enable it to always avoid calling
wl_connection_destroy().
Signed-off-by: Benjamin Herr <ben@0x539.de>
Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
If a message was too big to fit in the connection buffer, the code
in wl_buffer_put would just write past the end of it.
I haven't seen any real world use case that would trigger this bug, but
it was possible to trigger it by sending a long enough string to the
wl_data_source.offer request.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69267
In some cases, like Xwayland, stdout and stderr are redirected to
/dev/null, losing us valuable information, while wl_log can be
overridden, allowing us to send it to a log file instead. This
can help debugging immensely.
Otherwise the tail of fds_in buffer would just shift beyond the beginning.
That confuses the actual request handler and results in a crash further on
due to corrupted tail.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
This commit adds support for server-side languages bindings. This is done
in two ways:
1. Adding a wl_resource_set_dispatcher function that corresponds to
wl_resource_set_interface. The only difference between the two functions
is that the new version takes a dispatcher along with the implementation,
data, and destructor. This allows for runtime calling of native language
functions for callbacks instead of having to generate function pointers.
2. Adding versions of wl_resource_post_event and wl_resource_queue_event
that take an array of wl_argument instead of a variable argument list.
This allows for easier run-time argument conversion and removes the need
for libffi-based calling of variadic functions.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Functions like wl_argument_from_va_list expect from get_next_argument,
to initialize details->type but when the signature is empty or contains
only version (like in desktop-shell-protocol.c in weston) it is left
uninitialized.
This patch fixes it, by initializing details->type with '\0' value,
signaling end of arguments.
Signed-off-by: Mariusz Ceier <mceier+wayland@gmail.com>
This commit adds version information to wl_message signatures and a
wl_message_get_since function to retrieve. The since version comes in the
form of a (possible) integer at the begining of the message. If the
message starts with an integer, then it specifies the "since" version of
that message. Messages present in version one do not get this "since"
information. In this way we can run-time detect the version information
for a structure on a per-message basis.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
The current thread model assumes that the application or toolkit will have
one thread that either polls the display fd and dispatches events or just
dispatches in a loop. Only this main thread will read from the fd while
all other threads will block on a pthread condition and expect the main
thread to deliver events to them.
This turns out to be too restrictive. We can't assume that there
always will be a thread like that. Qt QML threaded rendering will
block the main thread on a condition that's signaled by a rendering
thread after it finishes rendering. This leads to a deadlock when the
rendering threads blocks in eglSwapBuffers(), and the main thread is
waiting on the condition. Another problematic use case is with games
that has a rendering thread for a splash screen while the main thread
is busy loading game data or compiling shaders. The main thread isn't
responsive and ends up blocking eglSwapBuffers() in the rendering thread.
We also can't assume that there will be only one thread polling on the
file descriptor. A valid use case is a thread receiving data from a
custom wayland interface as well as a device fd or network socket.
The thread may want to wait on either events from the wayland
interface or data from the fd, in which case it needs to poll on both
the wayland display fd and the device/network fd.
The solution seems pretty straightforward: just let all threads read
from the fd. However, the main-thread restriction was introduced to
avoid a race. Simplified, main loops will do something like this:
wl_display_dispatch_pending(display);
/* Race here if other thread reads from fd and places events
* in main eent queue. We go to sleep in poll while sitting on
* events that may stall the application if not dispatched. */
poll(fds, nfds, -1);
/* Race here if other thread reads and doesn't queue any
* events for main queue. wl_display_dispatch() below will block
* trying to read from the fd, while other fds in the mainloop
* are ignored. */
wl_display_dispatch(display);
The restriction that only the main thread can read from the fd avoids
these races, but has the problems described above.
This patch introduces new API to solve both problems. We add
int wl_display_prepare_read(struct wl_display *display);
and
int wl_display_read_events(struct wl_display *display);
wl_display_prepare_read() registers the calling thread as a potential
reader of events. Once data is available on the fd, all reader
threads must call wl_display_read_events(), at which point one of the
threads will read from the fd and distribute the events to event
queues. When that is done, all threads return from
wl_display_read_events().
From the point of view of a single thread, this ensures that between
calling wl_display_prepare_read() and wl_display_read_events(), no
other thread will read from the fd and queue events in its event
queue. This avoids the race conditions described above, and we avoid
relying on any one thread to be available to read events.
This commit adds a flags parameter to wl_closure_invoke(). The so far
added flags are ment to specify if the invokation is client side or
server side. When on the server side, closure arguments of type 'new_id'
should be invoked as a integer id while on the client side they should
be invoked as a pointer to a proxy object.
This fixes a bug happening when the address of a client side 'new_id'
proxy object did not fit in a 32 bit integer.
krh: Squashed test suite compile fix from Jason Ekstrand.
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
The primary purpose of this patch is to clean up wl_closure and separate
closure storage, libffi, and the wire format. To that end, a number of changes
have been made:
- The maximum number of closure arguments has been changed from a magic number
to a #define WL_CLOSURE_MAX_ARGS
- A wl_argument union has been added for storing a generalized closure
argument and wl_closure has been converted to use wl_argument instead of the
combination of libffi, the wire format, and a dummy extra buffer. As of
now, the "extra" field in wl_closure should be treated as bulk storage and
never direclty referenced outside of wl_connection_demarshal.
- Everything having to do with libffi has been moved into wl_closure_invoke
and the convert_arguments_to_ffi helper function.
- Everything having to do with the wire format has been restricted to
wl_connection_demarshal and the new static serialize_closure function. The
wl_closure_send and wl_closure_queue functions are now light wrappers around
serialize_closure.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Most extra data are just pointers, but in case of fds we store an int in
the extra space. That can cause un-aligned access to pointers on 64 bit
architectures. Make sure we always align pointer storage correctly.
wl_connection_read() assumes that the caller dispatched all messages
before calling it. wl_buffer_put_iov() does only provide enough room so we
fill the buffer. So the only case when the buffer overflows, is when a
previous read filled up the buffer but we couldn't parse a single message
from it. In this case, the client sent a message bigger than our buffer
and we should return an error and close the connection.
krh: Edited from Davids original patch to just check that the buffer
isn't full before we try reading into it.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
We rely on well-defined unsigned overflow behaviour so let's make the
index fields actually unsigned. Signed ints aren't guaranteed to have the
behavior we want (could be either ones or twos complement).
If we read more FDs than we have room for, we currently leak FDs because
we overwrite previous still pending FDs. Instead, we do now close incoming
FDs if the buffer is full and return EOVERFLOW.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Same problem as we had with close_fds(). We cannot rely on the fds_out
buffer being filled with less than MAX_FDS_OUT file descriptors.
Therefore, write at most MAX_FDS_OUT file-descriptors to the outgoing
buffer.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Same problem as with outgoing FDs. We need to close these on shutdown,
otherwise we leak open file descriptors.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
If we push two messages via wl_connection_write() and both messages
contain more than MAX_FDS_OUT file-descriptors combined, then
wl_connection_flush() will write only MAX_FDS_OUT of them, but close all
pending ones, too.
Furthermore, close_fds() will copy more FDs out of the buffer than it can
hold and cause a buffer overflow. Therefore, we simply pass a maximum
limit to close_fds().
During shutdown, we simply close all available FDs.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
When destroying a wl_connection object, there might still be data in the
queue. We would leak open file-descriptors so we must close them.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
We can't use the same behaviour in both the client and the server. In the
client this is a wl_proxy pointer in the server it's a pointer to the
uint32_t object id. This doesn't fix the problem, but it's a slightly
more useful default, since we typically use WAYLAND_DEBUG on the client.
In most cases the pointer equality test is sufficient. However, in
some cases, depending on how things are split across shared objects,
we can end up with multiple instances of the interface metadata
constants. So if the pointers match, the interfaces are equal, if
they don't match we have to compare the interface names.
On the client side where we queue up multiple events before dispatching, we
need to look up the receiving proxy and argument proxies immediately before
calling the handler. Between queueing up multiple events and eventually
invoking the handler, previous handlers may have destroyed some of the
proxies.
The update callback for the file descriptors was always a bit awkward and
un-intuitive. The idea was that whenever the protocol code needed to
write data to the fd it would call the 'update' function. This function
would adjust the mainloop so that it polls for POLLOUT on the fd so we
can eventually flush the data to the socket.
The problem is that in multi-threaded applications, any thread can issue
a request, which writes data to the output buffer and thus triggers the
update callback. Thus, we'll be calling out with the display mutex
held and may call from any thread.
The solution is to eliminate the udpate callback and just require that
the application or server flushes all connection buffers before blocking.
This turns out to be a simpler API, although we now require clients to
deal with EAGAIN and non-blocking writes. It also saves a few syscalls,
since the socket will be writable most of the time and most writes will
complete, so we avoid changing epoll to poll for POLLOUT, then write and
then change it back for each write.
Expose these to other files using wayland-private.h, so wayland-client.c
can walk NULLables properly.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
If a new object id arrives ensure that there is an empty array entry
created, otherwise we might get out of sync for new ids if object isn't
created by interface implementation.
Most of the time it does not make sense to pass a NULL object, string, or array
to a protocol request. This commit adds an explicit “allow-null” attribute
to mark the request arguments where NULL makes sense.
Passing a NULL object, string, or array to a protocol request which is not
marked as allow-null is now an error. An implementation will never receive
a NULL value for these arguments from a client.
Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
When the server send a new object ID, the client used to have to allocate
the proxy manually and without type-safety. We now allocate the proxy
in a client-side post-processing step on the incoming closure.
