VK_EXT_aquire_xlib_display and kernel security concerns

[James Jones]

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?

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.

Thanks,
-James

> Dave.
>