[Intel-gfx] [PATCH] drm: allow render capable master with DRM_AUTH ioctls

[Michel Dänzer]

On 2019-07-03 7:10 p.m., Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at 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)
> 
> v3:
> - Use single unlikely (Daniel V)
> 
> v4:
> - Reapply patch, check for amdgpu/radeon inline.
> 
> [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 at lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

As discussed on IRC, IMHO this change requires more justification.

The system I'm writing this on has vainfo 2.4.0 installed, which opens a
render node and works fine without this change.

Similarly, if kmscube hasn't been fixed to use a render node yet, surely
it easily can.

You're asserting that the problem is wide-spread, and that fixing all
broken userspace isn't feasible, but I haven't seen any evidence
supporting that.

Since this change is essentially a workaround for broken userspace which
can never have worked, and has the potential of subverting the ongoing
transition from using primary nodes to render nodes in userspace code,
there needs to be evidence supporting that the benefit outweighs the risk.


-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer