Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: "Christian König" <christian.koenig@amd.com>,
	"Maxime Ripard" <mripard@kernel.org>
Subject: [Intel-xe] [PATCH v3 2/2] drm/tests/drm_exec: Add a test for object freeing within drm_exec_fini()
Date: Thu,  7 Sep 2023 15:53:39 +0200	[thread overview]
Message-ID: <20230907135339.7971-3-thomas.hellstrom@linux.intel.com> (raw)
In-Reply-To: <20230907135339.7971-1-thomas.hellstrom@linux.intel.com>

Check that object freeing from within drm_exec_fini() works as expected
and is unlikely to generate any warnings.

v3:
- Condition the test on CONFIG_DEBUG_LOCK_ALLOC
- Make the test fail if the situation that generates the lockdep
  warning occurs. (Maxime Ripard)

Cc: Maxime Ripard <mripard@kernel.org>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/tests/drm_exec_test.c | 82 +++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_exec_test.c b/drivers/gpu/drm/tests/drm_exec_test.c
index 563949d777dd..83fddc6fe1ae 100644
--- a/drivers/gpu/drm/tests/drm_exec_test.c
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -21,6 +21,9 @@
 struct drm_exec_priv {
 	struct device *dev;
 	struct drm_device *drm;
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	struct drm_exec *exec;
+#endif
 };
 
 static int drm_exec_test_init(struct kunit *test)
@@ -170,6 +173,82 @@ static void test_prepare_array(struct kunit *test)
 	drm_gem_private_object_fini(&gobj2);
 }
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static void drm_exec_test_obj_free(struct drm_gem_object *gem)
+{
+	struct kunit *test = current->kunit_test;
+	struct drm_exec_priv *priv = test->priv;
+	bool resv_class_held;
+	bool first_object_locked;
+
+	/*
+	 * The lock alloc tracking code may warn if the dma_resv lock
+	 * class is still held, and we're freeing the first object we
+	 * locked.
+	 */
+	resv_class_held = (lockdep_is_held(&gem->resv->lock.base) ==
+			   LOCK_STATE_HELD);
+	first_object_locked = (gem == priv->exec->objects[0]);
+	KUNIT_EXPECT_FALSE(current->kunit_test,
+			   resv_class_held && first_object_locked);
+
+	dma_resv_fini(gem->resv);
+	kfree(gem);
+}
+
+static const struct drm_gem_object_funcs put_funcs = {
+	.free = drm_exec_test_obj_free,
+};
+
+/*
+ * Check that freeing objects from within drm_exec_fini()
+ * doesn't trigger a false lock alloc warning due to
+ * the dma_resv lock *class* still being held and we're
+ * freeing the first object locked, which *might* be
+ * registered as the address of the held lock of that
+ * lock class.
+ */
+static void test_early_put(struct kunit *test)
+{
+	struct drm_exec_priv *priv = test->priv;
+	struct drm_gem_object *gobj1;
+	struct drm_gem_object *gobj2;
+	struct drm_gem_object *array[2];
+	struct drm_exec exec;
+	int ret;
+
+	priv->exec = &exec;
+
+	gobj1 = kzalloc(sizeof(*gobj1), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_NULL(test, gobj1);
+	if (!gobj1)
+		return;
+
+	gobj2 = kzalloc(sizeof(*gobj2), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_NULL(test, gobj2);
+	if (!gobj2) {
+		kfree(gobj1);
+		return;
+	}
+
+	gobj1->funcs = &put_funcs;
+	gobj2->funcs = &put_funcs;
+	drm_gem_private_object_init(priv->drm, gobj1, PAGE_SIZE);
+	drm_gem_private_object_init(priv->drm, gobj2, PAGE_SIZE);
+	array[0] = gobj1;
+	array[1] = gobj2;
+
+	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+	drm_exec_until_all_locked(&exec)
+		ret = drm_exec_prepare_array(&exec, array, ARRAY_SIZE(array),
+					     1);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	drm_gem_object_put(gobj1);
+	drm_gem_object_put(gobj2);
+	drm_exec_fini(&exec);
+}
+#endif
+
 static void test_multiple_loops(struct kunit *test)
 {
 	struct drm_exec exec;
@@ -198,6 +277,9 @@ static struct kunit_case drm_exec_tests[] = {
 	KUNIT_CASE(test_prepare),
 	KUNIT_CASE(test_prepare_array),
 	KUNIT_CASE(test_multiple_loops),
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	KUNIT_CASE(test_early_put),
+#endif
 	{}
 };
 
-- 
2.41.0


  parent reply	other threads:[~2023-09-07 13:59 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-07 13:53 [Intel-xe] [PATCH v3 0/2] drm/tests: Fix for UAF and a test for drm_exec lock alloc tracking warning Thomas Hellström
2023-09-07 13:53 ` [Intel-xe] [PATCH v3 1/2] drm/tests: helpers: Avoid a driver uaf Thomas Hellström
2023-09-07 14:50   ` Maxime Ripard
2023-09-11 12:40   ` Francois Dugast
2023-09-11 13:04     ` Thomas Hellström
2023-09-14 11:59   ` [Intel-xe] (subset) " Maxime Ripard
2023-09-07 13:53 ` Thomas Hellström [this message]
2023-09-07 14:52   ` [Intel-xe] [PATCH v3 2/2] drm/tests/drm_exec: Add a test for object freeing within drm_exec_fini() Maxime Ripard
2023-09-07 14:37 ` [Intel-xe] [PATCH v3 0/2] drm/tests: Fix for UAF and a test for drm_exec lock alloc tracking warning Christian König
2023-09-07 14:47   ` Thomas Hellström
2023-09-07 14:49     ` Christian König
2023-09-08  7:37       ` Thomas Hellström
2023-09-08  8:52         ` Christian König
2023-09-08  9:04           ` Thomas Hellström
2023-09-08  9:14             ` Christian König
2023-09-08 11:13               ` Thomas Hellström
2023-09-08 14:31                 ` Thomas Hellström
2023-09-07 23:49 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
2023-09-07 23:49 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-09-07 23:50 ` [Intel-xe] ✓ CI.KUnit: success " Patchwork
2023-09-07 23:57 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-09-07 23:57 ` [Intel-xe] ✓ CI.Hooks: " Patchwork
2023-09-07 23:59 ` [Intel-xe] ✓ CI.checksparse: " Patchwork
2023-09-08  0:30 ` [Intel-xe] ✓ CI.BAT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230907135339.7971-3-thomas.hellstrom@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=mripard@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox