VK_EXT_aquire_xlib_display and kernel security concerns

Tue Oct 17 06:24:32 UTC 2017

Just throwing a bit more context into the discussion here.

On Mon, Oct 16, 2017 at 11:51 PM, James Jones <jajones at nvidia.com> wrote:
> On 10/16/2017 01:33 PM, Dave Airlie wrote:
>>
>> On 17 October 2017 at 06:01, James Jones <jajones at nvidia.com> wrote:
>>>
>>> On 10/16/2017 12:28 PM, Keith Packard wrote:
>>>>
>>>>
>>>> James Jones <jajones at nvidia.com> writes:
>>>>
>>>>> I think at a lower level, we have different views of how
>>>>> vkGetPhysicalDeviceDisplayPropertiesKHR/VK_KHR_display works.
>>>>> vkGetPhysicalDeviceDisplayPropertiesKHR() is suppose to enumerate all
>>>>> the displays on a given device.
>>>>
>>>>
>>>>
>>>> In many devices, the display controller and rendering hardware are
>>>> separate devices, so it's up to user space to figure out the connection
>>>> between them, and user DMAbufs to pass rendered output to the display.
>>>
>>>
>>>
>>> Understood.  In that case, no displays should be enumerated on the
>>> "rendering hardware" device in Vulkan, unless the driver vendor opts for
>>> heroics.
>>>
>>>>> I assumed DRM drivers would try the display-capable node, then fall
>>>>> back to a render node if that failed.
>>>>
>>>>
>>>>
>>>> No, they cannot as the display node is protected from normal user
>>>> access.
>>>
>>>
>>>
>>> Right, but normal processes don't need the display node, or aren't
>>> supposed
>>> to.  I assumed apps that need direct-to-display would have elevated
>>> permissions on systems configured to require it, or use "acquire"
>>> extensions, potentially combined with native secondary auth mechanisms,
>>> to
>>> get access.  This is why it bothers me that "normal user access" doesn't
>>> include display enumeration.
>>
>>
>> If they use acquire extensions they get display enumeration, the problem
>> is
>> display enumeration before the acquire would need special permission
>> elevation.
>>
>>>>> VK_KHR_display, but the core of it is really just a display
>>>>> enumeration API.
>>>>
>>>>
>>>>
>>>> Right, I think all I really need is to hand the driver a connection to
>>>> the X server and it can use that to enumerate the available displays
>>>> through the Vulkan API.
>>
>>
>> The thing is if you are running under X I don't think apps should be
>> bypassing
>> things and going straight to hw enumeration, it makes too many problems
>> harder,
>> and in systems where all you have are separated display/render devices it
>> makes this extension impossible to implement, since as you've said vulkan
>> has
>> no way to enumerate such things.
>>>
>>>
>>>>
>>>
>>
>>>
>>> Given the current definition of a Vulkan-capable device, yes.  It's
>>> annoying
>>> that Vulkan can't yet expose a device that only has displays and the
>>> ability
>>> to import memory objects and create images from them.
>>>
>>> I prefer the enumeration ability as a general principle though.  Is the
>>> discussion with Daniel Vetter captured somewhere so I can read up on the
>>> objections?  I couldn't find it with a naive Google or DRI devel search.
>>
>>
>> Since we have to actually publish our kernel APIs people find inventive
>> ways
>> to do things with them that we don't end up breaking later, and having to
>> find
>> ways to keep their stupid working. You don't have this problem, nobody
>> opens
>> /dev/nvidia directly from an app and does anything useful, and even if
>> they do
>> you guys don't care.
>>
>> Previously for example we opened up DRM_WAIT_VBLANK or forgot to close it,
>> this leds to apps directly poking it behind the X server back, trying
>> to determine
>> timings outside the compositor incorrectly.
>>
>> So if we expose all the enumeration APIs to "render" only nodes, then we
>> will
>> get information leaks, like the currently attached framebuffer for
>> planes, ways to
>> poll state like device connectedness, (we get people trying to bypass dpms
>> all
>> the time), able to pull the device properties out, it's just a genie
>> in a bottle scenario
>> once we provide it we can't repeal it later. It's only sane that we
>> restrict the API
>> to what we want to support.
>
>
> Thanks for the background Dave.  This is a good point.  If the granularity
> of current ioctls makes it difficult to draw the right line, would proposing
> new ioctls that can only query non-sensitive state (TBD what that is) be the
> right direction?

The problem isn't so much the lack of ioctls, but additional
information (which the kernel simply doesn't have). On upstream you
essentially have 2 types of drm fds: rendernodes, for rendering, and
master nodes, for display. Buffer sharing between them is done with
dma-buf. They can happen to be provided by the same driver, but you
always have to deal with both (at least if we ignore some older buffer
sharing schemes with some not-so-great security properties).

So for a vk app trying to display on an X11 display, just asking the
kernel directly doesn't make sense, it needs to go through the
compositor (which then knows which display comes from which display fd
it has, and can lease that to the client). If the kernel would expose
this directly to X11 apps (in the rendernodes even) we'd need to
either second-guess how the compositor works in the kernel (ugh) or
apps would need to second-guess how the compositors

> I don't think EXT_acquire_xlib_display is the right use case to justify such
> enumeration, as I like the proposed solution of enumerating through X11 in
> this case.  However, I think direct enumeration is generally useful
> functionality, if nothing else just for things like vkinfo or DRM
> equivalents.

It hink for vk on bare kms Keith has an extension so that you can pass
in the right kms fd (because again, the rendernode one you use for
rendering isn't all that useful).

So yeah, at least if you want to run on any system on drm, you need to
do some buffer sharing gymnastics on linux, and hence the display
extensions (as-is at least) need some buffer sharing dances already.
Which I think is totally ok, if an app wants raw access there's no
point in indirecting through a vk extension imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch