From patchwork Mon May 4 18:21:22 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 6329421 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 40F7DBEEE1 for ; Mon, 4 May 2015 18:21:27 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F23CE202A1 for ; Mon, 4 May 2015 18:21:25 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id B789D202DD for ; Mon, 4 May 2015 18:21:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 139496E037; Mon, 4 May 2015 11:21:24 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-qc0-f176.google.com (mail-qc0-f176.google.com [209.85.216.176]) by gabe.freedesktop.org (Postfix) with ESMTP id 623D26E037 for ; Mon, 4 May 2015 11:21:23 -0700 (PDT) Received: by qcbgy10 with SMTP id gy10so23297354qcb.3 for ; Mon, 04 May 2015 11:21:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=dNRD56vq/t/167rsZf4CxC7XpbmRj191x8aPHUvRCKQ=; b=Zfv3XsuqAa1Eby/6nTCcz6TdyyNTLkDQSH+bqM6X0cD+Hcp9R45MuwmgLObdQZFZoT qVP4SgHIhtzrFVYz7y/HEwlfxuuWOToHUpuHoVV7ENxFWI/W5Gb+reyyrh8pzbxBy8y6 wDoHqOf7ZegrzxpQYjtWn+AnZBkcbvFJWEudcr9iENa0xZOT5OXs52jOWV0PwXyfIOqy SYF8msWc19ltunpQpwIuLNv2JmJWngJda4Nl2981MTNHXb1I4DsuHMsqqGkvDU5rnKXm RzHgfG9t5b8BsRqxsSsewDxw3X0f3qnmhmDitIga2Fh+d5UyBTn0ruJxFvL+MWWj7g1g CVBQ== MIME-Version: 1.0 X-Received: by 10.140.101.81 with SMTP id t75mr28721923qge.9.1430763682910; Mon, 04 May 2015 11:21:22 -0700 (PDT) Received: by 10.140.166.133 with HTTP; Mon, 4 May 2015 11:21:22 -0700 (PDT) In-Reply-To: <20150504164929.GK22099@nuc-i3427.alporthouse.com> References: <1430748314-862-1-git-send-email-dh.herrmann@gmail.com> <1430748314-862-2-git-send-email-dh.herrmann@gmail.com> <20150504144653.GD22099@nuc-i3427.alporthouse.com> <20150504151104.GH22099@nuc-i3427.alporthouse.com> <20150504152544.GJ22099@nuc-i3427.alporthouse.com> <20150504164929.GK22099@nuc-i3427.alporthouse.com> Date: Mon, 4 May 2015 20:21:22 +0200 Message-ID: Subject: Re: [PATCH 2/3] drm: simplify authentication management From: David Herrmann To: Chris Wilson , David Herrmann , "dri-devel@lists.freedesktop.org" , Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi On Mon, May 4, 2015 at 6:49 PM, Chris Wilson 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 From 96feb83e95b2c533698ace24fe3cd25fbf114b92 Mon Sep 17 00:00:00 2001 From: David Herrmann 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 --- 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 + * + * 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#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