[PATCH libdrm] libdrm: Allow dynamic drm majors on linux

[Emil Velikov]

On 4 September 2018 at 23:33, Dave Airlie <airlied at gmail.com> wrote:
> On Tue, 4 Sep 2018 at 03:00, Thomas Hellstrom <thellstrom at vmware.com> wrote:
>>
>> On 09/03/2018 06:33 PM, Daniel Vetter wrote:
>> > On Mon, Sep 03, 2018 at 11:16:29AM +0200, Thomas Hellstrom wrote:
>> >> On 08/31/2018 05:30 PM, Thomas Hellstrom wrote:
>> >>> On 08/31/2018 05:27 PM, Emil Velikov wrote:
>> >>>> On 31 August 2018 at 15:38, Michel Dänzer <michel at daenzer.net> wrote:
>> >>>>> [ Adding the amd-gfx list ]
>> >>>>>
>> >>>>> On 2018-08-31 3:05 p.m., Thomas Hellstrom wrote:
>> >>>>>> On 08/31/2018 02:30 PM, Emil Velikov wrote:
>> >>>>>>> On 31 August 2018 at 12:54, Thomas Hellstrom <thellstrom at vmware.com>
>> >>>>>>> wrote:
>> >>>>>>>> To determine whether a device node is a drm device
>> >>>>>>>> node or not, the code
>> >>>>>>>> currently compares the node's major number to the static drm major
>> >>>>>>>> device
>> >>>>>>>> number.
>> >>>>>>>>
>> >>>>>>>> This breaks the standalone vmwgfx driver on XWayland dri clients,
>> >>>>>>>>
>> >>>>>>> Any particular reason why the code doesn't use a fixed node there?
>> >>>>>>> It will make the diff vs the in-kernel driver a bit smaller.
>> >>>>>> Because then it won't be able to interoperate with other in-tree
>> >>>>>> drivers, like virtual drm drivers or passthrough usb drm drivers.
>> >>>>>> There is no clean way to share the minor number allocation
>> >>>>>> with in-tree
>> >>>>>> drm, so standalone vmwgfx is using dynamic major allocation.
>> >>>>> I wonder why I haven't heard of any of these issues with the standalone
>> >>>>> version of amdgpu shipped in packaged AMD releases. Does that
>> >>>>> also use a
>> >>>>> different major number? If yes, maybe it's just that nobody has tried
>> >>>>> Xwayland clients with that driver. If no, how does it avoid the other
>> >>>>> issues described above?
>> >>>>>
>> >>>> AFAICT, the difference is that the standalone vmwgfx uses an internal
>> >>>> copy of drm core.
>> >>>> It doesn't reuse the in-kernel drm, hence it cannot know which minor
>> >>>> it can use.
>> >>>>
>> >>>> -Emil
>> >>> Actually, standalone vmwgfx could perhaps also try to allocate minors
>> >>> from 63 and downwards. That might work, but needs some verification.
>> >>>
>> >> So unfortuntately this doesn't work since the in-tree drm's file operations
>> >> are registered with the DRM_MAJOR.
>> >> So I still think the patch is the way to go. If people are concerned that
>> >> also fbdev file descriptors are allowed, perhaps there are other sysfs
>> >> traits we can look at?
>> > Somewhat out of curiosity, but why do you have to overwrite all of drm?
>> > amdgpu seems to be able to pull their stunt off without ...
>> > -Daniel
>>
>> At the time we launched the standalone vmwgfx, the DRM <-> driver
>> interface was moving considerably more rapidly than the DRM <-> kernel
>> interface. I think that's still the case. Hence less work for us. Also
>> meant we can install the full driver stack with latest features on
>> fairly old VMs without backported DRM functionality.
>>
>
> I think this should be fine for 99% of drm usage, there may be corner
> cases in wierd places, but I can't point to any that really matter
> (maybe strace?)
>
Having a closer look, I think this will break the Firefox/Chrome sandboxing :-\
I cannot see the path /sys/dev/char/%d:%d/device/drm in the allowed
list [1] [2].

Thomas, can you please send a patch to the respective teams or give
them a heads up?

Thanks
Emil

[1] https://hg.mozilla.org/releases/mozilla-beta/file/264fcd3206a6/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
[2] https://chromium.googlesource.com/chromium/src/+/8655d49f657d3878c937f1387b3d31fa66c8e76a/content/gpu/gpu_sandbox_hook_linux.cc