public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] tests/gem_mmap_gtt: add test mmap_closed_bo
@ 2022-12-22 10:04 Tvrtko Ursulin
  2022-12-22 11:01 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/gem_mmap_gtt: add test mmap_closed_bo (rev4) Patchwork
  2023-01-18  9:33 ` [igt-dev] [PATCH i-g-t] tests/gem_mmap_gtt: add test mmap_closed_bo Das, Nirmoy
  0 siblings, 2 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2022-12-22 10:04 UTC (permalink / raw)
  To: igt-dev, Intel-gfx

From: Chuansheng Liu <chuansheng.liu@intel.com>

Recently we figured out one memory leak in i915 driver when running
below alike test:

create_bo
gem_mmap_gtt bo
gem_mmap_gtt bo twice
close_bo

then the memory leak is detected. More details can be referred in
https://patchwork.freedesktop.org/patch/475802/?series=100532&rev=2

For detecting such issue, this test case mmap_closed_bo is created,
it will close the bo with keeping one mmap, then second mmap the bo,
in normal situation, we expect second mmap failure with EACCESS. But
it will succeed if driver has the vm_node allowance leak.

V2: (Tvrtko) some variable placement and comments tuning.
V3: (Tvrtko) Using igt_drop_caches_set(fd, DROP_FREED) directly.
v4: [tursulin] Fixed a conflict, added igt_describe.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/gem_mmap_gtt.c | 49 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
index c14ab50eeee7..68f5e8412af6 100644
--- a/tests/i915/gem_mmap_gtt.c
+++ b/tests/i915/gem_mmap_gtt.c
@@ -320,6 +320,52 @@ test_wc(int fd)
 		     5*gtt_writes/256., 5*cpu_writes/256.);
 }
 
+static void mmap_closed_bo(int fd)
+{
+	int loop = 0;
+
+	while (loop++ < 2) {
+		struct drm_i915_gem_mmap_gtt mmap_arg;
+		void *p1, *p2;
+		int i = loop;
+
+		memset(&mmap_arg, 0, sizeof(mmap_arg));
+		mmap_arg.handle = gem_create(fd, OBJECT_SIZE);
+		igt_assert(mmap_arg.handle);
+
+		while (i--) {
+			/*
+			 * Get mmap offset by calling GEM_MMAP_GTT one or multiple times in
+			 * order to try to provoke a memory leak in the driver.
+			 */
+			do_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg);
+		}
+
+		p1 = mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
+			MAP_SHARED, fd, mmap_arg.offset);
+		igt_assert(p1 != MAP_FAILED);
+
+		gem_close(fd, mmap_arg.handle);
+
+		/*
+		 * Drop the freed objects for consistent 2nd mmap result.
+		 */
+		igt_drop_caches_set(fd, DROP_FREED);
+
+		p2 = mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
+			MAP_SHARED, fd, mmap_arg.offset);
+
+		munmap(p1, OBJECT_SIZE);
+
+		/*
+		 * we expect mmapping p2 would fail, otherwise the driver
+		 * may not clean up the allowance of vm_node, it would
+		 * cause memory leak.
+		 */
+		igt_assert(p2 == MAP_FAILED);
+	}
+}
+
 static int mmap_gtt_version(int i915)
 {
 	int val = 0;
@@ -1315,6 +1361,9 @@ igt_main
 		     "writes into it from another GTT mmapped.");
 	igt_subtest("basic-write-gtt")
 		test_write_gtt(fd);
+	igt_describe("Check mmap of a closed handle is rejected.");
+	igt_subtest("mmap-closed-bo")
+		mmap_closed_bo(fd);
 	igt_describe("Inspect a GTT mmap using ptrace().");
 	igt_subtest("ptrace")
 		test_ptrace(fd);
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [igt-dev] [PATCH i-g-t] tests/gem_mmap_gtt: add test mmap_closed_bo
@ 2022-03-04  5:15 Chuansheng Liu
  2022-03-04 16:35 ` Tvrtko Ursulin
  0 siblings, 1 reply; 7+ messages in thread
From: Chuansheng Liu @ 2022-03-04  5:15 UTC (permalink / raw)
  To: igt-dev; +Cc: Tvrtko Ursulin

Recently we figured out one memory leak in i915 driver when running
below alike test:

create_bo
gem_mmap_gtt bo
gem_mmap_gtt bo twice
close_bo

then the memory leak is detected. More details can be referred in
https://patchwork.freedesktop.org/patch/475802/?series=100532&rev=2

For detecting such issue, this test case mmap_closed_bo is created,
it will close the bo with keeping one mmap, then second mmap the bo,
in normal situation, we expect second mmap failure with EACCESS. But
it will succeed if driver has the vm_node allowance leak.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
---
 tests/i915/gem_mmap_gtt.c | 42 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
index 92bbb5d2..0f4d5385 100644
--- a/tests/i915/gem_mmap_gtt.c
+++ b/tests/i915/gem_mmap_gtt.c
@@ -318,6 +318,46 @@ test_wc(int fd)
 		     5*gtt_writes/256., 5*cpu_writes/256.);
 }
 
+static void mmap_closed_bo(int fd)
+{
+	struct drm_i915_gem_mmap_gtt mmap_arg;
+	void *p1, *p2;
+	int loop = 0, i = 0;
+
+	while (loop++ < 2) {
+		memset(&mmap_arg, 0, sizeof(mmap_arg));
+
+		mmap_arg.handle = gem_create(fd, OBJECT_SIZE);
+		igt_assert(mmap_arg.handle);
+
+		i = loop;
+		while (i--) {
+			/* get offset, here we tries a loop to call GEM_MMAP_GTT many times,
+			 * it could trigger driver memory leak issue easily.
+			 */
+			do_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg);
+		}
+
+		p1 = mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
+			MAP_SHARED, fd, mmap_arg.offset);
+		igt_assert(p1 != MAP_FAILED);
+
+		gem_close(fd, mmap_arg.handle);
+		gem_quiescent_gpu(fd);
+
+		p2 = mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
+			MAP_SHARED, fd, mmap_arg.offset);
+
+		munmap(p1, OBJECT_SIZE);
+
+		/* we expect mmapping p2 would fail, otherwise the driver
+		 * may not clean up the allowance of vm_node, it would
+		 * cause memory leak.
+		 */
+		igt_assert(p2 == MAP_FAILED);
+	}
+}
+
 static int mmap_gtt_version(int i915)
 {
 	int val = 0;
@@ -1305,6 +1345,8 @@ igt_main
 		test_write(fd);
 	igt_subtest("basic-write-gtt")
 		test_write_gtt(fd);
+	igt_subtest("mmap_closed_bo")
+		mmap_closed_bo(fd);
 	igt_subtest("ptrace")
 		test_ptrace(fd);
 	igt_subtest("coherency")
-- 
2.25.0.rc2

^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-01-18  9:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-22 10:04 [igt-dev] [PATCH i-g-t] tests/gem_mmap_gtt: add test mmap_closed_bo Tvrtko Ursulin
2022-12-22 11:01 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/gem_mmap_gtt: add test mmap_closed_bo (rev4) Patchwork
2023-01-18  9:33 ` [igt-dev] [PATCH i-g-t] tests/gem_mmap_gtt: add test mmap_closed_bo Das, Nirmoy
  -- strict thread matches above, loose matches on Subject: below --
2022-03-04  5:15 Chuansheng Liu
2022-03-04 16:35 ` Tvrtko Ursulin
2022-03-07  1:02   ` Liu, Chuansheng
2022-03-07 10:10     ` Tvrtko Ursulin

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