diff mbox series

[i-g-t] tests/i915/query: Check no buffer overwrite

Message ID 20191122104307.30412-1-tvrtko.ursulin@linux.intel.com
State New, archived
Headers show
Series [i-g-t] tests/i915/query: Check no buffer overwrite | expand

Commit Message

Tvrtko Ursulin Nov. 22, 2019, 10:43 a.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Check that the engine query is not polluting the buffer past the size it
indicated it would write.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/i915_query.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Chris Wilson Nov. 22, 2019, 10:50 a.m. UTC | #1
Quoting Tvrtko Ursulin (2019-11-22 10:43:07)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Check that the engine query is not polluting the buffer past the size it
> indicated it would write.

True and definitely useful to check.

> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/i915/i915_query.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/i915_query.c b/tests/i915/i915_query.c
> index ecbec3ae141d..92dd8f48a5d0 100644
> --- a/tests/i915/i915_query.c
> +++ b/tests/i915/i915_query.c
> @@ -496,7 +496,8 @@ static void engines_invalid(int fd)
>  {
>         struct drm_i915_query_engine_info *engines;
>         struct drm_i915_query_item item;
> -       unsigned int len;
> +       unsigned int i, len;
> +       char *buf;
>  
>         /* Flags is MBZ. */
>         memset(&item, 0, sizeof(item));
> @@ -574,6 +575,20 @@ static void engines_invalid(int fd)
>                        -1, 0);
>         igt_assert(engines != MAP_FAILED);
>  
> +       /* Check no write past len. */
> +       memset(engines, 0, 4096);
> +       memset(&item, 0, sizeof(item));
> +       item.query_id = DRM_I915_QUERY_ENGINE_INFO;
> +       item.length = len;
> +       item.data_ptr = to_user_pointer(engines);
> +       i915_query_items(fd, &item, 1);
> +       igt_assert_eq(item.length, len);
> +       buf = (char *)engines;
> +       buf += len;
> +       for (i = 0; i < 4096 - len; i++, buf++)
> +               igt_assert_f(*buf == 0, "Garbage %u bytes after buffer! (%x)\n",
> +                            i, *buf);

I would suggest some non-zero value, maybe even random, as zero-extend
is a contender for a likely kernel bug.

Otherwise,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/tests/i915/i915_query.c b/tests/i915/i915_query.c
index ecbec3ae141d..92dd8f48a5d0 100644
--- a/tests/i915/i915_query.c
+++ b/tests/i915/i915_query.c
@@ -496,7 +496,8 @@  static void engines_invalid(int fd)
 {
 	struct drm_i915_query_engine_info *engines;
 	struct drm_i915_query_item item;
-	unsigned int len;
+	unsigned int i, len;
+	char *buf;
 
 	/* Flags is MBZ. */
 	memset(&item, 0, sizeof(item));
@@ -574,6 +575,20 @@  static void engines_invalid(int fd)
 		       -1, 0);
 	igt_assert(engines != MAP_FAILED);
 
+	/* Check no write past len. */
+	memset(engines, 0, 4096);
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, len);
+	buf = (char *)engines;
+	buf += len;
+	for (i = 0; i < 4096 - len; i++, buf++)
+		igt_assert_f(*buf == 0, "Garbage %u bytes after buffer! (%x)\n",
+			     i, *buf);
+
 	/* PROT_NONE is similar to unmapped area. */
 	memset(engines, 0, len);
 	igt_assert_eq(mprotect(engines, len, PROT_NONE), 0);