[PATCH v4 2/2] tests/intel/gem_change_handle_race: Add edge-case and

From: Vitaly Prosyak <[email protected]>

Extends gem_change_handle_race test suite with 5 additional subtests
covering edge cases and functional verification of the
DRM_IOCTL_GEM_CHANGE_HANDLE ioctl.

New subtests:
- noop-same-handle: Verifies handle == new_handle is a noop
- invalid-new-handle-exceeds-int-max: new_handle > INT_MAX returns -EINVAL
- invalid-handle-nonexistent: Non-existent handle returns -ENOENT
- edge-new-handle-zero: Documents kernel behavior for new_handle == 0
- functional-rename-verification: Verifies actual rename operation works

These tests complement the race condition testing from the first patch
by validating error handling and functional correctness.

v4 changes:
 - This patch is now completely clean: only adds 5 new test functions
 - No modifications to patch 1 code (all fixes moved to patch 1)
 - Stats: 234 insertions, 0 deletions (vs 252 insertions, 20 deletions in v3)

v3 changes:
 - Sanitized test names and comments per Kamil's feedback
 - Added comprehensive edge-case coverage

v2 changes:
 - Added these 5 subtests to complement v1's race tests

v1:
 - Not present (only 7 race tests in v1)

Signed-off-by: Vitaly Prosyak <[email protected]>
Change-Id: Icf676f626e353e0a11ba80e1a88e28627231b3ba
---
 tests/intel/gem_change_handle_race.c | 234 +++++++++++++++++++++++++++
 1 file changed, 234 insertions(+)

diff --git a/tests/intel/gem_change_handle_race.c b/tests/intel/gem_change_handle_race.c
index 5fde7b1aa..55a799bdc 100644
--- a/tests/intel/gem_change_handle_race.c
+++ b/tests/intel/gem_change_handle_race.c
@@ -1741,3 +1741,237 @@ static void test_race_close_before_lock(struct gpu_ctx *ctx)
  * When old handle equals new handle, the ioctl should return success
  * without modifying anything.
  */
