public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] kms_cursor_crc: Kernel now checks for integer overflow
@ 2015-02-02 22:20 Matt Roper
  2015-02-03 14:33 ` Jani Nikula
  0 siblings, 1 reply; 3+ messages in thread
From: Matt Roper @ 2015-02-02 22:20 UTC (permalink / raw)
  To: intel-gfx

As of kernel commit

    commit a679064a7e9e8799177a64a31668a34a1bc6a4f1
    Author: Matt Roper <matthew.d.roper@intel.com>
    Date:   Fri Jan 30 16:22:37 2015 -0800

        drm/i915: Switch planes from transitional helpers to full atomic helpers

the kernel now checks for cursor coordinates that would result in
integer overflow and returns -ERANGE, similar to the checking that was
already done for other plane types.  We update kms_cursor_crc here to
reflect this small behavior change:
 * Check for success at extreme boundary conditions INT_MAX-{width,height}
   rather than INT_MAX
 * Add new check for success at SHRT_MAX; if the driver were to
   internally use short values and overflow, we could have the cursor
   reappear on the screen.
 * Add a test for failure with proper error code at INT_MAX-{width,height}+1

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 tests/kms_cursor_crc.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index e1390a7..64fea12 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -139,6 +139,29 @@ static void do_single_test(data_t *data, int x, int y)
 	igt_assert(igt_crc_equal(&crc, &ref_crc));
 }
 
+static void do_fail_test(data_t *data, int x, int y, int expect)
+{
+	igt_display_t *display = &data->display;
+	igt_plane_t *cursor;
+	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
+	int ret;
+
+	igt_print_activity();
+
+	/* Hardware test */
+	igt_paint_test_pattern(cr, data->screenw, data->screenh);
+	cursor_enable(data);
+	cursor = igt_output_get_plane(data->output, IGT_PLANE_CURSOR);
+	igt_plane_set_position(cursor, x, y);
+	ret = igt_display_try_commit2(display, COMMIT_LEGACY);
+
+	igt_plane_set_position(cursor, 0, 0);
+	cursor_disable(data);
+	igt_display_commit(display);
+
+	igt_assert(ret == expect);
+}
+
 static void do_test(data_t *data,
 		    int left, int right, int top, int bottom)
 {
@@ -201,7 +224,11 @@ static void test_crc_offscreen(data_t *data)
 	do_test(data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512));
 
 	/* go nuts */
-	do_test(data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
+	do_test(data, INT_MIN, INT_MAX - cursor_w, INT_MIN, INT_MAX - cursor_h);
+	do_test(data, SHRT_MIN, SHRT_MAX, SHRT_MIN, SHRT_MAX);
+
+	/* Make sure we get -ERANGE on integer overflow */
+	do_fail_test(data, INT_MAX - cursor_w + 1, INT_MAX - cursor_h + 1, -ERANGE);
 }
 
 static void test_crc_sliding(data_t *data)
-- 
1.8.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH i-g-t] kms_cursor_crc: Kernel now checks for integer overflow
  2015-02-02 22:20 [PATCH i-g-t] kms_cursor_crc: Kernel now checks for integer overflow Matt Roper
@ 2015-02-03 14:33 ` Jani Nikula
  2015-02-03 15:36   ` Matt Roper
  0 siblings, 1 reply; 3+ messages in thread
From: Jani Nikula @ 2015-02-03 14:33 UTC (permalink / raw)
  To: Matt Roper, intel-gfx

On Tue, 03 Feb 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> As of kernel commit
>
>     commit a679064a7e9e8799177a64a31668a34a1bc6a4f1
>     Author: Matt Roper <matthew.d.roper@intel.com>
>     Date:   Fri Jan 30 16:22:37 2015 -0800
>
>         drm/i915: Switch planes from transitional helpers to full atomic helpers
>
> the kernel now checks for cursor coordinates that would result in
> integer overflow and returns -ERANGE, similar to the checking that was
> already done for other plane types.  We update kms_cursor_crc here to
> reflect this small behavior change:
>  * Check for success at extreme boundary conditions INT_MAX-{width,height}
>    rather than INT_MAX
>  * Add new check for success at SHRT_MAX; if the driver were to
>    internally use short values and overflow, we could have the cursor
>    reappear on the screen.
>  * Add a test for failure with proper error code at INT_MAX-{width,height}+1
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88917

?


> ---
>  tests/kms_cursor_crc.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index e1390a7..64fea12 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -139,6 +139,29 @@ static void do_single_test(data_t *data, int x, int y)
>  	igt_assert(igt_crc_equal(&crc, &ref_crc));
>  }
>  
> +static void do_fail_test(data_t *data, int x, int y, int expect)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_plane_t *cursor;
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> +	int ret;
> +
> +	igt_print_activity();
> +
> +	/* Hardware test */
> +	igt_paint_test_pattern(cr, data->screenw, data->screenh);
> +	cursor_enable(data);
> +	cursor = igt_output_get_plane(data->output, IGT_PLANE_CURSOR);
> +	igt_plane_set_position(cursor, x, y);
> +	ret = igt_display_try_commit2(display, COMMIT_LEGACY);
> +
> +	igt_plane_set_position(cursor, 0, 0);
> +	cursor_disable(data);
> +	igt_display_commit(display);
> +
> +	igt_assert(ret == expect);
> +}
> +
>  static void do_test(data_t *data,
>  		    int left, int right, int top, int bottom)
>  {
> @@ -201,7 +224,11 @@ static void test_crc_offscreen(data_t *data)
>  	do_test(data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512));
>  
>  	/* go nuts */
> -	do_test(data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
> +	do_test(data, INT_MIN, INT_MAX - cursor_w, INT_MIN, INT_MAX - cursor_h);
> +	do_test(data, SHRT_MIN, SHRT_MAX, SHRT_MIN, SHRT_MAX);
> +
> +	/* Make sure we get -ERANGE on integer overflow */
> +	do_fail_test(data, INT_MAX - cursor_w + 1, INT_MAX - cursor_h + 1, -ERANGE);
>  }
>  
>  static void test_crc_sliding(data_t *data)
> -- 
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH i-g-t] kms_cursor_crc: Kernel now checks for integer overflow
  2015-02-03 14:33 ` Jani Nikula
