[PATCH wayland] client: Don't segfault when receiving error on destroyed object

[Marek Chalupa]

Hi,

can confirm the segfault, tested it (will send the test I used for it as 
a follow-up). The only API change problem could be in returning
NULL as the interface - if the user does not check for it, he/she 
dereferences NULL. But I don't think anybody (except us in tests) is
using wl_display_get_protocol_error() for the error analysis, so IMO its 
OK break.

Reviewed-by: Marek Chalupa <mchqwerty at gmail.com>

Cheers,
Marek

On 02/22/16 06:37, Jonas Ådahl wrote:
> If an error is received on a destroyed object, we'd get NULL passed
> to display_handle_error() instead of a pointer to a valid wl_proxy.
>
> The logging is changed to report [unknown interface] and [unknown id]
> instead of the actual interface name and id.
>
> The wl_display_get_protocol_error() documentation is updated to handle
> the situation. For when the proxy was NULL, the object id 0 and
> interface NULL is written.
>
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>
> This is technically an API change, but I see no less breaking change.
> Considering that clients would segfault before ever reaching here without this
> patch, maybe it's an Ok break.
>
>
> Jonas
>
>
>   src/wayland-client.c | 32 ++++++++++++++++++++++++--------
>   1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index ef12bfc..87fc0e4 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -177,7 +177,7 @@ display_protocol_error(struct wl_display *display, uint32_t code,
>   		return;
>
>   	/* set correct errno */
> -	if (wl_interface_equal(intf, &wl_display_interface)) {
> +	if (intf && wl_interface_equal(intf, &wl_display_interface)) {
>   		switch (code) {
>   		case WL_DISPLAY_ERROR_INVALID_OBJECT:
>   		case WL_DISPLAY_ERROR_INVALID_METHOD:
> @@ -790,12 +790,26 @@ display_handle_error(void *data,
>   		     uint32_t code, const char *message)
>   {
>   	struct wl_proxy *proxy = object;
> +	uint32_t object_id;
> +	const struct wl_interface *interface;
>
> -	wl_log("%s@%u: error %d: %s\n",
> -	       proxy->object.interface->name, proxy->object.id, code, message);
> +	if (proxy) {
> +		wl_log("%s@%u: error %d: %s\n",
> +		       proxy->object.interface->name,
> +		       proxy->object.id,
> +		       code, message);
>
> -	display_protocol_error(display, code, proxy->object.id,
> -			       proxy->object.interface);
> +		object_id = proxy->object.id;
> +		interface = proxy->object.interface;
> +	} else {
> +		wl_log("[unknown interface]@[unknown id]: error %d: %s\n",
> +		       code, message);
> +
> +		object_id = 0;
> +		interface = NULL;
> +	}
> +
> +	display_protocol_error(display, code, object_id, interface);
>   }
>
>   static void
> @@ -1756,10 +1770,12 @@ wl_display_get_error(struct wl_display *display)
>   /** Retrieves the information about a protocol error:
>    *
>    * \param display    The Wayland display
> - * \param interface  if not NULL, stores the interface where the error occurred
> + * \param interface  if not NULL, stores the interface where the error occurred,
> + *                   or NULL, if unknown.
>    * \param id         if not NULL, stores the object id that generated
> - *                   the error. There's no guarantee the object is
> - *                   still valid; the client must know if it deleted the object.
> + *                   the error, or 0, if the object id is unknown. There's no
> + *                   guarantee the object is still valid; the client must know
> + *                   if it deleted the object.
>    * \return           The error code as defined in the interface specification.
>    *
>    * \code
>