[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
On 25/08/2013 17:09, David Herrmann wrote: > Hi > > On Fri, Aug 23, 2013 at 2:00 PM, Martin Peres 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 >>
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi On Fri, Aug 23, 2013 at 2:00 PM, Martin Peres 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
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi On Fri, Aug 23, 2013 at 2:00 PM, Martin Peres martin.pe...@free.fr 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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
On 25/08/2013 17:09, David Herrmann wrote: Hi On Fri, Aug 23, 2013 at 2:00 PM, Martin Peres martin.pe...@free.fr 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
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi On Fri, Aug 23, 2013 at 2:34 PM, Christian K?nig wrote: >> Feel free to send a patch to dri-devel or just let me know the ioctls >> and I will include it in this series. > > > Do you have a branch somewhere I can grab? > > That's a bit easier than applying the patchset from the list. https://github.com/dvdhrm/linux/commits/rnodes I rebase it on update, so don't "git pull" it. Cheers David
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
> Feel free to send a patch to dri-devel or just let me know the ioctls > and I will include it in this series. Do you have a branch somewhere I can grab? That's a bit easier than applying the patchset from the list. Thanks, Christian. Am 23.08.2013 14:31, schrieb David Herrmann: > Hi > > On Fri, Aug 23, 2013 at 1:28 PM, Christian K?nig > wrote: >> Hi David, >> >> Am 23.08.2013 13:13, schrieb David Herrmann: >> >>> 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. >>> >>> 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. >>> >>> 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. >>> >>> Maybe we can get this into 3.11? >> >> A bit unlikely, but 3.12 should work fine. > whoops, 3.12 of course. > >> I'm working on a project that can make good use of this, so if Alex doesn't >> mind like to add the necessary radeon flags (should be only a few one liners >> anyway). > Feel free to send a patch to dri-devel or just let me know the ioctls > and I will include it in this series. > > Regards > David >
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi On Fri, Aug 23, 2013 at 1:28 PM, Christian K?nig wrote: > Hi David, > > Am 23.08.2013 13:13, schrieb David Herrmann: > >> 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. >> >> 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. >> >> 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. >> >> Maybe we can get this into 3.11? > > > A bit unlikely, but 3.12 should work fine. whoops, 3.12 of course. > I'm working on a project that can make good use of this, so if Alex doesn't > mind like to add the necessary radeon flags (should be only a few one liners > anyway). Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series. Regards David
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
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!
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi David, Am 23.08.2013 13:13, schrieb David Herrmann: > 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. > > 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. > > 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. > > Maybe we can get this into 3.11? A bit unlikely, but 3.12 should work fine. I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway). Cheers, Christian. > Regards > David > > David Herrmann (4): >drm/vma: add access management helpers >drm/gem: implement vma access management >drm: verify vma access in TTM+GEM drivers >drm: implement experimental render nodes > > Kristian H?gsberg (1): >drm/i915: Support render nodes > > Martin Peres (1): >drm/nouveau: Support render nodes > > Documentation/DocBook/drm.tmpl| 71 > drivers/gpu/drm/ast/ast_ttm.c | 4 +- > drivers/gpu/drm/cirrus/cirrus_ttm.c | 4 +- > drivers/gpu/drm/drm_drv.c | 15 ++-- > drivers/gpu/drm/drm_fops.c| 14 +-- > drivers/gpu/drm/drm_gem.c | 18 > drivers/gpu/drm/drm_pci.c | 9 ++ > drivers/gpu/drm/drm_platform.c| 9 ++ > drivers/gpu/drm/drm_stub.c| 10 +++ > drivers/gpu/drm/drm_usb.c | 9 ++ > drivers/gpu/drm/drm_vma_manager.c | 155 > ++ > drivers/gpu/drm/i915/i915_dma.c | 36 > drivers/gpu/drm/i915/i915_drv.c | 3 +- > drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 +- > drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +- > drivers/gpu/drm/nouveau/nouveau_drm.c | 24 +++--- > drivers/gpu/drm/qxl/qxl_ttm.c | 4 +- > drivers/gpu/drm/radeon/radeon_ttm.c | 4 +- > include/drm/drmP.h| 9 ++ > include/drm/drm_vma_manager.h | 21 - > 20 files changed, 373 insertions(+), 54 deletions(-) >
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
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. 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. 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. Maybe we can get this into 3.11? Regards David David Herrmann (4): drm/vma: add access management helpers drm/gem: implement vma access management drm: verify vma access in TTM+GEM drivers drm: implement experimental render nodes Kristian H?gsberg (1): drm/i915: Support render nodes Martin Peres (1): drm/nouveau: Support render nodes Documentation/DocBook/drm.tmpl| 71 drivers/gpu/drm/ast/ast_ttm.c | 4 +- drivers/gpu/drm/cirrus/cirrus_ttm.c | 4 +- drivers/gpu/drm/drm_drv.c | 15 ++-- drivers/gpu/drm/drm_fops.c| 14 +-- drivers/gpu/drm/drm_gem.c | 18 drivers/gpu/drm/drm_pci.c | 9 ++ drivers/gpu/drm/drm_platform.c| 9 ++ drivers/gpu/drm/drm_stub.c| 10 +++ drivers/gpu/drm/drm_usb.c | 9 ++ drivers/gpu/drm/drm_vma_manager.c | 155 ++ drivers/gpu/drm/i915/i915_dma.c | 36 drivers/gpu/drm/i915/i915_drv.c | 3 +- drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +- drivers/gpu/drm/nouveau/nouveau_drm.c | 24 +++--- drivers/gpu/drm/qxl/qxl_ttm.c | 4 +- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +- include/drm/drmP.h| 9 ++ include/drm/drm_vma_manager.h | 21 - 20 files changed, 373 insertions(+), 54 deletions(-) -- 1.8.3.4
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
On Fri, Aug 23, 2013 at 7:28 AM, Christian K?nig wrote: > Hi David, > > Am 23.08.2013 13:13, schrieb David Herrmann: > >> 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. >> >> 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. >> >> 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. >> >> Maybe we can get this into 3.11? > > > A bit unlikely, but 3.12 should work fine. > > I'm working on a project that can make good use of this, so if Alex doesn't > mind like to add the necessary radeon flags (should be only a few one liners > anyway). > No objections from me. Alex
[PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
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. 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. 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. Maybe we can get this into 3.11? Regards David David Herrmann (4): drm/vma: add access management helpers drm/gem: implement vma access management drm: verify vma access in TTM+GEM drivers drm: implement experimental render nodes Kristian Høgsberg (1): drm/i915: Support render nodes Martin Peres (1): drm/nouveau: Support render nodes Documentation/DocBook/drm.tmpl| 71 drivers/gpu/drm/ast/ast_ttm.c | 4 +- drivers/gpu/drm/cirrus/cirrus_ttm.c | 4 +- drivers/gpu/drm/drm_drv.c | 15 ++-- drivers/gpu/drm/drm_fops.c| 14 +-- drivers/gpu/drm/drm_gem.c | 18 drivers/gpu/drm/drm_pci.c | 9 ++ drivers/gpu/drm/drm_platform.c| 9 ++ drivers/gpu/drm/drm_stub.c| 10 +++ drivers/gpu/drm/drm_usb.c | 9 ++ drivers/gpu/drm/drm_vma_manager.c | 155 ++ drivers/gpu/drm/i915/i915_dma.c | 36 drivers/gpu/drm/i915/i915_drv.c | 3 +- drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +- drivers/gpu/drm/nouveau/nouveau_drm.c | 24 +++--- drivers/gpu/drm/qxl/qxl_ttm.c | 4 +- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +- include/drm/drmP.h| 9 ++ include/drm/drm_vma_manager.h | 21 - 20 files changed, 373 insertions(+), 54 deletions(-) -- 1.8.3.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
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! ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi On Fri, Aug 23, 2013 at 1:28 PM, Christian König christian.koe...@amd.com wrote: Hi David, Am 23.08.2013 13:13, schrieb David Herrmann: 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. 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. 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. Maybe we can get this into 3.11? A bit unlikely, but 3.12 should work fine. whoops, 3.12 of course. I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway). Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series. Regards David ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi On Fri, Aug 23, 2013 at 2:34 PM, Christian König christian.koe...@amd.com wrote: Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series. Do you have a branch somewhere I can grab? That's a bit easier than applying the patchset from the list. https://github.com/dvdhrm/linux/commits/rnodes I rebase it on update, so don't git pull it. Cheers David ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Hi David, Am 23.08.2013 13:13, schrieb David Herrmann: 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. 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. 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. Maybe we can get this into 3.11? A bit unlikely, but 3.12 should work fine. I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway). Cheers, Christian. Regards David David Herrmann (4): drm/vma: add access management helpers drm/gem: implement vma access management drm: verify vma access in TTM+GEM drivers drm: implement experimental render nodes Kristian Høgsberg (1): drm/i915: Support render nodes Martin Peres (1): drm/nouveau: Support render nodes Documentation/DocBook/drm.tmpl| 71 drivers/gpu/drm/ast/ast_ttm.c | 4 +- drivers/gpu/drm/cirrus/cirrus_ttm.c | 4 +- drivers/gpu/drm/drm_drv.c | 15 ++-- drivers/gpu/drm/drm_fops.c| 14 +-- drivers/gpu/drm/drm_gem.c | 18 drivers/gpu/drm/drm_pci.c | 9 ++ drivers/gpu/drm/drm_platform.c| 9 ++ drivers/gpu/drm/drm_stub.c| 10 +++ drivers/gpu/drm/drm_usb.c | 9 ++ drivers/gpu/drm/drm_vma_manager.c | 155 ++ drivers/gpu/drm/i915/i915_dma.c | 36 drivers/gpu/drm/i915/i915_drv.c | 3 +- drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +- drivers/gpu/drm/nouveau/nouveau_drm.c | 24 +++--- drivers/gpu/drm/qxl/qxl_ttm.c | 4 +- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +- include/drm/drmP.h| 9 ++ include/drm/drm_vma_manager.h | 21 - 20 files changed, 373 insertions(+), 54 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series. Do you have a branch somewhere I can grab? That's a bit easier than applying the patchset from the list. Thanks, Christian. Am 23.08.2013 14:31, schrieb David Herrmann: Hi On Fri, Aug 23, 2013 at 1:28 PM, Christian König christian.koe...@amd.com wrote: Hi David, Am 23.08.2013 13:13, schrieb David Herrmann: 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. 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. 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. Maybe we can get this into 3.11? A bit unlikely, but 3.12 should work fine. whoops, 3.12 of course. I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway). Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series. Regards David ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] DRM: VMA Access Management and Render Nodes
On Fri, Aug 23, 2013 at 7:28 AM, Christian König christian.koe...@amd.com wrote: Hi David, Am 23.08.2013 13:13, schrieb David Herrmann: 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. 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. 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. Maybe we can get this into 3.11? A bit unlikely, but 3.12 should work fine. I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway). No objections from me. Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
