[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes

[Martin Peres]

Le 23/08/2013 13:13, David Herrmann a écrit :
> Hi
>
> I reduced the vma access-management patches to a minimum. I now do filp*
> tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence,
> all gem drivers are safe now. For TTM drivers, I now use the already available
> verify_access() callback to get access to the underlying gem-object. Pretty
> simple.. Why hadn't I thought of that before?
>
> Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But
> they do their own access-management, anyway.

Great! Thanks! Have you checked they are really safe with my "exploits"?
I'll have another round of review even if it looked good the last time I 
checked.
> The 3 patches on top implement render-nodes. I added a "drm_rnodes" module
> parameter to core drm. You need to pass "drm.rnodes=1" on the kernel
> command-line or via sysfs _before_ loading a driver. Otherwise, render nodes
> will not be created.

By default, having the render nodes doesn't change the way the userspace
works at all. So, what is the point of protecting it behind a parameter?

Is it to make it clear this isn't part of the API yet? I would say that 
as long
as libdrm hasn't been updated, this isn't part of the API anyway.
> This allows us to test render-nodes and play with the API. I added FLINK for
> now so we can better test it. Not sure whether we should allow it in the end,
> though.

 From a security point of view, I don't think we should keep it as 
applications shouldn't
be trusted for not doing stupid things (because of code injection). So, 
unless we
plan on adding access control to flink via LSM, we shouldn't expose the 
feature
on render nodes.

 From a dev point of view, keeping it means that the XServer doesn't
have to know whether mesa supports render nodes or not. This is because
the authentication dance isn't available on render nodes so the x-server
has to tell mesa if it should authenticate or not. The other solution is 
to allow
the authentication ioctls on render nodes and just return 0 if this is a 
render node.
This way, we won't need any modification in mesa/xserver/dri2proto to pass
the information that no authentication is needed. In this solution, only 
libdrm and
the ddx should be modified to make use of the render node. That's not how I
did it on my render node patchset, can't remember why...

What do you guys think?
>
> Maybe we can get this into 3.11?

As long as we don't have to keep the interface stable (I don't want to 
expose flink
on render nodes), I'm all for pushing the code now. Otherwise, a kernel 
branch
somewhere is sufficient.

Do you plan on checking my userspace patches too? Those are enough to 
make use
of the render nodes on X, but I haven't tested that all the combinations 
of version
would still work. The libdrm work should be quite solid though (there 
are even libdrm
tests for the added functionalities :)).

Thanks for your work David!