[PATCH 1/2] drm: vc4: set permissions for ioctls

[Emil Velikov]

On 10 June 2016 at 21:08, Eric Anholt <eric at anholt.net> wrote:
> Emil Velikov <emil.l.velikov at gmail.com> writes:
>
>> On 10 June 2016 at 00:42, Eric Anholt <eric at anholt.net> wrote:
>>> Rob Herring <robh at kernel.org> writes:
>>>
>>>> Ioctls generally have DRM_AUTH and DRM_RENDER_ALLOW set to restrict them
>>>> to authorized clients and render nodes. Without this, access from render
>>>> nodes fails.
>>>
>>> We've already got a fix to add RENDER_ALLOW submitted in the latest
>>> drm-vc4-fixes.  There's no reason to require auth on this
>>> implementation, though.
>>>
>> Not 100% sure but I think you do. At least every other driver does...
>>
>> Why: I'm thinking that without DRM_AUTH one will be able to open the
>> card# node and issue the said IOCTLs even if the client is not
>> authenticated. Which, obviously isn't a huge deal, but doesn't sound
>> right.
>>
>> Then again, my knowledge of vc4 is virtually non-existent, so there
>> might be something special happening here ?
>
> Let's flip this around: What is the problem you see with calling any of
> the ioctls without having gone through the auth dance?  I don't believe
> there's any reason to require auth, since you only have access to the
> buffers you create or import.
>
> Basically, auth was created a stopgap solution for "but if anyone had
> access to the DRM device, they could scrape the X frontbuffer!"

Personally I don't see any serious issues* with keeping DRM_AUTH out
of these. Although one could argue that the lack of it up-to recently
one was using non-auth access to the card node. The latter of which
lead to the DRM_RENDER_ALLOW going unnoticed.

That aside, I would urge that we have consistency on the topic.
Whether adding DRM_AUTH to the said VC4 ioctls, dropping DRM_AUTH
everywhere (if DRM_RENDER_ALLOW is present on the said ioclt) or
something else.

I believe Daniel V was wondering about the second at some stage.

Regards,
Emil

* Barring buggy user-space tailored for this behaviour.