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@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