[PATCH wayland 1/4] Add a "side" field and some sanity checks to wl_map.
Kristian Høgsberg
hoegsberg at gmail.com
Tue Jun 4 21:50:23 PDT 2013
On Sat, Jun 01, 2013 at 05:40:52PM -0500, Jason Ekstrand wrote:
> 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.
No, the free list usage is fine. It's always only used for locally
allocated IDs (ie, client allocated IDs in wayland-client or server
allocated IDs in wayland-server). Only wl_map_insert_new() allocates
out of the free-list and only wl_map_remove() puts entries into the
free-list.
Aside from the the patch is fine though. The wl_map API is cleaner
this way and it's good that we now catch those invalid combinations.
Kristian
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
> src/wayland-client.c | 10 ++++------
> src/wayland-private.h | 5 +++--
> src/wayland-server.c | 7 +++----
> src/wayland-util.c | 25 ++++++++++++++++++++++---
> 4 files changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 7847370..0f5e093 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -226,8 +226,7 @@ wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
> proxy->refcount = 1;
>
> pthread_mutex_lock(&display->mutex);
> - proxy->object.id = wl_map_insert_new(&display->objects,
> - WL_MAP_CLIENT_SIDE, proxy);
> + proxy->object.id = wl_map_insert_new(&display->objects, proxy);
> pthread_mutex_unlock(&display->mutex);
>
> return proxy;
> @@ -518,17 +517,16 @@ wl_display_connect_to_fd(int fd)
> memset(display, 0, sizeof *display);
>
> display->fd = fd;
> - wl_map_init(&display->objects);
> + wl_map_init(&display->objects, WL_MAP_CLIENT_SIDE);
> wl_event_queue_init(&display->queue, display);
> wl_list_init(&display->event_queue_list);
> pthread_mutex_init(&display->mutex, NULL);
>
> - wl_map_insert_new(&display->objects, WL_MAP_CLIENT_SIDE, NULL);
> + wl_map_insert_new(&display->objects, NULL);
>
> display->proxy.object.interface = &wl_display_interface;
> display->proxy.object.id =
> - wl_map_insert_new(&display->objects,
> - WL_MAP_CLIENT_SIDE, display);
> + wl_map_insert_new(&display->objects, display);
> display->proxy.display = display;
> display->proxy.object.implementation = (void(**)(void)) &display_listener;
> display->proxy.user_data = display;
> diff --git a/src/wayland-private.h b/src/wayland-private.h
> index c4ce6b0..270b470 100644
> --- a/src/wayland-private.h
> +++ b/src/wayland-private.h
> @@ -44,14 +44,15 @@
> struct wl_map {
> struct wl_array client_entries;
> struct wl_array server_entries;
> + uint32_t side;
> uint32_t free_list;
> };
>
> typedef void (*wl_iterator_func_t)(void *element, void *data);
>
> -void wl_map_init(struct wl_map *map);
> +void wl_map_init(struct wl_map *map, uint32_t side);
> void wl_map_release(struct wl_map *map);
> -uint32_t wl_map_insert_new(struct wl_map *map, uint32_t side, void *data);
> +uint32_t wl_map_insert_new(struct wl_map *map, void *data);
> int wl_map_insert_at(struct wl_map *map, uint32_t i, void *data);
> int wl_map_reserve_new(struct wl_map *map, uint32_t i);
> void wl_map_remove(struct wl_map *map, uint32_t i);
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index a3d3887..c96be56 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -336,7 +336,7 @@ wl_client_create(struct wl_display *display, int fd)
> if (client->connection == NULL)
> goto err_source;
>
> - wl_map_init(&client->objects);
> + wl_map_init(&client->objects, WL_MAP_SERVER_SIDE);
>
> if (wl_map_insert_at(&client->objects, 0, NULL) < 0)
> goto err_map;
> @@ -379,8 +379,7 @@ wl_client_add_resource(struct wl_client *client,
> {
> if (resource->object.id == 0) {
> resource->object.id =
> - wl_map_insert_new(&client->objects,
> - WL_MAP_SERVER_SIDE, resource);
> + wl_map_insert_new(&client->objects, resource);
> } else if (wl_map_insert_at(&client->objects,
> resource->object.id, resource) < 0) {
> wl_resource_post_error(client->display_resource,
> @@ -932,7 +931,7 @@ wl_client_new_object(struct wl_client *client,
> {
> uint32_t id;
>
> - id = wl_map_insert_new(&client->objects, WL_MAP_SERVER_SIDE, NULL);
> + id = wl_map_insert_new(&client->objects, NULL);
> return wl_client_add_object(client,
> interface, implementation, id, data);
>
> diff --git a/src/wayland-util.c b/src/wayland-util.c
> index 598ab42..57f496b 100644
> --- a/src/wayland-util.c
> +++ b/src/wayland-util.c
> @@ -152,9 +152,10 @@ union map_entry {
> };
>
> WL_EXPORT void
> -wl_map_init(struct wl_map *map)
> +wl_map_init(struct wl_map *map, uint32_t side)
> {
> memset(map, 0, sizeof *map);
> + map->side = side;
> }
>
> WL_EXPORT void
> @@ -165,13 +166,13 @@ wl_map_release(struct wl_map *map)
> }
>
> WL_EXPORT uint32_t
> -wl_map_insert_new(struct wl_map *map, uint32_t side, void *data)
> +wl_map_insert_new(struct wl_map *map, void *data)
> {
> union map_entry *start, *entry;
> struct wl_array *entries;
> uint32_t base;
>
> - if (side == WL_MAP_CLIENT_SIDE) {
> + if (map->side == WL_MAP_CLIENT_SIDE) {
> entries = &map->client_entries;
> base = 0;
> } else {
> @@ -203,8 +204,14 @@ wl_map_insert_at(struct wl_map *map, uint32_t i, void *data)
> struct wl_array *entries;
>
> if (i < WL_SERVER_ID_START) {
> + if (map->side == WL_MAP_CLIENT_SIDE)
> + return -1;
> +
> entries = &map->client_entries;
> } else {
> + if (map->side == WL_MAP_SERVER_SIDE)
> + return -1;
> +
> entries = &map->server_entries;
> i -= WL_SERVER_ID_START;
> }
> @@ -230,8 +237,14 @@ wl_map_reserve_new(struct wl_map *map, uint32_t i)
> struct wl_array *entries;
>
> if (i < WL_SERVER_ID_START) {
> + if (map->side == WL_MAP_CLIENT_SIDE)
> + return -1;
> +
> entries = &map->client_entries;
> } else {
> + if (map->side == WL_MAP_SERVER_SIDE)
> + return -1;
> +
> entries = &map->server_entries;
> i -= WL_SERVER_ID_START;
> }
> @@ -262,8 +275,14 @@ wl_map_remove(struct wl_map *map, uint32_t i)
> struct wl_array *entries;
>
> if (i < WL_SERVER_ID_START) {
> + if (map->side == WL_MAP_SERVER_SIDE)
> + return;
> +
> entries = &map->client_entries;
> } else {
> + if (map->side == WL_MAP_CLIENT_SIDE)
> + return;
> +
> entries = &map->server_entries;
> i -= WL_SERVER_ID_START;
> }
> --
> 1.8.1.4
>
> _______________________________________________
> 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