| Message ID | 1429798025-18940-1-git-send-email-peter.antoine@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23 April 2015 at 15:07, Peter Antoine <peter.antoine@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@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@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Apr 27, 2015 at 04:24:37PM +0100, Thomas Wood wrote: > On 23 April 2015 at 15:07, Peter Antoine <peter.antoine@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@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; Would not a simpler test be to try and legally acquire a hwlock? If it fails, hwlocks are not supported. No need for a PARAM. -Chris
Thanks for the review, new patch inbound. -----Original Message----- From: Thomas Wood [mailto:thomas.wood@intel.com] Sent: Monday, April 27, 2015 4:25 PM To: Antoine, Peter Cc: Intel Graphics Development; airlied@redhat.com; dri-devel@lists.freedesktop.org; Daniel Vetter Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/drm_hw_lock: Tests for hw_lock fixes. On 23 April 2015 at 15:07, Peter Antoine <peter.antoine@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@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@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 2015-04-27 at 16:33 +0100, Chris Wilson wrote: > On Mon, Apr 27, 2015 at 04:24:37PM +0100, Thomas Wood wrote: > > On 23 April 2015 at 15:07, Peter Antoine <peter.antoine@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@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; > > Would not a simpler test be to try and legally acquire a hwlock? > If it fails, hwlocks are not supported. No need for a PARAM. > -Chris > The test relies on the hw_lock not being created (f the HW-LOCKs are supported). If I grab, then delete the lock I might find more problems with this code. :) As you have to be root to create and delete the hwlock the security issues are minimal as you have root already you don't really need to use these ioctls to harm the system. So as the exposure is minimal, any more fixes on code that is legacy and being turned off seems to be a bit of a time waste. Also, the PARAM should give legitimate code the opportunity to avoid the problems but avoiding these features completely. Peter.
On Tue, Apr 28, 2015 at 09:28:51AM +0000, Antoine, Peter wrote: > Also, the PARAM should give legitimate code the opportunity to avoid the > problems but avoiding these features completely. There are no legitimate users. -Chris
On Tue, Apr 28, 2015 at 09:28:51AM +0000, Antoine, Peter wrote: > On Mon, 2015-04-27 at 16:33 +0100, Chris Wilson wrote: > > On Mon, Apr 27, 2015 at 04:24:37PM +0100, Thomas Wood wrote: > > > On 23 April 2015 at 15:07, Peter Antoine <peter.antoine@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@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; > > > > Would not a simpler test be to try and legally acquire a hwlock? > > If it fails, hwlocks are not supported. No need for a PARAM. > > -Chris > > > > The test relies on the hw_lock not being created (f the HW-LOCKs are > supported). If I grab, then delete the lock I might find more problems > with this code. :) > > As you have to be root to create and delete the hwlock the security > issues are minimal as you have root already you don't really need to use > these ioctls to harm the system. So as the exposure is minimal, any more > fixes on code that is legacy and being turned off seems to be a bit of a > time waste. > > Also, the PARAM should give legitimate code the opportunity to avoid the > problems but avoiding these features completely. root != kernel, at least in secure boot enviroments. Which means not even root is allowed to crash/exploit the kernel ... Yes there's a pile of .config options and stuff you need to set correctly to fully lock out root from the kernel, but in general drivers really shouldn't have their own root2kernel backdoors. -Daniel
Please ignore this test as fixes are being implemented differently. On Thu, 2015-04-23 at 15:07 +0100, Peter Antoine 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@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 > +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 \ > $(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@intel.com> > + */ > + > +/* > + * Testcase: Test the HW_LOCKs for correct support and non-crashing. > + * > + * 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()); > + > + 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()); > + > + /* 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); > + } > + > + g_supported = drm_has_legacy_context(fd); > + > + 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); > +} > +
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 +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 \ $(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@intel.com> + */ + +/* + * Testcase: Test the HW_LOCKs for correct support and non-crashing. + * + * 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()); + + 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()); + + /* 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); + } + + g_supported = drm_has_legacy_context(fd); + + 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); +} +
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@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