[PATCH] drm/i915: Reject out-of-bound sprites

[PATCH] drm/i915: Reject out-of-bound sprites

[Chris Wilson]

We make no attempt to fixup the source as we adjust the destination
rectangle, expect the caller to have already fixed that up. This makes
no sense, asking userspace to compensate for the kernel adjusting the
one rectangle not the other, so simply reject any attempt to show an
partially visible box. Userspace should know better.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_sprite.c |   31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 22d65f7..97866c2 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -498,31 +498,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	}
 
 	/*
-	 * Clamp the width & height into the visible area.  Note we don't
-	 * try to scale the source if part of the visible region is offscreen.
-	 * The caller must handle that by adjusting source offset and size.
+	 * Reject any attempt to display video outside the visible area.
+	 * The caller must handle this by adjusting source offset and size.
 	 */
-	if ((crtc_x < 0) && ((crtc_x + crtc_w) > 0)) {
-		crtc_w += crtc_x;
-		crtc_x = 0;
-	}
-	if ((crtc_x + crtc_w) <= 0) /* Nothing to display */
-		goto out;
-	if ((crtc_x + crtc_w) > primary_w)
-		crtc_w = primary_w - crtc_x;
-
-	if ((crtc_y < 0) && ((crtc_y + crtc_h) > 0)) {
-		crtc_h += crtc_y;
-		crtc_y = 0;
-	}
-	if ((crtc_y + crtc_h) <= 0) /* Nothing to display */
-		goto out;
-	if (crtc_y + crtc_h > primary_h)
-		crtc_h = primary_h - crtc_y;
-
-	if (!crtc_w || !crtc_h) /* Again, nothing to display */
-		goto out;
-
+	if (crtc_x < 0 || crtc_x + crtc_w > primary_w)
+		return -EINVAL;
+	if (crtc_y < 0 || crtc_y + crtc_h > primary_h)
+		return -EINVAL;
 	/*
 	 * We may not have a scaler, eg. HSW does not have it any more
 	 */
@@ -571,7 +553,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 out_unlock:
 	mutex_unlock(&dev->struct_mutex);
-out:
 	return ret;
 }
 
-- 
1.7.10.4

Re: [PATCH] drm/i915: Reject out-of-bound sprites

[Daniel Vetter]

On Tue, Dec 18, 2012 at 10:18:28PM +0000, Chris Wilson wrote:
> We make no attempt to fixup the source as we adjust the destination
> rectangle, expect the caller to have already fixed that up. This makes
> no sense, asking userspace to compensate for the kernel adjusting the
> one rectangle not the other, so simply reject any attempt to show an
> partially visible box. Userspace should know better.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I guess this makes sense for now, given that the only user we have (ddx)
needs to copy things around anyway ... Still, some common users and
testcases across drivers would be nice for this </wishful-thinking>.

Applied to fixes.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_sprite.c |   31 ++++++-------------------------
>  1 file changed, 6 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 22d65f7..97866c2 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -498,31 +498,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	}
>  
>  	/*
> -	 * Clamp the width & height into the visible area.  Note we don't
> -	 * try to scale the source if part of the visible region is offscreen.
> -	 * The caller must handle that by adjusting source offset and size.
> +	 * Reject any attempt to display video outside the visible area.
> +	 * The caller must handle this by adjusting source offset and size.
>  	 */
> -	if ((crtc_x < 0) && ((crtc_x + crtc_w) > 0)) {
> -		crtc_w += crtc_x;
> -		crtc_x = 0;
> -	}
> -	if ((crtc_x + crtc_w) <= 0) /* Nothing to display */
> -		goto out;
> -	if ((crtc_x + crtc_w) > primary_w)
> -		crtc_w = primary_w - crtc_x;
> -
> -	if ((crtc_y < 0) && ((crtc_y + crtc_h) > 0)) {
> -		crtc_h += crtc_y;
> -		crtc_y = 0;
> -	}
> -	if ((crtc_y + crtc_h) <= 0) /* Nothing to display */
> -		goto out;
> -	if (crtc_y + crtc_h > primary_h)
> -		crtc_h = primary_h - crtc_y;
> -
> -	if (!crtc_w || !crtc_h) /* Again, nothing to display */
> -		goto out;
> -
> +	if (crtc_x < 0 || crtc_x + crtc_w > primary_w)
> +		return -EINVAL;
> +	if (crtc_y < 0 || crtc_y + crtc_h > primary_h)
> +		return -EINVAL;
>  	/*
>  	 * We may not have a scaler, eg. HSW does not have it any more
>  	 */
> @@ -571,7 +553,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  out_unlock:
>  	mutex_unlock(&dev->struct_mutex);
> -out:
>  	return ret;
>  }
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Reject out-of-bound sprites

[Daniel Vetter]

On Tue, Jan 08, 2013 at 12:11:38PM +0100, Daniel Vetter wrote:
> On Tue, Dec 18, 2012 at 10:18:28PM +0000, Chris Wilson wrote:
> > We make no attempt to fixup the source as we adjust the destination
> > rectangle, expect the caller to have already fixed that up. This makes
> > no sense, asking userspace to compensate for the kernel adjusting the
> > one rectangle not the other, so simply reject any attempt to show an
> > partially visible box. Userspace should know better.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> I guess this makes sense for now, given that the only user we have (ddx)
> needs to copy things around anyway ... Still, some common users and
> testcases across drivers would be nice for this </wishful-thinking>.
> 
> Applied to fixes.

Dropped again, since Ville has a more complete solution to this in his
queue.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch