[2/3] drm: simplify authentication management
diff mbox

Message ID CANq1E4T4ii3M2nZtzurpT6bw2Oh8CexJZmTKXGZw04qcOU=VRA@mail.gmail.com
State New
Headers show

Commit Message

David Herrmann May 4, 2015, 6:21 p.m. UTC
Hi

On Mon, May 4, 2015 at 6:49 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, May 04, 2015 at 06:28:25PM +0200, David Herrmann wrote:
>> I'm not sure how to write a benchmark for this. I mean, I can easily
>> craft one that causes the IDR to degenerate, but it requires us to
>> keep huge numbers of files open.
>> But this fact makes IDR rather suboptimal for cyclic allocations, so I
>> switched to idr_alloc() now. This guarantees tight/packed ID ranges
>> and user-space cannot degenerate the layers, anymore. That is, unless
>> you open more than 256 FDs on a device in parallel, we're fine with a
>> single IDR layer; always. This should work fine, right?
>
> That pretty much circumvents my only worry! If there is a client leak,
> the system will eventually keel under the load, and that we have a huge
> number of magics open is insignificant.
>
> As far as test coverage I would focus on
>
> (a) authenticating up to vfs-file-max fds (i.e. check that we hit
> the system limits without authmagic failing)
>
> (b) churn through a small number of clients for a few minutes, just
> basically checking for anomalous behaviour and that allocation times
> every minute or so remain constant.
>
> (c) just check that we can authenticate! always useful for patches that
> touch the authmagic system
>
> I was thinking of a few more, but they basically serve to show the holes
> in the authmagic scheme.

Attached is an i-g-t patch to test for basic drm-auth/magic behavior.
Comments welcome!

I also sent v2 of this patch seconds ago.

Thanks
David

Patch
diff mbox

From 96feb83e95b2c533698ace24fe3cd25fbf114b92 Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Mon, 4 May 2015 20:15:54 +0200
Subject: [PATCH] tests: add drm_auth tests for generic DRM-auth-magic testing

This adds tests/drm_auth.c which tests for drmGetMagic() and
drmAuthMagic() deficiencies.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 tests/.gitignore       |   1 +
 tests/Makefile.sources |   1 +
 tests/drm_auth.c       | 163 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 165 insertions(+)
 create mode 100644 tests/drm_auth.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 796e330..50bf3eb 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -3,6 +3,7 @@  core_get_client_auth
 core_getclient
 core_getstats
 core_getversion
+drm_auth
 drm_import_export
 drm_read
 drm_vma_limiter
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 4cbc50d..2797a0f 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -91,6 +91,7 @@  TESTS_progs = \
 	core_getclient \
 	core_getstats \
 	core_getversion \
+	drm_auth \
 	drm_import_export \
 	drm_read \
 	drm_vma_limiter \
diff --git a/tests/drm_auth.c b/tests/drm_auth.c
new file mode 100644
index 0000000..3a97d68
--- /dev/null
+++ b/tests/drm_auth.c
@@ -0,0 +1,163 @@ 
+/*
+ * Copyright 2015 David Herrmann <dh.herrmann@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/*
+ * Testcase: drmGetMagic() and drmAuthMagic()
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <signal.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include <sys/poll.h>
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+#include "igt_aux.h"
+
+IGT_TEST_DESCRIPTION("Call drmGetMagic() and drmAuthMagic() and see if it behaves.");
+
+static int magic_cmp(const void *p1, const void *p2)
+{
+	return *(const drm_magic_t*)p1 < *(const drm_magic_t*)p2;
+}
+
+static void test_many_magics(int master)
+{
+	drm_magic_t magic, *magics = NULL;
+	unsigned int i, j, ns, allocated = 0;
+	char path[512];
+	int *fds = NULL, slave;
+
+	sprintf(path, "/proc/self/fd/%d", master);
+
+	for (i = 0; ; ++i) {
+		/* open slave and make sure it's NOT a master */
+		slave = open(path, O_RDWR | O_CLOEXEC);
+		if (slave < 0) {
+			igt_assert(errno == EMFILE);
+			break;
+		}
+		igt_assert(drmSetMaster(slave) < 0);
+
+		/* resize magic-map */
+		if (i >= allocated) {
+			ns = allocated * 2;
+			igt_assert(ns >= allocated);
+
+			if (!ns)
+				ns = 128;
+
+			magics = realloc(magics, sizeof(*magics) * ns);
+			igt_assert(magics);
+
+			fds = realloc(fds, sizeof(*fds) * ns);
+			igt_assert(fds);
+
+			allocated = ns;
+		}
+
+		/* insert magic */
+		igt_assert(drmGetMagic(slave, &magic) == 0);
+		igt_assert(magic > 0);
+
+		magics[i] = magic;
+		fds[i] = slave;
+	}
+
+	/* make sure we could at least open a reasonable number of files */
+	igt_assert(i > 128);
+
+	/*
+	 * We cannot open the DRM file anymore. Lets sort the magic-map and
+	 * verify no magic was used multiple times.
+	 */
+	qsort(magics, i, sizeof(*magics), magic_cmp);
+	for (j = 1; j < i; ++j)
+		igt_assert(magics[j] != magics[j - 1]);
+
+	/* make sure we can authenticate all of them */
+	for (j = 0; j < i; ++j)
+		igt_assert(drmAuthMagic(master, magics[j]) == 0);
+
+	/* close files again */
+	for (j = 0; j < i; ++j)
+		close(fds[j]);
+
+	free(fds);
+	free(magics);
+}
+
+static void test_basic_auth(int master)
+{
+	drm_magic_t magic, old_magic;
+	int slave;
+
+	/* open slave and make sure it's NOT a master */
+	slave = drm_open_any();
+	igt_require(slave >= 0);
+	igt_require(drmSetMaster(slave) < 0);
+
+	/* retrieve magic for slave */
+	igt_assert(drmGetMagic(slave, &magic) == 0);
+	igt_assert(magic > 0);
+
+	/* verify the same magic is returned every time */
+	old_magic = magic;
+	igt_assert(drmGetMagic(slave, &magic) == 0);
+	igt_assert_eq(magic, old_magic);
+
+	/* verify magic can be authorized exactly once, on the master */
+	igt_assert(drmAuthMagic(slave, magic) < 0);
+	igt_assert(drmAuthMagic(master, magic) == 0);
+	igt_assert(drmAuthMagic(master, magic) < 0);
+
+	/* verify that the magic did not change */
+	old_magic = magic;
+	igt_assert(drmGetMagic(slave, &magic) == 0);
+	igt_assert_eq(magic, old_magic);
+
+	close(slave);
+}
+
+igt_main
+{
+	int master;
+
+	igt_fixture
+		master = drm_open_any_master();
+
+	igt_subtest("basic-auth")
+		test_basic_auth(master);
+
+	igt_subtest("many-magics")
+		test_many_magics(master);
+}
-- 
2.3.7