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? ;) v2 is on its way. >> 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". >> 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. > 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. >> >> 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. Cheers David _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
