[Intel-gfx] [PATCH i-g-t] tests/drm_hw_lock: Tests for hw_lock fixes.

Thomas Wood thomas.wood at intel.com
Mon Apr 27 08:24:37 PDT 2015


On 23 April 2015 at 15:07, Peter Antoine <peter.antoine at intel.com> wrote:
> There are several issues with the hardware locks functions that stretch
> from kernel crashes to priority escalations. This new test will test the
> the fixes for these features.
>
> This test will cause a driver/kernel crash on un-patched kernels, the
> following patches should be applied to stop the crashes:
>
>   drm: Kernel Crash in drm_unlock
>   drm: Fixes unsafe deference in locks.
>
> Issue: VIZ-5485
> Signed-off-by: Peter Antoine <peter.antoine at intel.com>
> ---
>  lib/ioctl_wrappers.c   |  19 +++++
>  lib/ioctl_wrappers.h   |   1 +
>  tests/Makefile.sources |   1 +
>  tests/drm_hw_lock.c    | 207 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 228 insertions(+)
>  create mode 100644 tests/drm_hw_lock.c
>
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 000d394..ad8b3d3 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -964,6 +964,25 @@ bool gem_has_bsd2(int fd)
>  {
>         return gem_has_enable_ring(fd,LOCAL_I915_PARAM_HAS_BSD2);
>  }
> +#define I915_PARAM_HAS_LEGACY_CONTEXT   35


Please add some API documentation for this new function here.

