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!
