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

[Petri Latvala <petri.latvala@intel.com>]

On Wed, Feb 06, 2019 at 01:18:28PM +0000, Emil Velikov wrote:
> 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
> 
> Signed-off-by: Emil Velikov <emil.velikov@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@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;
> +}
> +
> +
> +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_assert(check_auth(slave) == false);
> +
> +	/* Issuing the following ioctl will fail, no doubt about it. */
> +	igt_assert(drmPrimeFDToHandle(slave, prime_fd, &handle) < 0);
> +
> +	/*
> +	 * 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);
> +	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);
> +
> +	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);


You can't use igt_assert outside of igt_fixture/igt_subtest*.


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