> +bool drm_has_legacy_context(int fd)
> +{
> +       int tmp = 0;
> +       drm_i915_getparam_t gp;
> +
> +       memset(&gp, 0, sizeof(gp));
> +       gp.value = &tmp;
> +       gp.param = I915_PARAM_HAS_LEGACY_CONTEXT;
> +
> +       /*
> +        * if legacy context param is not supported, then it's old and we
> +        * can assume that the HW_LOCKS are supported.
> +        */
> +       if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0)
> +               return true;
> +
> +       return tmp == 1;
> +}
>  /**
>   * gem_available_aperture_size:
>   * @fd: open i915 drm file descriptor
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index ced7ef3..3adc700 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -120,6 +120,7 @@ bool gem_has_bsd(int fd);
>  bool gem_has_blt(int fd);
>  bool gem_has_vebox(int fd);
>  bool gem_has_bsd2(int fd);
> +bool drm_has_legacy_context(int fd);
>  bool gem_uses_aliasing_ppgtt(int fd);
>  int gem_available_fences(int fd);
>  uint64_t gem_available_aperture_size(int fd);
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 71de6de..2f69afc 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -84,6 +84,7 @@ TESTS_progs_M = \
>         pm_sseu \
>         prime_self_import \
>         template \
> +       drm_hw_lock \

Please also add the binary name to .gitignore.

>         $(NULL)
>
>  TESTS_progs = \
> diff --git a/tests/drm_hw_lock.c b/tests/drm_hw_lock.c
> new file mode 100644
> index 0000000..aad50ba
> --- /dev/null
> +++ b/tests/drm_hw_lock.c
> @@ -0,0 +1,207 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * 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:
> + *    Peter Antoine <peter.antoine at intel.com>
> + */
> +
> +/*
> + * Testcase: Test the HW_LOCKs for correct support and non-crashing.

This would be a good sentence to use in the IGT_TEST_DESCRIPTION
macro, to add a description for the test.


> + *
> + * This test will check that he hw_locks are only g_supported for drivers that
> + * require that support. If it is not g_supported then the functions all return
> + * the correct error code.
> + *
> + * If g_supported it will check that the functions do not crash when the crash
> + * tests are used, also that one of the tests is a security level escalation
> + * that should be rejected.
> + */
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <sys/ioctl.h>
> +#include <drm.h>
> +#include "drmtest.h"
> +#include "igt_core.h"
> +#include "ioctl_wrappers.h"
> +
> +#ifndef DRM_KERNEL_CONTEXT
> +#      define DRM_KERNEL_CONTEXT       (0)
> +#endif
> +
> +#ifndef _DRM_LOCK_HELD
> +#      define _DRM_LOCK_HELD   0x80000000U /**< Hardware lock is held */
> +#endif
> +
> +#ifndef _DRM_LOCK_CONT
> +#      define _DRM_LOCK_CONT   0x40000000U /**< Hardware lock is contended */
> +#endif
> +
> +static bool    g_sig_fired;
> +static bool    g_supported;
> +static struct sigaction old_action;
> +
> +static void sig_term_handler(int value)
> +{
> +       g_sig_fired = true;
> +}
> +
> +static bool set_term_handler(void)
> +{
> +       static struct sigaction new_action;
> +
> +       new_action.sa_handler = sig_term_handler;
> +       sigemptyset(&new_action.sa_mask);
> +       new_action.sa_flags = 0;
> +
> +       igt_assert(sigaction(SIGTERM, &new_action, &old_action) == 0);
> +
> +       if (old_action.sa_handler != SIG_IGN)
> +               return true;
> +       else
> +               return false;
> +}
> +
> +static void restore_term_handler(void)
> +{
> +       sigaction(SIGTERM, &old_action, NULL);
> +}
> +
> +static void does_lock_crash(int fd)
> +{
> +       struct drm_lock rubbish;
> +       int ret;
> +
> +       memset(&rubbish, 'A', sizeof(struct drm_lock));
> +
> +       g_sig_fired = false;
> +       igt_assert(set_term_handler());

Since set_term_handler and restore_term_handler are called at the
start and end of every subtest, they could be placed in the respective
igt_fixture blocks before and after the subtests in igt_main.


> +
> +       ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> +
> +       igt_assert(ret == -1 && (!g_supported || g_sig_fired));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       restore_term_handler();
> +}
> +
> +static void does_unlock_crash(int fd)
> +{
> +       struct drm_lock rubbish;
> +       int ret;
> +
> +       g_sig_fired = false;
> +       igt_assert(set_term_handler());
> +
> +       memset(&rubbish, 'A', sizeof(struct drm_lock));
> +
> +       ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> +
> +       igt_assert(ret == -1 && (!g_supported || g_sig_fired));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       restore_term_handler();
> +}
> +
> +static void priority_escalation(int fd)
> +{
> +       struct drm_lock rubbish;
> +       int ret;
> +
> +       g_sig_fired = false;
> +       igt_assert(set_term_handler());

g_sig_fired is not checked in these tests, so presumably it doesn't
need to be set before each test?


> +
> +       /* this should be rejected */
> +       rubbish.context = DRM_KERNEL_CONTEXT;
> +       ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> +       igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       /* this should also be rejected */
> +       rubbish.context = _DRM_LOCK_HELD | DRM_KERNEL_CONTEXT;
> +       g_sig_fired = false;
> +       ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> +       igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       /* this should also be rejected */
> +       rubbish.context = _DRM_LOCK_CONT | DRM_KERNEL_CONTEXT;
> +       g_sig_fired = false;
> +       ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> +       igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       /* this should be rejected */
> +       rubbish.context = DRM_KERNEL_CONTEXT;
> +       g_sig_fired = false;
> +       ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> +       igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       /* this should also be rejected */
> +       rubbish.context = _DRM_LOCK_HELD | DRM_KERNEL_CONTEXT;
> +       g_sig_fired = false;
> +       ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> +       igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       /* this should also be rejected */
> +       rubbish.context = _DRM_LOCK_CONT | DRM_KERNEL_CONTEXT;
> +       g_sig_fired = false;
> +       ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> +       igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> +       igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> +       restore_term_handler();
> +}
> +
> +igt_main
> +{
> +       int fd = -1;
> +
> +       g_sig_fired = false;
> +       g_supported = false;
> +
> +       igt_fixture {
> +               fd = drm_open_any();
> +               igt_assert(fd >= 0);

drm_open_any will always return a valid file descriptor. If no device
is found, it will call igt_skip.


> +       }
> +
> +       g_supported = drm_has_legacy_context(fd);

This needs to be inside the igt_fixture block above, otherwise it may
interfere with subtest enumeration.


> +
> +       igt_info("HW LOCK Supported: %s\n", g_supported?"Yes":"No");
> +
> +       igt_subtest("lock-crash") {
> +               does_lock_crash(fd);
> +       }
> +
> +       igt_subtest("unlock-crash") {
> +               does_unlock_crash(fd);
> +       }
> +
> +       igt_subtest("priority-escalation") {
> +               priority_escalation(fd);
> +       }
> +
> +       igt_fixture
> +               close(fd);
> +}
> +
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


More information about the Intel-gfx mailing list