[PATCH i-g-t] lib|tests: Don't dereference engine outside for_each_ctx_engine macro

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Tue Aug 13 12:45:36 UTC 2024


Engine used inside for_each_ctx_engine() macro is valid only inside its
block as it is accessing local stack memory. Using outside the block works
by an accident if luckily nothing will overwrite this stack before use.
Fix found risky usages by using engine only inside the block or do its
copy.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
Cc: Andi Shyti <andi.shyti at linux.intel.com>
---
 lib/igt_fb.c                    |  8 ++++----
 tests/intel/gem_lmem_swapping.c | 14 ++++++++++----
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ab162a5b74..addff2cfba 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -3050,13 +3050,13 @@ static void blitcopy(const struct igt_fb *dst_fb,
 						   dst_fb->size);
 		} else if (ahnd && block_copy_ok(src_fb) && block_copy_ok(dst_fb)) {
 			for_each_ctx_engine(src_fb->fd, ictx, e) {
-				if (gem_engine_can_block_copy(src_fb->fd, e))
+				if (gem_engine_can_block_copy(src_fb->fd, e)) {
+					do_block_copy(src_fb, dst_fb, mem_region, i, ahnd,
+						      bb, bb_size, ictx, e);
 					break;
+				}
 			}
 			igt_assert_f(e, "No block copy capable engine found!\n");
-
-			do_block_copy(src_fb, dst_fb, mem_region, i, ahnd,
-				      bb, bb_size, ictx, e);
 		} else {
 			igt_blitter_src_copy(dst_fb->fd,
 					     ahnd, ctx, NULL,
diff --git a/tests/intel/gem_lmem_swapping.c b/tests/intel/gem_lmem_swapping.c
index b8f24742b8..b125261519 100644
--- a/tests/intel/gem_lmem_swapping.c
+++ b/tests/intel/gem_lmem_swapping.c
@@ -189,6 +189,7 @@ init_object_ccs(int i915, struct object *obj, struct blt_copy_object *tmp,
 {
 	struct blt_block_copy_data_ext ext = {}, *pext = &ext;
 	const struct intel_execution_engine2 *e;
+	struct intel_execution_engine2 ec;
 	struct blt_copy_data blt = {};
 	struct blt_copy_batch *cmd;
 	uint64_t size = 4096;
@@ -196,8 +197,10 @@ init_object_ccs(int i915, struct object *obj, struct blt_copy_object *tmp,
 
 	obj->seed = seed;
 	for_each_ctx_engine(i915, ctx, e) {
-		if (gem_engine_can_block_copy(i915, e))
+		if (gem_engine_can_block_copy(i915, e)) {
+			ec = *e;
 			break;
+		}
 	}
 	igt_assert_f(e, "Ctx don't have blt engine\n");
 
@@ -224,7 +227,7 @@ init_object_ccs(int i915, struct object *obj, struct blt_copy_object *tmp,
 	blt_set_object_ext(&ext.dst, 0, obj->blt_obj->x2, obj->blt_obj->y2,
 			   SURFACE_TYPE_2D);
 
-	blt_block_copy(i915, ctx, e, ahnd, &blt, pext);
+	blt_block_copy(i915, ctx, &ec, ahnd, &blt, pext);
 	free(cmd);
 }
 
@@ -255,14 +258,17 @@ verify_object_ccs(int i915, const struct object *obj,
 {
 	struct blt_block_copy_data_ext ext = {}, *pext = &ext;
 	const struct intel_execution_engine2 *e;
+	struct intel_execution_engine2 ec;
 	struct blt_copy_data blt = {};
 	struct blt_copy_batch *cmd;
 	uint64_t size = 4096;
 	unsigned long j, val, *buf;
 
 	for_each_ctx_engine(i915, ctx, e) {
-		if (gem_engine_can_block_copy(i915, e))
+		if (gem_engine_can_block_copy(i915, e)) {
+			ec = *e;
 			break;
+		}
 	}
 	igt_assert_f(e, "Ctx don't have blt engine\n");
 
@@ -284,7 +290,7 @@ verify_object_ccs(int i915, const struct object *obj,
 	blt_set_object_ext(&ext.src, 0, obj->blt_obj->x2, obj->blt_obj->y2,
 			   SURFACE_TYPE_2D);
 	blt_set_object_ext(&ext.dst, 0, tmp->x2, tmp->y2, SURFACE_TYPE_2D);
-	blt_block_copy(i915, ctx, e, ahnd, &blt, pext);
+	blt_block_copy(i915, ctx, &ec, ahnd, &blt, pext);
 
 	buf = gem_mmap__device_coherent(i915, tmp->handle, 0,
 					obj->size, PROT_READ);
-- 
2.34.1



More information about the igt-dev mailing list