All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm: annotate drm_core_check_feature() dev arg. as const
@ 2019-01-14  8:54 Emil Velikov
  2019-01-14  8:54 ` [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls Emil Velikov
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Emil Velikov @ 2019-01-14  8:54 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, intel-gfx

From: Emil Velikov <emil.velikov@collabora.com>

This static inline function doesn't modify any state.

Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/drm/drm_drv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 3199ef70c007..211d2bfd0b94 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -658,7 +658,7 @@ static inline bool drm_dev_is_unplugged(struct drm_device *dev)
  *
  * Returns true if the @feature is supported, false otherwise.
  */
-static inline bool drm_core_check_feature(struct drm_device *dev, u32 feature)
+static inline bool drm_core_check_feature(const struct drm_device *dev, u32 feature)
 {
 	return dev->driver->driver_features & dev->driver_features & feature;
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls
  2019-01-14  8:54 [PATCH v2 1/2] drm: annotate drm_core_check_feature() dev arg. as const Emil Velikov
@ 2019-01-14  8:54 ` Emil Velikov
  2019-01-14 10:15   ` Daniel Vetter
  2019-01-14 11:39 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Emil Velikov @ 2019-01-14  8:54 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx, emil.l.velikov

From: Emil Velikov <emil.velikov@collabora.com>

There are cases (in mesa and applications) where one would open the
primary node without properly authenticating the client.

Sometimes we don't check if the authentication succeeds, but there's
also cases we simply forget to do it.

The former was a case for Mesa where it did not not check the return
value of drmGetMagic() [1]. That was fixed recently although, there's
the question of older drivers or other apps that exbibit this behaviour.

While omitting the call results in issues as seen in [2] and [3].

In the libva case, libva itself doesn't authenticate the DRM client and
the vaGetDisplayDRM documentation doesn't mention if the app should
either.

As of today, the official vainfo utility doesn't authenticate.

To workaround issues like these, some users resort to running their apps
under sudo. Which admittedly isn't always a good idea.

Since any DRIVER_RENDER driver has sufficient isolation between clients,
we can use that, for unauthenticated [primary node] ioctls that require
DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW.

v2:
- Rework/simplify if check (Daniel V)
- Add examples to commit messages, elaborate. (Daniel V)

[1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136
[2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html
[3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1
Testcase: igt/core_unauth_vs_render
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
Daniel, the if conditionals did not work exactly as you put them.
This is the closest thing that I can think of.
---
 drivers/gpu/drm/drm_ioctl.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 94bd872d56c4..08a0b4cc3a76 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -507,6 +507,13 @@ int drm_version(struct drm_device *dev, void *data,
 	return err;
 }
 
+static inline bool
+drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags)
+{
+	return drm_core_check_feature(dev, DRIVER_RENDER) &&
+		(flags & DRM_RENDER_ALLOW);
+}
+
 /**
  * drm_ioctl_permit - Check ioctl permissions against caller
  *
@@ -521,14 +528,19 @@ int drm_version(struct drm_device *dev, void *data,
  */
 int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 {
+	const struct drm_device *dev = file_priv->minor->dev;
+
 	/* ROOT_ONLY is only for CAP_SYS_ADMIN */
 	if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)))
 		return -EACCES;
 
-	/* AUTH is only for authenticated or render client */
-	if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) &&
-		     !file_priv->authenticated))
-		return -EACCES;
+	/* AUTH is only for master ... */
+	if ((flags & DRM_AUTH) && drm_is_primary_client(file_priv)) {
+		/* authenticated ones, or render capable on DRM_RENDER_ALLOW. */
+		if (unlikely(!file_priv->authenticated) &&
+		    unlikely(!drm_render_driver_and_ioctl(dev, flags)))
+			return -EACCES;
+	}
 
 	/* MASTER is only for master or control clients */
 	if (unlikely((flags & DRM_MASTER) &&
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls
  2019-01-14  8:54 ` [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls Emil Velikov
@ 2019-01-14 10:15   ` Daniel Vetter
  2019-01-16 11:58     ` Emil Velikov
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2019-01-14 10:15 UTC (permalink / raw)
  To: Emil Velikov; +Cc: intel-gfx, dri-devel

On Mon, Jan 14, 2019 at 08:54:08AM +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
> 
> There are cases (in mesa and applications) where one would open the
> primary node without properly authenticating the client.
> 
> Sometimes we don't check if the authentication succeeds, but there's
> also cases we simply forget to do it.
> 
> The former was a case for Mesa where it did not not check the return
> value of drmGetMagic() [1]. That was fixed recently although, there's
> the question of older drivers or other apps that exbibit this behaviour.
> 
> While omitting the call results in issues as seen in [2] and [3].
> 
> In the libva case, libva itself doesn't authenticate the DRM client and
> the vaGetDisplayDRM documentation doesn't mention if the app should
> either.
> 
> As of today, the official vainfo utility doesn't authenticate.
> 
> To workaround issues like these, some users resort to running their apps
> under sudo. Which admittedly isn't always a good idea.
> 
> Since any DRIVER_RENDER driver has sufficient isolation between clients,
> we can use that, for unauthenticated [primary node] ioctls that require
> DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW.
> 
> v2:
> - Rework/simplify if check (Daniel V)
> - Add examples to commit messages, elaborate. (Daniel V)
> 
> [1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136
> [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html
> [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1
> Testcase: igt/core_unauth_vs_render
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Daniel, the if conditionals did not work exactly as you put them.
> This is the closest thing that I can think of.
> ---
>  drivers/gpu/drm/drm_ioctl.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 94bd872d56c4..08a0b4cc3a76 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -507,6 +507,13 @@ int drm_version(struct drm_device *dev, void *data,
>  	return err;
>  }
>  
> +static inline bool
> +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags)
> +{
> +	return drm_core_check_feature(dev, DRIVER_RENDER) &&
> +		(flags & DRM_RENDER_ALLOW);
> +}
> +
>  /**
>   * drm_ioctl_permit - Check ioctl permissions against caller
>   *
> @@ -521,14 +528,19 @@ int drm_version(struct drm_device *dev, void *data,
>   */
>  int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
>  {
> +	const struct drm_device *dev = file_priv->minor->dev;
> +
>  	/* ROOT_ONLY is only for CAP_SYS_ADMIN */
>  	if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)))
>  		return -EACCES;
>  
> -	/* AUTH is only for authenticated or render client */
> -	if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) &&
> -		     !file_priv->authenticated))
> -		return -EACCES;
> +	/* AUTH is only for master ... */
> +	if ((flags & DRM_AUTH) && drm_is_primary_client(file_priv)) {
> +		/* authenticated ones, or render capable on DRM_RENDER_ALLOW. */
> +		if (unlikely(!file_priv->authenticated) &&
> +		    unlikely(!drm_render_driver_and_ioctl(dev, flags)))

The double-unlikely looks a bit strange, I'd move it out so there's only
one. But this is correct too (because unlikely() && unlikely == unlikely(
&& )). Either way:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +			return -EACCES;
> +	}
>  
>  	/* MASTER is only for master or control clients */
>  	if (unlikely((flags & DRM_MASTER) &&
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const
  2019-01-14  8:54 [PATCH v2 1/2] drm: annotate drm_core_check_feature() dev arg. as const Emil Velikov
  2019-01-14  8:54 ` [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls Emil Velikov
@ 2019-01-14 11:39 ` Patchwork
  2019-01-14 11:57 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-01-14 17:49 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-01-14 11:39 UTC (permalink / raw)
  To: Emil Velikov; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const
URL   : https://patchwork.freedesktop.org/series/55154/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
57a47732f5af drm: annotate drm_core_check_feature() dev arg. as const
9ebc7fe4aa84 drm: allow render capable master with DRM_AUTH ioctls
-:35: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#35: 
[1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136

total: 0 errors, 1 warnings, 0 checks, 36 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const
  2019-01-14  8:54 [PATCH v2 1/2] drm: annotate drm_core_check_feature() dev arg. as const Emil Velikov
  2019-01-14  8:54 ` [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls Emil Velikov
  2019-01-14 11:39 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const Patchwork
@ 2019-01-14 11:57 ` Patchwork
  2019-01-14 17:49 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-01-14 11:57 UTC (permalink / raw)
  To: Emil Velikov; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const
URL   : https://patchwork.freedesktop.org/series/55154/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5414 -> Patchwork_11287
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55154/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_11287 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] / [fdo#108529] +1

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  
#### Possible fixes ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#108767] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - fi-kbl-7567u:       {SKIP} [fdo#109271] -> PASS +33

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +1

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#109241]: https://bugs.freedesktop.org/show_bug.cgi?id=109241
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (44 -> 38)
------------------------------

  Additional (2): fi-glk-j4005 fi-bdw-5557u 
  Missing    (8): fi-ilk-m540 fi-skl-gvtdvm fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-ctg-p8600 fi-icl-u3 fi-pnv-d510 


Build changes
-------------

    * Linux: CI_DRM_5414 -> Patchwork_11287

  CI_DRM_5414: f17b5429e6651fc08af802ab40281bae50f3a1a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4767: d9cd03c887a5f46bc002280d70a6716bb2bf8d43 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11287: 9ebc7fe4aa845e514ed9cd9f70accf718fdc38cf @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9ebc7fe4aa84 drm: allow render capable master with DRM_AUTH ioctls
57a47732f5af drm: annotate drm_core_check_feature() dev arg. as const

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11287/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const
  2019-01-14  8:54 [PATCH v2 1/2] drm: annotate drm_core_check_feature() dev arg. as const Emil Velikov
                   ` (2 preceding siblings ...)
  2019-01-14 11:57 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-01-14 17:49 ` Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-01-14 17:49 UTC (permalink / raw)
  To: Emil Velikov; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const
URL   : https://patchwork.freedesktop.org/series/55154/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5414_full -> Patchwork_11287_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_11287_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@kms_atomic_transition@plane-all-transition-nonblocking:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#109225]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_chv_cursor_fail@pipe-a-128x128-right-edge:
    - shard-skl:          PASS -> FAIL [fdo#104671]

  * igt@kms_chv_cursor_fail@pipe-b-128x128-bottom-edge:
    - shard-skl:          NOTRUN -> FAIL [fdo#104671]

  * igt@kms_color@pipe-b-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-onscreen:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@nonblocking-modeset-vs-cursor-atomic:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +12

  * igt@kms_flip_tiling@flip-changes-tiling:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +9

  * igt@kms_flip_tiling@flip-changes-tiling-y:
    - shard-skl:          PASS -> FAIL [fdo#107931] / [fdo#108303]

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-pwrite:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#107720] / [fdo#107724]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#107724] +6

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-badstride:
    - shard-skl:          PASS -> FAIL [fdo#105682]

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-render:
    - shard-skl:          PASS -> FAIL [fdo#103167] / [fdo#105682]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-pgflip-blt:
    - shard-skl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_panel_fitting@atomic-fastset:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109263]

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-apl:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +4

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815] +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_rotation_crc@primary-rotation-180:
    - shard-skl:          NOTRUN -> FAIL [fdo#103925] / [fdo#107815]

  * igt@pm_rpm@reg-read-ioctl:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#107713] / [fdo#108840]
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@system-suspend-modeset:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#104108] / [fdo#107807]

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries_display_off:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS +1

  * igt@gem_workarounds@suspend-resume:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS

  * igt@kms_atomic_transition@plane-toggle-modeset-transition:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +1

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-apl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +4

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS +1

  * igt@kms_flip@flip-vs-modeset-vs-hang:
    - shard-apl:          DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +10

  * igt@kms_flip_tiling@flip-y-tiled:
    - shard-skl:          FAIL [fdo#108303] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-gtt:
    - shard-iclb:         DMESG-FAIL [fdo#107724] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-iclb:         FAIL [fdo#105683] / [fdo#108040] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-iclb:         FAIL [fdo#109355] -> PASS +11

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-rte:
    - shard-iclb:         FAIL [fdo#105682] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +1

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-iclb:         FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_properties@connector-properties-legacy:
    - shard-iclb:         FAIL -> PASS

  * igt@kms_psr@cursor_blt:
    - shard-iclb:         FAIL [fdo#107383] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@pm_rpm@system-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] / [fdo#107807] -> PASS

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-shrfb-fliptrack:
    - shard-iclb:         FAIL [fdo#109355] -> DMESG-FAIL [fdo#107724]

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-blt:
    - shard-iclb:         FAIL [fdo#109355] -> DMESG-WARN [fdo#107724] / [fdo#108336]

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-iclb:         FAIL [fdo#109355] -> DMESG-FAIL [fdo#103167] / [fdo#107724]

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-iclb:         DMESG-FAIL [fdo#106885] -> FAIL [fdo#108948]

  * igt@kms_psr@sprite_render:
    - shard-iclb:         FAIL [fdo#107383] -> DMESG-WARN [fdo#107724]

  * igt@kms_setmode@basic:
    - shard-iclb:         FAIL [fdo#99912] -> DMESG-FAIL [fdo#107724] / [fdo#99912]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103925]: https://bugs.freedesktop.org/show_bug.cgi?id=103925
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107720]: https://bugs.freedesktop.org/show_bug.cgi?id=107720
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107931]: https://bugs.freedesktop.org/show_bug.cgi?id=107931
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109225]: https://bugs.freedesktop.org/show_bug.cgi?id=109225
  [fdo#109241]: https://bugs.freedesktop.org/show_bug.cgi?id=109241
  [fdo#109263]: https://bugs.freedesktop.org/show_bug.cgi?id=109263
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109275]: https://bugs.freedesktop.org/show_bug.cgi?id=109275
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109286]: https://bugs.freedesktop.org/show_bug.cgi?id=109286
  [fdo#109287]: https://bugs.freedesktop.org/show_bug.cgi?id=109287
  [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109292]: https://bugs.freedesktop.org/show_bug.cgi?id=109292
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109355]: https://bugs.freedesktop.org/show_bug.cgi?id=109355
  [fdo#109358]: https://bugs.freedesktop.org/show_bug.cgi?id=109358
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5414 -> Patchwork_11287

  CI_DRM_5414: f17b5429e6651fc08af802ab40281bae50f3a1a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4767: d9cd03c887a5f46bc002280d70a6716bb2bf8d43 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11287: 9ebc7fe4aa845e514ed9cd9f70accf718fdc38cf @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11287/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls
  2019-01-14 10:15   ` Daniel Vetter
@ 2019-01-16 11:58     ` Emil Velikov
  0 siblings, 0 replies; 7+ messages in thread
From: Emil Velikov @ 2019-01-16 11:58 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel

On 2019/01/14, Daniel Vetter wrote:
> On Mon, Jan 14, 2019 at 08:54:08AM +0000, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> > 
> > There are cases (in mesa and applications) where one would open the
> > primary node without properly authenticating the client.
> > 
> > Sometimes we don't check if the authentication succeeds, but there's
> > also cases we simply forget to do it.
> > 
> > The former was a case for Mesa where it did not not check the return
> > value of drmGetMagic() [1]. That was fixed recently although, there's
> > the question of older drivers or other apps that exbibit this behaviour.
> > 
> > While omitting the call results in issues as seen in [2] and [3].
> > 
> > In the libva case, libva itself doesn't authenticate the DRM client and
> > the vaGetDisplayDRM documentation doesn't mention if the app should
> > either.
> > 
> > As of today, the official vainfo utility doesn't authenticate.
> > 
> > To workaround issues like these, some users resort to running their apps
> > under sudo. Which admittedly isn't always a good idea.
> > 
> > Since any DRIVER_RENDER driver has sufficient isolation between clients,
> > we can use that, for unauthenticated [primary node] ioctls that require
> > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW.
> > 
> > v2:
> > - Rework/simplify if check (Daniel V)
> > - Add examples to commit messages, elaborate. (Daniel V)
> > 
> > [1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136
> > [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html
> > [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1
> > Testcase: igt/core_unauth_vs_render
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > ---
> > Daniel, the if conditionals did not work exactly as you put them.
> > This is the closest thing that I can think of.
> > ---
> >  drivers/gpu/drm/drm_ioctl.c | 20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 94bd872d56c4..08a0b4cc3a76 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -507,6 +507,13 @@ int drm_version(struct drm_device *dev, void *data,
> >  	return err;
> >  }
> >  
> > +static inline bool
> > +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags)
> > +{
> > +	return drm_core_check_feature(dev, DRIVER_RENDER) &&
> > +		(flags & DRM_RENDER_ALLOW);
> > +}
> > +
> >  /**
> >   * drm_ioctl_permit - Check ioctl permissions against caller
> >   *
> > @@ -521,14 +528,19 @@ int drm_version(struct drm_device *dev, void *data,
> >   */
> >  int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
> >  {
> > +	const struct drm_device *dev = file_priv->minor->dev;
> > +
> >  	/* ROOT_ONLY is only for CAP_SYS_ADMIN */
> >  	if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)))
> >  		return -EACCES;
> >  
> > -	/* AUTH is only for authenticated or render client */
> > -	if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) &&
> > -		     !file_priv->authenticated))
> > -		return -EACCES;
> > +	/* AUTH is only for master ... */
> > +	if ((flags & DRM_AUTH) && drm_is_primary_client(file_priv)) {
> > +		/* authenticated ones, or render capable on DRM_RENDER_ALLOW. */
> > +		if (unlikely(!file_priv->authenticated) &&
> > +		    unlikely(!drm_render_driver_and_ioctl(dev, flags)))
> 
> The double-unlikely looks a bit strange, I'd move it out so there's only
> one. But this is correct too (because unlikely() && unlikely == unlikely(
> && )). Either way:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
Ack, thanks. To stay consistent with the surrounding code I'll use:

	if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) {
		/* authenticated ones, or render capable on DRM_RENDER_ALLOW. */
		if (!file_priv->authenticated &&
		    !drm_render_driver_and_ioctl(dev, flags))



Btw, if you can skim through the IGT test that would be appreciated.

Cheers,
Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-16 11:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-14  8:54 [PATCH v2 1/2] drm: annotate drm_core_check_feature() dev arg. as const Emil Velikov
2019-01-14  8:54 ` [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls Emil Velikov
2019-01-14 10:15   ` Daniel Vetter
2019-01-16 11:58     ` Emil Velikov
2019-01-14 11:39 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm: annotate drm_core_check_feature() dev arg. as const Patchwork
2019-01-14 11:57 ` ✓ Fi.CI.BAT: success " Patchwork
2019-01-14 17:49 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.