[igt-dev] [PATCH i-g-t] tests/core_unauth_vs_render: new test for the relaxed DRM_AUTH handling

* [igt-dev] [PATCH i-g-t] tests/core_unauth_vs_render: new test for the relaxed DRM_AUTH handling
@ 2019-02-08 18:03 Emil Velikov
  2019-02-08 18:47 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/core_unauth_vs_render: new test for the relaxed DRM_AUTH handling (rev3) Patchwork
  0 siblings, 1 reply; 2+ messages in thread
From: Emil Velikov @ 2019-02-08 18:03 UTC (permalink / raw)
  To: igt-dev; +Cc: emil.l.velikov

From: Emil Velikov <emil.velikov@collabora.com>

As the inline comment says, this test checks that the kernel allows
unauthenticated master with render capable, RENDER_ALLOW ioctls.

The kernel commit has extra details why.

v2:
 - drop RUN_AS_ROOT guard
 - call check_auth() on the {,un}authenticated device
 - check the device is PRIME (import) capable
 - check the device has render node
 - tweak expectations based on above three
 - elaborate why we care only about -EACCES

v3:
 - fold into existing core_auth.c
 - move igt_assert within the subtest
 - make has_prime_import() an igt_require()
 - check for BADF before and after, as requested. Not strictly needed.
 - swap igt_info+drm_open_driver with comment + __drm_open_driver
Former calls igt_skip() which is problematic with igt_fork().

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
Hope that covers everything Dan. Alternatively I would prefer to keep
polishing this, while the kernel fix is merged. This way existing users
would work :-)
---
 tests/core_auth.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/tests/core_auth.c b/tests/core_auth.c
index 0c016a37..0b9073cb 100644
--- a/tests/core_auth.c
+++ b/tests/core_auth.c
@@ -1,5 +1,6 @@
 /*
  * Copyright 2015 David Herrmann <dh.herrmann@gmail.com>
+ * Copyright 2018 Collabora, Ltd
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -40,6 +41,7 @@
 #include <sys/time.h>
 #include <sys/poll.h>
 #include <sys/resource.h>
+#include <sys/sysmacros.h>
 #include "drm.h"
 
 #ifdef __linux__
@@ -190,6 +192,98 @@ static void test_basic_auth(int master)
 	close(slave);
 }
 
+static bool has_prime_import(int fd)
+{
+	uint64_t value;
+
+	if (drmGetCap(fd, DRM_CAP_PRIME, &value))
+		return false;
+
+	return value & DRM_PRIME_CAP_IMPORT;
+}
+
+static void check_auth_sanity(int master)
+{
+	uint32_t handle;
+
+	igt_assert(check_auth(master) == true);
+	igt_require(has_prime_import(master));
+
+	igt_assert(drmPrimeFDToHandle(master, -1, &handle) < 0);
+
+	/* IOCTL requires authenticated master as done in drm_permit.
+	 * As we get past that, we'll fail due to the invalid FD.
+	 *
+	 * Note: strictly speaking this is unrelated to the goal of
+	 * the test, although danvet requested it.
+	 */
+	igt_assert(errno == EBADF);
+}
+
+static bool has_render_node(int fd)
+{
+	char node_name[80];
+	struct stat sbuf;
+
+	if (fstat(fd, &sbuf))
+		return false;
+
+	sprintf(node_name, "/dev/dri/renderD%d", minor(sbuf.st_rdev) | 0x80);
+	if (stat(node_name, &sbuf))
+		return false;
+
+	return true;
+}
+
+/*
+ * Testcase: Render capable, unauthenticated master doesn't throw -EACCES for
+ * DRM_RENDER_ALLOW ioctls.
+ */
+static void test_unauth_vs_render(int master)
+{
+	int slave;
+	uint32_t handle;
+
+	/*
+	 * FIXME: when drm_open_driver() fails to open() a node (insufficient
+	 * permissions or otherwise, it will igt_skip.
+	 * As of today, igt_skip and igt_fork do not work together.
+	 */
+	slave = __drm_open_driver(DRIVER_ANY);
+	/*
+	 * FIXME: relate to the master fd passed with the above open and fix
+	 * all of IGT.
+	 */
+
+	igt_assert(slave >= 0);
+
+	/*
+	 * The second open() happens without CAP_SYS_ADMIN, thus it will NOT
+	 * be authenticated.
+	 */
+	igt_assert(check_auth(slave) == false);
+
+	/* Issuing the following ioctl will fail, no doubt about it. */
+	igt_assert(drmPrimeFDToHandle(slave, -1, &handle) < 0);
+
+	/*
+	 * Updated kernels allow render capable, unauthenticated master to
+	 * issue DRM_AUTH ioctls (like FD2HANDLE above), as long as they are
+	 * annotated as DRM_RENDER_ALLOW.
+	 *
+	 * Otherwise, errno is set to -EACCES
+	 *
+	 * Note: We are _not_ interested in the FD2HANDLE specific errno,
+	 * yet the EBADF check is added on the explicit request by danvet.
+	 */
+	if (has_render_node(slave))
+		igt_assert(errno == EBADF);
+	else
+		igt_assert(errno == EACCES);
+
+	close(slave);
+}
+
 igt_main
 {
 	int master;
@@ -228,4 +322,19 @@ igt_main
 		igt_subtest("many-magics")
 			test_many_magics(master);
 	}
+
+	igt_subtest_group {
+		igt_fixture
+			master = drm_open_driver(DRIVER_ANY);
+
+		igt_subtest("unauth-vs-render") {
+			check_auth_sanity(master);
+
+			igt_fork(child, 1) {
+				igt_drop_root();
+				test_unauth_vs_render(master);
+			}
+			igt_waitchildren();
+		}
+	}
 }
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 2+ messages in thread