@ 2015-02-03 15:36   ` Matt Roper
  0 siblings, 0 replies; 3+ messages in thread
From: Matt Roper @ 2015-02-03 15:36 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Feb 03, 2015 at 04:33:51PM +0200, Jani Nikula wrote:
> On Tue, 03 Feb 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > As of kernel commit
> >
> >     commit a679064a7e9e8799177a64a31668a34a1bc6a4f1
> >     Author: Matt Roper <matthew.d.roper@intel.com>
> >     Date:   Fri Jan 30 16:22:37 2015 -0800
> >
> >         drm/i915: Switch planes from transitional helpers to full atomic helpers
> >
> > the kernel now checks for cursor coordinates that would result in
> > integer overflow and returns -ERANGE, similar to the checking that was
> > already done for other plane types.  We update kms_cursor_crc here to
> > reflect this small behavior change:
> >  * Check for success at extreme boundary conditions INT_MAX-{width,height}
> >    rather than INT_MAX
> >  * Add new check for success at SHRT_MAX; if the driver were to
> >    internally use short values and overflow, we could have the cursor
> >    reappear on the screen.
> >  * Add a test for failure with proper error code at INT_MAX-{width,height}+1
> >
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> 
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88917
> 
> ?

Yep, this should fix that one.  Good catch.


Matt

> 
> 
> > ---
> >  tests/kms_cursor_crc.c | 29 ++++++++++++++++++++++++++++-
> >  1 file changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index e1390a7..64fea12 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -139,6 +139,29 @@ static void do_single_test(data_t *data, int x, int y)
> >  	igt_assert(igt_crc_equal(&crc, &ref_crc));
> >  }
> >  
> > +static void do_fail_test(data_t *data, int x, int y, int expect)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	igt_plane_t *cursor;
> > +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> > +	int ret;
> > +
> > +	igt_print_activity();
> > +
> > +	/* Hardware test */
> > +	igt_paint_test_pattern(cr, data->screenw, data->screenh);
> > +	cursor_enable(data);
> > +	cursor = igt_output_get_plane(data->output, IGT_PLANE_CURSOR);
> > +	igt_plane_set_position(cursor, x, y);
> > +	ret = igt_display_try_commit2(display, COMMIT_LEGACY);
> > +
> > +	igt_plane_set_position(cursor, 0, 0);
> > +	cursor_disable(data);
> > +	igt_display_commit(display);
> > +
> > +	igt_assert(ret == expect);
> > +}
> > +
> >  static void do_test(data_t *data,
> >  		    int left, int right, int top, int bottom)
> >  {
> > @@ -201,7 +224,11 @@ static void test_crc_offscreen(data_t *data)
> >  	do_test(data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512));
> >  
> >  	/* go nuts */
> > -	do_test(data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
> > +	do_test(data, INT_MIN, INT_MAX - cursor_w, INT_MIN, INT_MAX - cursor_h);
> > +	do_test(data, SHRT_MIN, SHRT_MAX, SHRT_MIN, SHRT_MAX);
> > +
> > +	/* Make sure we get -ERANGE on integer overflow */
> > +	do_fail_test(data, INT_MAX - cursor_w + 1, INT_MAX - cursor_h + 1, -ERANGE);
> >  }
> >  
> >  static void test_crc_sliding(data_t *data)
> > -- 
> > 1.8.5.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-02-03 15:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-02 22:20 [PATCH i-g-t] kms_cursor_crc: Kernel now checks for integer overflow Matt Roper
2015-02-03 14:33 ` Jani Nikula
2015-02-03 15:36   ` Matt Roper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox