Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Thomas Hellstrom
On 09/13/2013 10:58 AM, Maarten Lankhorst wrote: Op 13-09-13 10:23, Thomas Hellstrom schreef: On 09/13/2013 09:51 AM, Maarten Lankhorst wrote: Op 13-09-13 09:46, Thomas Hellstrom schreef: On 09/13/2013 09:16 AM, Maarten Lankhorst wrote: Op 13-09-13 08:44, Thomas Hellstrom schreef: On 09/12/2

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Peter Zijlstra
On Fri, Sep 13, 2013 at 10:41:54AM +0200, Daniel Vetter wrote: > On Fri, Sep 13, 2013 at 10:29 AM, Peter Zijlstra wrote: > > On Fri, Sep 13, 2013 at 09:46:03AM +0200, Thomas Hellstrom wrote: > >> >>if (!bo_tryreserve()) { > >> >> up_read mmap_sem(); // Release the mmap_sem to avoid deadlocks.

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Maarten Lankhorst
Op 13-09-13 10:23, Thomas Hellstrom schreef: > On 09/13/2013 09:51 AM, Maarten Lankhorst wrote: >> Op 13-09-13 09:46, Thomas Hellstrom schreef: >>> On 09/13/2013 09:16 AM, Maarten Lankhorst wrote: Op 13-09-13 08:44, Thomas Hellstrom schreef: > On 09/12/2013 11:50 PM, Maarten Lankhorst wrot

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Daniel Vetter
On Fri, Sep 13, 2013 at 10:29 AM, Peter Zijlstra wrote: > On Fri, Sep 13, 2013 at 09:46:03AM +0200, Thomas Hellstrom wrote: >> >>if (!bo_tryreserve()) { >> >> up_read mmap_sem(); // Release the mmap_sem to avoid deadlocks. >> >> bo_reserve(); // Wait for the BO to become avai

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Thomas Hellstrom
On 09/13/2013 10:32 AM, Daniel Vetter wrote: On Fri, Sep 13, 2013 at 10:23 AM, Thomas Hellstrom wrote: As previously mentioned, copy_from_user should return -EFAULT, since the VMAs are marked with VM_IO. It should not recurse into fault(), so evil user-space looses. I haven't put a printk in t

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Daniel Vetter
On Fri, Sep 13, 2013 at 10:23 AM, Thomas Hellstrom wrote: > As previously mentioned, copy_from_user should return -EFAULT, since the > VMAs are marked with VM_IO. It should not recurse into fault(), so evil > user-space looses. I haven't put a printk in the code to prove this, but gem mmap also s

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Peter Zijlstra
On Fri, Sep 13, 2013 at 09:46:03AM +0200, Thomas Hellstrom wrote: > >>if (!bo_tryreserve()) { > >> up_read mmap_sem(); // Release the mmap_sem to avoid deadlocks. > >> bo_reserve(); // Wait for the BO to become available > >> (interruptible) > >> bo_unreserve();

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Daniel Vetter
On Fri, Sep 13, 2013 at 7:33 AM, Thomas Hellstrom wrote: > Given that all copy_to_user / copy_from_user paths are actually hit during > testing, right? Ime it requires a bit of ingenuity to properly test this from userspace. We're using a few tricks in drm/i915 kernel testing: - When we hand a gt

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Thomas Hellstrom
On 09/13/2013 09:51 AM, Maarten Lankhorst wrote: Op 13-09-13 09:46, Thomas Hellstrom schreef: On 09/13/2013 09:16 AM, Maarten Lankhorst wrote: Op 13-09-13 08:44, Thomas Hellstrom schreef: On 09/12/2013 11:50 PM, Maarten Lankhorst wrote: Op 12-09-13 18:44, Thomas Hellstrom schreef: On 09/12/2

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Maarten Lankhorst
Op 13-09-13 09:46, Thomas Hellstrom schreef: > On 09/13/2013 09:16 AM, Maarten Lankhorst wrote: >> Op 13-09-13 08:44, Thomas Hellstrom schreef: >>> On 09/12/2013 11:50 PM, Maarten Lankhorst wrote: Op 12-09-13 18:44, Thomas Hellstrom schreef: > On 09/12/2013 05:45 PM, Maarten Lankhorst wrot

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Thomas Hellstrom
On 09/13/2013 09:16 AM, Maarten Lankhorst wrote: Op 13-09-13 08:44, Thomas Hellstrom schreef: On 09/12/2013 11:50 PM, Maarten Lankhorst wrote: Op 12-09-13 18:44, Thomas Hellstrom schreef: On 09/12/2013 05:45 PM, Maarten Lankhorst wrote: Op 12-09-13 17:36, Daniel Vetter schreef: On Thu, Sep 1

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-13 Thread Maarten Lankhorst
Op 13-09-13 08:44, Thomas Hellstrom schreef: > On 09/12/2013 11:50 PM, Maarten Lankhorst wrote: >> Op 12-09-13 18:44, Thomas Hellstrom schreef: >>> On 09/12/2013 05:45 PM, Maarten Lankhorst wrote: Op 12-09-13 17:36, Daniel Vetter schreef: > On Thu, Sep 12, 2013 at 5:06 PM, Peter Zijlstra

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Hellstrom
On 09/12/2013 11:50 PM, Maarten Lankhorst wrote: Op 12-09-13 18:44, Thomas Hellstrom schreef: On 09/12/2013 05:45 PM, Maarten Lankhorst wrote: Op 12-09-13 17:36, Daniel Vetter schreef: On Thu, Sep 12, 2013 at 5:06 PM, Peter Zijlstra wrote: So I'm poking around the preemption code and stumble

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Hellstrom
On 09/12/2013 11:50 PM, Maarten Lankhorst wrote: Op 12-09-13 18:44, Thomas Hellstrom schreef: On 09/12/2013 05:45 PM, Maarten Lankhorst wrote: Op 12-09-13 17:36, Daniel Vetter schreef: On Thu, Sep 12, 2013 at 5:06 PM, Peter Zijlstra wrote: So I'm poking around the preemption code and stumble

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Maarten Lankhorst
Op 12-09-13 18:44, Thomas Hellstrom schreef: > On 09/12/2013 05:45 PM, Maarten Lankhorst wrote: >> Op 12-09-13 17:36, Daniel Vetter schreef: >>> On Thu, Sep 12, 2013 at 5:06 PM, Peter Zijlstra >>> wrote: So I'm poking around the preemption code and stumbled upon: drivers/gpu/drm/i9

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Hellstrom
On 09/12/2013 10:39 PM, Thomas Gleixner wrote: On Thu, 12 Sep 2013, Daniel Vetter wrote: On Thu, Sep 12, 2013 at 10:20 PM, Thomas Gleixner wrote: I think for ttm drivers it's just execbuf being exploitable. But on drm/i915 we've had the same issue with the pwrite/pread ioctls, so a simple glB

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Gleixner
On Thu, 12 Sep 2013, Peter Zijlstra wrote: > On Thu, Sep 12, 2013 at 05:35:43PM +0100, Chris Wilson wrote: > > Not quite, as it would be possible for the evil userspace to trigger a > > GPU hang that would cause the sane userspace to spin indefinitely > > waiting for the error recovery to kick in

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Gleixner
On Thu, 12 Sep 2013, Daniel Vetter wrote: > On Thu, Sep 12, 2013 at 10:20 PM, Thomas Gleixner wrote: > >> I think for ttm drivers it's just execbuf being exploitable. But on > >> drm/i915 we've > >> had the same issue with the pwrite/pread ioctls, so a simple > >> glBufferData(glMap) kind of recu

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Peter Zijlstra
On Thu, Sep 12, 2013 at 05:35:43PM +0100, Chris Wilson wrote: > Not quite, as it would be possible for the evil userspace to trigger a > GPU hang that would cause the sane userspace to spin indefinitely > waiting for the error recovery to kick in. So with FIFOn+1 preempting FIFOn its a live-lock

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Daniel Vetter
On Thu, Sep 12, 2013 at 6:22 PM, Peter Zijlstra wrote: > If 'sane' userspace is never supposed to do this, then only insane > userspace is going to hurt from this and that's a GOOD (tm) thing, > right? ;-) Afaik sane userspace doesn't hit the _deadlock_ (or lifelock if we have the set_need_resche

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Daniel Vetter
On Thu, Sep 12, 2013 at 9:58 PM, Thomas Gleixner wrote: > >> On Thu, Sep 12, 2013 at 6:22 PM, Peter Zijlstra wrote: >> > If 'sane' userspace is never supposed to do this, then only insane >> > userspace is going to hurt from this and that's a GOOD (tm) thing, >> > right? ;-) >> >> Afaik sane user

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Daniel Vetter
On Thu, Sep 12, 2013 at 6:44 PM, Thomas Hellstrom wrote: > > I think a possible fix would be if fault() were allowed to return an error > and drop the mmap_sem() before returning. > > Otherwise we need to track down all copy_to_user / copy_from_user which > happen with bo::reserve held. For maxim

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Gleixner
On Thu, 12 Sep 2013, Daniel Vetter wrote: > On Thu, Sep 12, 2013 at 6:22 PM, Peter Zijlstra wrote: > > If 'sane' userspace is never supposed to do this, then only insane > > userspace is going to hurt from this and that's a GOOD (tm) thing, > > right? ;-) > > Afaik sane userspace doesn't hit the

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Daniel Vetter
On Thu, Sep 12, 2013 at 10:20 PM, Thomas Gleixner wrote: >> I think for ttm drivers it's just execbuf being exploitable. But on >> drm/i915 we've >> had the same issue with the pwrite/pread ioctls, so a simple >> glBufferData(glMap) kind of recursion from gl clients blew the kernel >> to pieces ..

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Gleixner
On Thu, 12 Sep 2013, Daniel Vetter wrote: > On Thu, Sep 12, 2013 at 9:58 PM, Thomas Gleixner wrote: > > > >> On Thu, Sep 12, 2013 at 6:22 PM, Peter Zijlstra > >> wrote: > >> > If 'sane' userspace is never supposed to do this, then only insane > >> > userspace is going to hurt from this and that

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Hellstrom
On 09/12/2013 05:45 PM, Maarten Lankhorst wrote: Op 12-09-13 17:36, Daniel Vetter schreef: On Thu, Sep 12, 2013 at 5:06 PM, Peter Zijlstra wrote: So I'm poking around the preemption code and stumbled upon: drivers/gpu/drm/i915/i915_gem.c:set_need_resched(); drivers/gpu/drm/ttm

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Chris Wilson
On Thu, Sep 12, 2013 at 06:22:10PM +0200, Peter Zijlstra wrote: > On Thu, Sep 12, 2013 at 05:58:49PM +0200, Daniel Vetter wrote: > > On Thu, Sep 12, 2013 at 5:43 PM, Peter Zijlstra > > wrote: > > >> The one in ttm is just bonghits to shut up lockdep: ttm can recurse > > >> into it's own pagefault

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Thomas Hellstrom
On 09/12/2013 05:58 PM, Daniel Vetter wrote: On Thu, Sep 12, 2013 at 5:43 PM, Peter Zijlstra wrote: The one in ttm is just bonghits to shut up lockdep: ttm can recurse into it's own pagefault handler and then deadlock, the trylock just keeps lockdep quiet. Could you describe how it could recu

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Peter Zijlstra
On Thu, Sep 12, 2013 at 05:58:49PM +0200, Daniel Vetter wrote: > On Thu, Sep 12, 2013 at 5:43 PM, Peter Zijlstra wrote: > >> The one in ttm is just bonghits to shut up lockdep: ttm can recurse > >> into it's own pagefault handler and then deadlock, the trylock just > >> keeps lockdep quiet. We've

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Daniel Vetter
On Thu, Sep 12, 2013 at 5:43 PM, Peter Zijlstra wrote: >> The one in ttm is just bonghits to shut up lockdep: ttm can recurse >> into it's own pagefault handler and then deadlock, the trylock just >> keeps lockdep quiet. We've had that bug arise in drm/i915 due to some >> fun userspace did and now

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Peter Zijlstra
On Thu, Sep 12, 2013 at 05:36:43PM +0200, Daniel Vetter wrote: > The set_need_resched in i915_gem.c:i915_gem_fault can actually be > removed. It was there to give the error handler a chance to sneak in > and reset the hw/sw tracking when the gpu is dead. That hack goes back > to the days when the l

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Maarten Lankhorst
Op 12-09-13 17:36, Daniel Vetter schreef: > On Thu, Sep 12, 2013 at 5:06 PM, Peter Zijlstra wrote: >> So I'm poking around the preemption code and stumbled upon: >> >> drivers/gpu/drm/i915/i915_gem.c:set_need_resched(); >> drivers/gpu/drm/ttm/ttm_bo_vm.c:set

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Daniel Vetter
On Thu, Sep 12, 2013 at 5:06 PM, Peter Zijlstra wrote: > > So I'm poking around the preemption code and stumbled upon: > > drivers/gpu/drm/i915/i915_gem.c:set_need_resched(); > drivers/gpu/drm/ttm/ttm_bo_vm.c:set_need_resched(); > drivers/gpu/drm/ttm/ttm_bo_

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Maarten Lankhorst
Op 12-09-13 17:06, Peter Zijlstra schreef: > Hi Dave, > > So I'm poking around the preemption code and stumbled upon: > > drivers/gpu/drm/i915/i915_gem.c:set_need_resched(); > drivers/gpu/drm/ttm/ttm_bo_vm.c:set_need_resched(); > drivers/gpu/drm/ttm/ttm_bo_vm

[BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Peter Zijlstra
Hi Dave, So I'm poking around the preemption code and stumbled upon: drivers/gpu/drm/i915/i915_gem.c:set_need_resched(); drivers/gpu/drm/ttm/ttm_bo_vm.c:set_need_resched(); drivers/gpu/drm/ttm/ttm_bo_vm.c:set_need_resched(); drivers/

Re: [BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE

2013-09-12 Thread Peter Zijlstra
On Thu, Sep 12, 2013 at 05:11:25PM +0200, Maarten Lankhorst wrote: > Op 12-09-13 17:06, Peter Zijlstra schreef: > > Hi Dave, > > > > So I'm poking around the preemption code and stumbled upon: > > > > drivers/gpu/drm/i915/i915_gem.c:set_need_resched(); > > drivers/gpu/drm/ttm/ttm_bo