[PATCH weston v3 14/17] compositor: check viewport source rect validity

[Bryce Harrington]

On Tue, Apr 26, 2016 at 03:51:06PM +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> wp_viewporter spec requires protocol errors when the viewport is outside
> the buffer area.
> 
> The viewport is checked in wl_surface.commit handler as the error needs
> to be delivered as a reply to the commit, not at state apply time later
> (e.g. with synchronized sub-surfaces, or at render time).
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>  src/compositor.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 1d605c7..4dfa7b7 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2804,6 +2804,71 @@ weston_surface_build_buffer_matrix(const struct weston_surface *surface,
>  	weston_matrix_scale(matrix, vp->buffer.scale, vp->buffer.scale, 1);
>  }
>  
> +/**
> + * Compute a + b > c while being safe to overflows.
> + */
> +static bool
> +fixed_sum_gt(wl_fixed_t a, wl_fixed_t b, wl_fixed_t c)
> +{
> +	return (int64_t)a + (int64_t)b > (int64_t)c;
> +}
> +
> +static bool
> +weston_surface_is_pending_viewport_source_valid(
> +	const struct weston_surface *surface)
> +{
> +	const struct weston_surface_state *pend = &surface->pending;
> +	const struct weston_buffer_viewport *vp = &pend->buffer_viewport;
> +	int width_from_buffer = 0;
> +	int height_from_buffer = 0;
> +	wl_fixed_t w;
> +	wl_fixed_t h;
> +
> +	/* If viewport source rect is not set, always ok. */

nitpick: semicolon rather than comma

> +	if (vp->buffer.src_width == wl_fixed_from_int(-1))
> +		return true;
> +
> +	if (pend->newly_attached) {
> +		if (pend->buffer) {
> +			convert_size_by_transform_scale(&width_from_buffer,
> +							&height_from_buffer,
> +							pend->buffer->width,
> +							pend->buffer->height,
> +							vp->buffer.transform,
> +							vp->buffer.scale);
> +		} else {
> +			/* No buffer, viewport is irrelevant. */

nitpick: semicolon rather than comma

> +			return true;
> +		}
> +	} else {
> +		width_from_buffer = surface->width_from_buffer;
> +		height_from_buffer = surface->height_from_buffer;
> +	}
> +
> +	assert((width_from_buffer == 0) == (height_from_buffer == 0));
> +	assert(width_from_buffer >= 0 && height_from_buffer >= 0);

Clever logic!  But maybe too clever to use in asserts?

> +	/* No buffer, viewport is irrelevant. */

nitpick: semicolon rather than comma

> +	if (width_from_buffer == 0 || height_from_buffer == 0)
> +		return true;
> +
> +	/* overflow checks for wl_fixed_from_int() */
> +	if (width_from_buffer > wl_fixed_to_int(INT32_MAX))
> +		return false;
> +	if (height_from_buffer > wl_fixed_to_int(INT32_MAX))
> +		return false;
> +
> +	w = wl_fixed_from_int(width_from_buffer);
> +	h = wl_fixed_from_int(height_from_buffer);
> +
> +	if (fixed_sum_gt(vp->buffer.src_x, vp->buffer.src_width, w))
> +		return false;
> +	if (fixed_sum_gt(vp->buffer.src_y, vp->buffer.src_height, h))
> +		return false;
> +
> +	return true;
> +}
> +
>  /* Translate pending damage in buffer co-ordinates to surface
>   * co-ordinates and union it with a pixman_region32_t.
>   * This should only be called after the buffer is attached.
> @@ -2943,6 +3008,16 @@ surface_commit(struct wl_client *client, struct wl_resource *resource)
>  	struct weston_surface *surface = wl_resource_get_user_data(resource);
>  	struct weston_subsurface *sub = weston_surface_to_subsurface(surface);
>  
> +	if (!weston_surface_is_pending_viewport_source_valid(surface)) {
> +		assert(surface->viewport_resource);
> +
> +		wl_resource_post_error(surface->viewport_resource,
> +			WP_VIEWPORT_ERROR_OUT_OF_BUFFER,
> +			"wl_surface@%d has viewport source outside buffer",
> +			wl_resource_get_id(resource));
> +		return;
> +	}
> +
>  	if (sub) {
>  		weston_subsurface_commit(sub);
>  		return;
> -- 
> 2.7.3
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel