On 25/08/2013 17:09, David Herrmann wrote:
Hi
On Fri, Aug 23, 2013 at 2:00 PM, Martin Peres <martin.peres@xxxxxxx> wrote:
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.
Good you asked. I tested whether it works, I didn't actually verify
that it correctly fails in case of exploits. And in fact there is a
small bug (I return "1" instead of -EACCES, stupid verify_access()) so
user-space gets a segfault accessing the mmap when trying to exploit
this. That actually doesn't sound that bad, does it? ;)
Good to know I could contribute a little to your work. You're doing a
great job!
v2 is on its way.
Yep, saw it.
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.
Hm, I wouldn't say so. Applications like weston and kmscon no longer
use the legacy drmOpen() facility. They use udev+open(). So once it's
upstream, it's part of the API regardless of libdrm. So the sole
purpose of drm_rnodes is to mark it as "experimental".
Ah, I guess I'll have to have a look at this. I basically got preempted
from adding render node support to Weston and I didn't take the time
to check it again, the vma patches were more important first. Thanks
for saving me a giant headache with GEM/TTM, I spent two week ends
trying to track a leak for radeon cards.
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.
This is also what I think. We have a chance to get rid of all legacy
stuff, so maybe we should just drop it all.
Great!
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?
We discussed that a bit on IRC. Of course, we can add a lot of
wrappers and workarounds. We can make all the drmAuth stuff *just
work*. But that means, we keep all the legacy. As said, we have the
chance to introduce a new API and drop all the legacy. I think it is
worth a shot. And we also notice quite fast which user-space programs
need some rework.
Well, if by "all the legacy", you mean the authentication-related
functions then yes.
How do you plan on handling the case where the ddx has been updated and
passes
the render node to a not-yet-updated mesa? Mesa will try to authenticate
and it will fail.
Keeping the authentication IOCTLs seem to me like a lesser evil,
especially since they
would basically do nothing.
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 :)).
I plan on having a working user-space for XDC. Most of your patches
can be copied unchanged indeed. But servers other than Xorg don't use
that, so they need separate fixes.
Brilliant :) Looking forward to it!
Martin
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
