[igt-dev] [PATCH i-g-t v2] igt/gem_exec_params: Drop drm master privileges only on drm master fds

[Chris Wilson]

Quoting Antonio Argenziano (2018-01-31 21:21:32)
> Subtest 'secure-non-master' tries to drop drm-master privileges on a
> non drm-master fd. Instead drop master privileges after the subtest
> acquires it so that other subtests are not affected by it.
> 
> v2:
>         - Require root privilege for the sub-test. (Chris)
> 
> Signed-off-by: Antonio Argenziano <antonio.argenziano at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  tests/gem_exec_params.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c
> index e1c48b40..2f9227e3 100644
> --- a/tests/gem_exec_params.c
> +++ b/tests/gem_exec_params.c
> @@ -343,13 +343,18 @@ igt_main
>         }
>  
>         igt_subtest("secure-non-master") {
> +               igt_require(__igt_device_set_master(fd)); /* Requires root privilege */
>                 igt_device_drop_master(fd);
> +
>                 execbuf.flags = I915_EXEC_RENDER | I915_EXEC_SECURE;
>                 RUN_FAIL(EPERM);

Fwiw, I like the spacing around the second stanza better as that couples
acquiring root with the successful test. So I'd like the
igt_device_drop_master() next to the RUN_FAIL block.

igt_assert_eq(__gem_execbuf(fd, &execbuf), -EPERM);

> +
>                 igt_device_set_master(fd);
>                 igt_assert(drmIoctl(fd,
>                                     DRM_IOCTL_I915_GEM_EXECBUFFER2,
>                                     &execbuf) == 0);
gem_execbuf(fd, &execbuf);

> +
> +               igt_device_drop_master(fd); /* Only needs temporary master */
>         }
>  
>         /* HANDLE_LUT and NO_RELOC are already exercised by gem_exec_lut_handle,

Preferably with the whitespace tweak, and with or without the migration
to gem_execbuf/__gem_execbuf,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris