[PATCH v2] server: Add API to protect access to an SHM buffer
Kristian Høgsberg
hoegsberg at gmail.com
Wed Nov 13 16:36:06 PST 2013
On Wed, Nov 13, 2013 at 03:32:05PM +0000, Neil Roberts wrote:
> Ok, here is a second version of the patch which leaves the signal
> handler permanently installed and uses thread-local storage to keep
> track of the current pool.
Yeah, that looks good now, both patches pushed.
thanks,
Kristian
> Regards,
> - Neil
>
> ------- >8 --------------- (use git am --scissors to automatically chop here)
>
> 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.
> ---
> src/wayland-server.h | 6 +++
> src/wayland-shm.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 137 insertions(+)
>
> diff --git a/src/wayland-server.h b/src/wayland-server.h
> index 36c9a15..f5427fd 100644
> --- a/src/wayland-server.h
> +++ b/src/wayland-server.h
> @@ -412,6 +412,12 @@ wl_resource_get_destroy_listener(struct wl_resource *resource,
>
> struct wl_shm_buffer;
>
> +void
> +wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
> +
> +void
> +wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
> +
> struct wl_shm_buffer *
> wl_shm_buffer_get(struct wl_resource *resource);
>
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index eff29c3..28f52f4 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -32,10 +32,20 @@
> #include <string.h>
> #include <sys/mman.h>
> #include <unistd.h>
> +#include <assert.h>
> +#include <signal.h>
> +#include <pthread.h>
>
> #include "wayland-private.h"
> #include "wayland-server.h"
>
> +/* This once_t is used to synchronize installing the SIGBUS handler
> + * and creating the TLS key. This will be done in the first call
> + * wl_shm_buffer_begin_access which can happen from any thread */
> +static pthread_once_t wl_shm_sigbus_once = PTHREAD_ONCE_INIT;
> +static pthread_key_t wl_shm_sigbus_data_key;
> +static struct sigaction wl_shm_old_sigbus_action;
> +
> struct wl_shm_pool {
> struct wl_resource *resource;
> int refcount;
> @@ -52,6 +62,12 @@ struct wl_shm_buffer {
> struct wl_shm_pool *pool;
> };
>
> +struct wl_shm_sigbus_data {
> + struct wl_shm_pool *current_pool;
> + int access_count;
> + int fallback_mapping_used;
> +};
> +
> static void
> shm_pool_unref(struct wl_shm_pool *pool)
> {
> @@ -368,3 +384,118 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
> {
> return buffer->height;
> }
> +
> +static void
> +reraise_sigbus(void)
> +{
> + /* If SIGBUS is raised for some other reason than accessing
> + * the pool then we'll uninstall the signal handler so we can
> + * reraise it. This would presumably kill the process */
> + sigaction(SIGBUS, &wl_shm_old_sigbus_action, NULL);
> + raise(SIGBUS);
> +}
> +
> +static void
> +sigbus_handler(int signum, siginfo_t *info, void *context)
> +{
> + struct wl_shm_sigbus_data *sigbus_data =
> + pthread_getspecific(wl_shm_sigbus_data_key);
> + struct wl_shm_pool *pool;
> +
> + if (sigbus_data == NULL) {
> + reraise_sigbus();
> + return;
> + }
> +
> + pool = sigbus_data->current_pool;
> +
> + /* If the offending address is outside the mapped space for
> + * the pool then the error is a real problem so we'll reraise
> + * the signal */
> + if (pool == NULL ||
> + (char *) info->si_addr < pool->data ||
> + (char *) info->si_addr >= pool->data + pool->size) {
> + reraise_sigbus();
> + return;
> + }
> +
> + sigbus_data->fallback_mapping_used = 1;
> +
> + /* This should replace the previous mapping */
> + if (mmap(pool->data, pool->size,
> + PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS,
> + 0, 0) == (void *) -1) {
> + reraise_sigbus();
> + return;
> + }
> +}
> +
> +static void
> +destroy_sigbus_data(void *data)
> +{
> + struct wl_shm_sigbus_data *sigbus_data = data;
> +
> + free(sigbus_data);
> +}
> +
> +static void
> +init_sigbus_data_key(void)
> +{
> + struct sigaction new_action = {
> + .sa_sigaction = sigbus_handler,
> + .sa_flags = SA_SIGINFO | SA_NODEFER
> + };
> +
> + sigemptyset(&new_action.sa_mask);
> +
> + sigaction(SIGBUS, &new_action, &wl_shm_old_sigbus_action);
> +
> + pthread_key_create(&wl_shm_sigbus_data_key, destroy_sigbus_data);
> +}
> +
> +WL_EXPORT void
> +wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer)
> +{
> + struct wl_shm_pool *pool = buffer->pool;
> + struct wl_shm_sigbus_data *sigbus_data;
> +
> + pthread_once(&wl_shm_sigbus_once, init_sigbus_data_key);
> +
> + sigbus_data = pthread_getspecific(wl_shm_sigbus_data_key);
> + if (sigbus_data == NULL) {
> + sigbus_data = malloc(sizeof *sigbus_data);
> + if (sigbus_data == NULL)
> + return;
> +
> + memset(sigbus_data, 0, sizeof *sigbus_data);
> +
> + pthread_setspecific(wl_shm_sigbus_data_key, sigbus_data);
> + }
> +
> + assert(sigbus_data->current_pool == NULL ||
> + sigbus_data->current_pool == pool);
> +
> + sigbus_data->current_pool = pool;
> + sigbus_data->access_count++;
> +}
> +
> +WL_EXPORT void
> +wl_shm_buffer_end_access(struct wl_shm_buffer *buffer)
> +{
> + struct wl_shm_sigbus_data *sigbus_data =
> + pthread_getspecific(wl_shm_sigbus_data_key);
> +
> + assert(sigbus_data->access_count >= 1);
> +
> + if (--sigbus_data->access_count == 0) {
> + if (sigbus_data->fallback_mapping_used) {
> + wl_resource_post_error(buffer->resource,
> + WL_SHM_ERROR_INVALID_FD,
> + "error accessing SHM buffer");
> + sigbus_data->fallback_mapping_used = 0;
> + }
> +
> + sigbus_data->current_pool = NULL;
> + }
> +}
> --
> 1.8.3.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list