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

[Koenig, Christian]

Am 27.05.19 um 14:10 schrieb Emil Velikov:
> On 2019/05/27, Christian König wrote:
>> Am 27.05.19 um 10:17 schrieb Emil Velikov:
>>> 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:
>>> - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU
>>> issue is fixed with earlier patch.
>> As far as I can see this only affects the following two IOCTLs after
>> removing DRM_AUTH from the DRM_RENDER_ALLOW IOCTLs:
>>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD,
>>> drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>>          DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE,
>>> drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW)
>> So I think it would be simpler to just remove DRM_AUTH from those two
>> instead of allowing it for everybody.
>>
> If I understand you correctly this will remove DRM_AUTH also for drivers
> which expose only a primary node. I'm not sure if that is a good idea.

That's a good point, but I have doubts that those drivers implement the 
necessary callbacks and/or set the core feature flag for the IOCTLs.

So the maximum what could happen is that you change the returned error 
from -EACCES into -EOPNOTSUPP/-ENOSYS.

Regards,
Christian.

> That said, if others are OK with the idea I will prepare a patch.
>
> Thanks
> Emil