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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel