[igt-dev] [PATCH i-g-t] tests/core_unauth_vs_render: new test for the relaxed DRM_AUTH handling
Daniel Vetter
daniel at ffwll.ch
Thu Feb 7 14:17:49 UTC 2019
On Wed, Feb 06, 2019 at 01:18:28PM +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at 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
>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> tests/core_unauth_vs_render.c | 182 ++++++++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 2 files changed, 183 insertions(+)
> create mode 100644 tests/core_unauth_vs_render.c
>
> diff --git a/tests/core_unauth_vs_render.c b/tests/core_unauth_vs_render.c
> new file mode 100644
> index 00000000..82dd2ce9
> --- /dev/null
> +++ b/tests/core_unauth_vs_render.c
> @@ -0,0 +1,182 @@
> +/*
> + * 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"),
> + * 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.
> + *
> + * Authors:
> + * Emil Velikov <emil.velikov at collabora.com>
> + */
> +
> +/*
> + * Testcase: Render capable, unauthenticated master doesn't throw -EACCES for
> + * DRM_RENDER_ALLOW ioctls.
> + */
> +
> +#include "igt.h"
> +#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 <sys/resource.h>
> +#include <sys/sysmacros.h>
> +#include "drm.h"
> +
> +#ifdef __linux__
> +# include <sys/syscall.h>
> +#else
> +# include <pthread.h>
> +#endif
> +
> +/* Checks whether the thread id is the current thread */
> +static bool
> +is_local_tid(pid_t tid)
> +{
> +#ifndef __linux__
> + return pthread_self() == tid;
> +#else
> + /* On Linux systems, drmGetClient() would return the thread ID instead
> + of the actual process ID */
> + return syscall(SYS_gettid) == tid;
> +#endif
> +}
> +
> +
> +static bool check_auth(int fd)
> +{
> + pid_t client_pid;
> + int i, auth, pid, uid;
> + unsigned long magic, iocs;
> + bool is_authenticated = false;
> +
> + client_pid = getpid();
> + for (i = 0; !is_authenticated; i++) {
> + if (drmGetClient(fd, i, &auth, &pid, &uid, &magic, &iocs) != 0)
> + break;
> + is_authenticated = auth && (pid == client_pid || is_local_tid(pid));
> + }
> + return is_authenticated;
> +}
btw the core_auth merger landed, so you can stuff your new subtest in
there now.
> +
> +
> +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 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;
> +}
> +
> +IGT_TEST_DESCRIPTION("Call drmPrimeFDToHandle() from unauthenticated master doesn't return -EACCES.");
> +
> +static void test_unauth_vs_render(int master)
> +{
> + int slave;
> + int prime_fd = -1;
> + uint32_t handle;
> +
> + /*
> + * The second open() happens without CAP_SYS_ADMIN, thus it will NOT
> + * be authenticated.
> + */
> + igt_info("Openning card node from a non-priv. user.\n");
> + igt_info("On failure, double-check the node permissions\n");
> + /* FIXME: relate to the master given and fix all of IGT */
> + slave = drm_open_driver(DRIVER_ANY);
> +
> + igt_require(slave >= 0);
igt_require/skip need to be outside of igt_fork. But this one here should
be an igt_assert I think, and the testcase needs to somehow make sure that
it will succeed. I think the namespace trick is probably the best option,
but that means open-coding the clone stuff, or rewriting igt_fork. I think
I need to look into that a bit ...
> + igt_assert(check_auth(slave) == false);
> +
> + /* Issuing the following ioctl will fail, no doubt about it. */
> + igt_assert(drmPrimeFDToHandle(slave, prime_fd, &handle) < 0);
Hm, I'd run this first on master and make sure we get -EBADF as errno.
Just to make sure the ioctl call we're doing does get through the
drm_ioctl() layers.
> +
> + /*
> + * Updated kernels allow render capable, unauthenticated master to
> + * issue DRM_AUTH ioctls (like the above), as long as they are
> + * annotated as DRM_RENDER_ALLOW - just like FD2HANDLE above.
> + *
> + * Otherwise, errno is set to -EACCES
> + *
> + * Note: We are _not_ interested in the FD2HANDLE specific errno. Those
> + * should be checked other standalone tests.
> + */
> + bool imp = has_prime_import(slave);
Hm I think has_prime_import should be an igt_require (and outside of the
igt_fork).
> + bool rend = has_render_node(slave);
> + igt_info("import %d rend %d\n", imp, rend);
> + if (has_prime_import(slave) && has_render_node(slave))
> + igt_assert(errno != EACCES);
Still think we should check for the errno we expect here (i.e. EBADF, if
we filter out !has_prime_import earlier).
-Daniel
> +
> + else
> + igt_assert(errno == EACCES);
> +
> + close(slave);
> +}
> +
> +/*
> + * IGT is executed as root, although that may(?) change in the future.
> + * Thus we need to drop the privileges so that the second open() results in a
> + * client which is not unauthenticated. Running as normal user circumvents that.
> + *
> + * In both cases, we need to ensure the file permissions of the node are
> + * sufficient.
> + */
> +
> +igt_main
> +{
> + int master;
> +
> + igt_fixture
> + master = drm_open_driver(DRIVER_ANY);
> +
> + igt_assert(check_auth(master) == true);
> +
> + igt_subtest("unauth-vs-render") {
> + igt_fork(child, 1) {
> + igt_drop_root();
> + test_unauth_vs_render(master);
> + }
> + igt_waitchildren();
> + }
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 0f12df26..e5200b36 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -1,5 +1,6 @@
> test_progs = [
> 'core_auth',
> + 'core_unauth_vs_render',
> 'core_getclient',
> 'core_getstats',
> 'core_getversion',
> --
> 2.20.1
>
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the igt-dev
mailing list