Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] lib|tests: Don't dereference engine outside for_each_ctx_engine macro
@ 2024-08-13 12:45 Zbigniew Kempczyński
  2024-08-13 13:00 ` ✗ GitLab.Pipeline: warning for " Patchwork
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Zbigniew Kempczyński @ 2024-08-13 12:45 UTC (permalink / raw)
  To: igt-dev; +Cc: Zbigniew Kempczyński, Andi Shyti

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@intel.com>
Cc: Andi Shyti <andi.shyti@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


^ permalink raw reply related	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-08-14  9:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-13 12:45 [PATCH i-g-t] lib|tests: Don't dereference engine outside for_each_ctx_engine macro Zbigniew Kempczyński
2024-08-13 13:00 ` ✗ GitLab.Pipeline: warning for " Patchwork
2024-08-13 13:13 ` ✗ CI.xeBAT: failure " Patchwork
2024-08-13 13:16 ` ✓ Fi.CI.BAT: success " Patchwork
2024-08-13 13:36 ` [PATCH i-g-t] " Peter Senna Tschudin
2024-08-13 13:56   ` Zbigniew Kempczyński
2024-08-13 14:12     ` Peter Senna Tschudin
2024-08-13 19:51       ` Andi Shyti
2024-08-14  7:11         ` Peter Senna Tschudin
2024-08-14  9:34           ` Andi Shyti
2024-08-13 14:55 ` ✗ CI.xeBAT: failure for lib|tests: Don't dereference engine outside for_each_ctx_engine macro (rev2) Patchwork
2024-08-13 15:06 ` ✓ Fi.CI.BAT: success " Patchwork
2024-08-13 16:18 ` ✗ CI.xeFULL: failure for lib|tests: Don't dereference engine outside for_each_ctx_engine macro Patchwork
2024-08-14  8:52   ` Zbigniew Kempczyński
2024-08-13 17:14 ` ✗ CI.xeFULL: failure for lib|tests: Don't dereference engine outside for_each_ctx_engine macro (rev2) Patchwork
2024-08-13 19:49 ` ✓ Fi.CI.IGT: success for lib|tests: Don't dereference engine outside for_each_ctx_engine macro Patchwork
2024-08-13 22:49 ` ✗ Fi.CI.IGT: failure for lib|tests: Don't dereference engine outside for_each_ctx_engine macro (rev2) Patchwork
2024-08-14  8:50   ` Zbigniew Kempczyński
2024-08-14  7:10 ` [PATCH i-g-t] lib|tests: Don't dereference engine outside for_each_ctx_engine macro Peter Senna Tschudin

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