+static void test_noop_same_handle(struct gpu_ctx *ctx)
+{
+	uint32_t handle;
+
+	handle = gem_create_bo(ctx, 4096);
+
+	/* Renaming to the same handle should succeed (noop) */
+	igt_assert_eq(gem_change_handle(ctx, handle, handle), 0);
+
+	/* Handle should still be valid */
+	igt_assert_eq(gem_close_bo(ctx, handle), 0);
+
+	igt_info("  PASS: handle == new_handle is a noop\n");
+}
+
+/**
+ * test_new_handle_exceeds_int_max - new_handle > INT_MAX should fail
+ *
+ * The kernel enforces idr_alloc() limitation: new_handle must be <= INT_MAX.
+ */
+static void test_new_handle_exceeds_int_max(struct gpu_ctx *ctx)
+{
+	uint32_t handle;
+	int ret;
+
+	handle = gem_create_bo(ctx, 4096);
+
+	/* new_handle > INT_MAX must fail with -EINVAL */
+	ret = gem_change_handle(ctx, handle, (uint32_t)INT_MAX + 1);
+	igt_assert(ret < 0);
+	igt_assert_eq(errno, EINVAL);
+
+	/* Also test 0xFFFFFFFF */
+	ret = gem_change_handle(ctx, handle, 0xFFFFFFFF);
+	igt_assert(ret < 0);
+	igt_assert_eq(errno, EINVAL);
+
+	/* Original handle should still be valid */
+	igt_assert_eq(gem_close_bo(ctx, handle), 0);
+
+	igt_info("  PASS: new_handle > INT_MAX returns -EINVAL\n");
+}
+
+/**
+ * test_invalid_handle - non-existent handle should fail
+ *
+ * Using a handle that doesn't exist should return -ENOENT.
+ */
+static void test_invalid_handle(struct gpu_ctx *ctx)
+{
+	int ret;
+
+	/* Use a handle that was never allocated */
+	ret = gem_change_handle(ctx, 0xDEAD, 0xBEEF);
+	igt_assert(ret < 0);
+	igt_assert_eq(errno, ENOENT);
+
+	/* Handle 0 is also invalid */
+	ret = gem_change_handle(ctx, 0, 100);
+	igt_assert(ret < 0);
+	igt_assert_eq(errno, ENOENT);
+
+	igt_info("  PASS: invalid/non-existent handle returns -ENOENT\n");
+}
+
+/**
+ * test_new_handle_zero - new_handle == 0 (unhandled case)
+ *
+ * Dave got a report that new_handle == 0 is not properly handled.
+ * Document the current kernel behavior.
+ */
+static void test_new_handle_zero(struct gpu_ctx *ctx)
+{
+	uint32_t handle;
+	int ret;
+
+	handle = gem_create_bo(ctx, 4096);
+
+	/*
+	 * new_handle == 0: This is an unhandled edge case.
+	 * Handle 0 is typically reserved/invalid in DRM.
+	 * The kernel should reject this with -EINVAL.
+	 */
+	ret = gem_change_handle(ctx, handle, 0);
+	if (ret < 0) {
+		igt_info("  new_handle=0 rejected with errno=%d (%s)\n",
+			 errno, strerror(errno));
+		/* Expected: kernel rejects handle 0 */
+		igt_assert(errno == EINVAL || errno == ENOENT || errno == ENOSPC);
+		/* Original handle still valid */
+		igt_assert_eq(gem_close_bo(ctx, handle), 0);
+	} else {
+		/*
+		 * If kernel allowed it, the object moved to handle 0.
+		 * This is arguably a bug - document it.
+		 */
+		igt_warn("  WARNING: kernel allowed new_handle=0 (may be a bug)\n");
+		/* Old handle should be gone */
+		igt_assert(gem_close_bo(ctx, handle) < 0);
+		/* New handle 0 should exist */
+		igt_assert_eq(gem_close_bo(ctx, 0), 0);
+	}
+
+	igt_info("  PASS: new_handle=0 edge case handled\n");
+}
+
+/**
+ * test_functional_rename - verify rename actually works
+ *
+ * After a successful rename:
+ * - GEM_CLOSE(old_handle) must FAIL (handle no longer exists)
+ * - GEM_CLOSE(new_handle) must SUCCEED (object is there)
+ *
+ * Simona suspects the merged version of Francis' patch may have broken
+ * this fundamental behavior.
+ */
+static void test_functional_rename(struct gpu_ctx *ctx)
+{
+	uint32_t handle, new_h;
+	int ret;
+
+	handle = gem_create_bo(ctx, 4096);
+	new_h = handle + 1000; /* Pick a handle far away to avoid collisions */
+
+	/* Perform the rename */
+	ret = gem_change_handle(ctx, handle, new_h);
+	igt_assert_f(ret == 0,
+		     "gem_change_handle(%u -> %u) failed: %s\n",
+		     handle, new_h, strerror(errno));
+
+	/* Old handle must be invalid now */
+	ret = gem_close_bo(ctx, handle);
+	igt_assert_f(ret < 0 && errno == EINVAL,
+		     "GEM_CLOSE(old_handle=%u) should fail with EINVAL, "
+		     "got ret=%d errno=%d (%s)\n",
+		     handle, ret, errno, strerror(errno));
+
+	/* New handle must be valid */
+	ret = gem_close_bo(ctx, new_h);
+	igt_assert_f(ret == 0,
+		     "GEM_CLOSE(new_handle=%u) should succeed, "
+		     "got ret=%d errno=%d (%s)\n",
+		     new_h, ret, errno, strerror(errno));
+
+	igt_info("  PASS: rename works - old handle invalid, new handle valid\n");
+}
+
+int igt_main()
+{
+	struct gpu_ctx ctx = { .fd = -1 };
+
+	igt_fixture() {
+		ctx.fd = drm_open_driver_render(DRIVER_ANY);
+		igt_require(ctx.fd >= 0);
+
+		ctx.driver = detect_driver(ctx.fd);
+		igt_require_f(ctx.driver != DRIVER_TYPE_UNKNOWN,
+			      "Unsupported GPU driver\n");
+
+		igt_info("═══════════════════════════════════════════════\n");
+		igt_info("  GPU-Agnostic Race Condition Tests\n");
+		igt_info("  Running on: %s\n", gpu_name(ctx.driver));
+		igt_info("═══════════════════════════════════════════════\n");
+
+#if HAS_AMDGPU
+		if (ctx.driver == DRIVER_TYPE_AMDGPU) {
+			uint32_t major, minor;
+			int err = amdgpu_device_initialize(ctx.fd, &major, &minor,
+							   &ctx.amdgpu_device);
+			igt_require(err == 0);
+			igt_info("  AMDGPU version: %d.%d\n", major, minor);
+		}
+#endif
+
+		if (ctx.driver == DRIVER_TYPE_I915) {
+			igt_require_gem(ctx.fd);
+			igt_info("  i915 GEM verified\n");
+		}
+
+		igt_info("═══════════════════════════════════════════════\n\n");
+	}
+
+	igt_describe("Race Condition #1: CHANGE_HANDLE vs GEM_CLOSE (concurrent access issues)");
+	igt_subtest("race-change-vs-close")
+		test_race_change_vs_close(&ctx);
+
+	igt_describe("Race Condition #2: Concurrent CHANGE_HANDLE (handle corruption)");
+	igt_subtest("race-change-vs-change")
+		test_race_change_vs_change(&ctx);
+
+	igt_describe("Race Condition #3: CHANGE_HANDLE vs Prime (stale references)");
+	igt_subtest("race-change-vs-prime")
+		test_race_change_vs_prime(&ctx);
+	igt_describe("Aggressive concurrent access race: CHANGE_HANDLE vs CLOSE with KASAN detection");
+	igt_subtest("race-aggressive-change-vs-close")
+		test_race_aggressive(&ctx);
+	igt_describe("Single-thread: One BO swapped H<->H+1 with periodic FD close");
+	igt_subtest("race-exploit-single-thread")
+		test_race_single_thread(&ctx);
+	igt_describe("Random handle probing: Try random new_handle values to find free slots");
+	igt_subtest("race-exploit-random-handles")
+		test_race_random_handles(&ctx);
+	igt_describe(" GEM_CLOSE races CHANGE_HANDLE lookup ");
+	igt_subtest("race-close-before-lock")
+		test_race_close_before_lock(&ctx);
+
+	igt_describe("Edge case: handle == new_handle should be a noop");
+	igt_subtest("noop-same-handle")
+		test_noop_same_handle(&ctx);
+
+	igt_describe("Edge case: new_handle > INT_MAX must return -EINVAL");
+	igt_subtest("invalid-new-handle-exceeds-int-max")
+		test_new_handle_exceeds_int_max(&ctx);
+
+	igt_describe("Edge case: non-existent handle must return -ENOENT");
+	igt_subtest("invalid-handle-nonexistent")
+		test_invalid_handle(&ctx);
+
+	igt_describe("Edge case: new_handle == 0 (unhandled case reported to Dave)");
+	igt_subtest("edge-new-handle-zero")
+		test_new_handle_zero(&ctx);
+
+	igt_describe("Functional: verify rename moves object from old to new handle");
+	igt_subtest("functional-rename-verification")
+		test_functional_rename(&ctx);
+
+	igt_fixture() {
+#if HAS_AMDGPU
+		if (ctx.driver == DRIVER_TYPE_AMDGPU && ctx.amdgpu_device)
+			amdgpu_device_deinitialize(ctx.amdgpu_device);
+#endif
+		drm_close_driver(ctx.fd);
+	}
+}
-- 
2.54.0