[PATCH] wayland-server: return new ID in wl_client_add_resource()

[Kristian Høgsberg]

On Mon, Sep 10, 2012 at 11:23:39AM +0200, David Herrmann wrote:
> Sorry, misspelled the ML address. Forwarding it to wayland-devel:
> 
> ---------- Forwarded message ----------
> 
> wl_client_add_resource() used to return no error even though the new
> resource wasn't added to the client. This currently makes it very easy to
> DOS weston by simply posting thousands of "create_surface" requests with
> an invalid ID. Weston simply assumes the wl_client_add_resource() request
> succeeds but will never destroy the surface again as the "destroy" signal
> is never called (because the surface isn't linked into the wl_map).
> 
> This change makes wl_client_add_resource() return the new ID of the added
> object and 0 on failure. Servers (like weston) can now correctly
> immediately destroy the surface when this call fails instead of leaving
> the surface around and producing memory-leaks.
> 
> Instead of returning -1 on failure and 0 on success, I made it return the
> new ID as this seems more appropriate. We can directly use it when calling
> it with new_id==0.

That makes sense, applied.

Kristian

> Signed-off-by: David Herrmann <dh.herrmann at googlemail.com>
> ---
>  src/wayland-server.c | 12 ++++++++----
>  src/wayland-server.h |  2 +-
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 88e8433..416cec1 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -387,23 +387,27 @@ wl_client_get_credentials(struct wl_client *client,
>                 *gid = client->ucred.gid;
>  }
> 
> -WL_EXPORT void
> +WL_EXPORT uint32_t
>  wl_client_add_resource(struct wl_client *client,
>                        struct wl_resource *resource)
>  {
> -       if (resource->object.id == 0)
> +       if (resource->object.id == 0) {
>                 resource->object.id =
>                         wl_map_insert_new(&client->objects,
>                                           WL_MAP_SERVER_SIDE, resource);
> -       else if (wl_map_insert_at(&client->objects,
> -                                 resource->object.id, resource) < 0)
> +       } else if (wl_map_insert_at(&client->objects,
> +                                 resource->object.id, resource) < 0) {
>                 wl_resource_post_error(client->display_resource,
>                                        WL_DISPLAY_ERROR_INVALID_OBJECT,
>                                        "invalid new id %d",
>                                        resource->object.id);
> +               return 0;
> +       }
> 
>         resource->client = client;
>         wl_signal_init(&resource->destroy_signal);
> +
> +       return resource->object.id;
>  }
> 
>  WL_EXPORT struct wl_resource *
> diff --git a/src/wayland-server.h b/src/wayland-server.h
> index 83b6635..f73fa88 100644
> --- a/src/wayland-server.h
> +++ b/src/wayland-server.h
> @@ -355,7 +355,7 @@ void wl_resource_post_no_memory(struct wl_resource
> *resource);
> 
>  #include "wayland-server-protocol.h"
> 
> -void
> +uint32_t
>  wl_client_add_resource(struct wl_client *client,
>                        struct wl_resource *resource);
> 
> --
> 1.7.12