Re: [Intel-gfx] [PATCH] drm/i915: Prevent double unref following alloc failure during execbuffer

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Dec 04, 2013 at 05:37:14PM +0000, Chris Wilson wrote:
> On Wed, Dec 04, 2013 at 09:23:24AM -0800, Ben Widawsky wrote:
> > On Wed, Dec 04, 2013 at 09:52:58AM +0000, Chris Wilson wrote:
> > > Whilst looking up the objects required for an execbuffer, an untimely
> > > allocation failure in creating the vma results in the object being
> > > unreferenced from two lists. The ownership during the lookup is meant to
> > > be moved from the list of objects being looked to the vma, and this
> > > double unreference upon error results in a use-after-free.
> > > 
> > > Fixes regression from
> > > commit 27173f1f95db5e74ceb35fe9a2f2f348ea11bac9
> > > Author: Ben Widawsky <ben@bwidawsk.net>
> > > Date:   Wed Aug 14 11:38:36 2013 +0200
> > > 
> > >     drm/i915: Convert execbuf code to use vmas
> > > 
> > > Based on the fix by Ben Widawsky.
> > 
> > A note on why this is an improvement over my fix would have been nice. I
> > had implemented something similar too, but found my eventual patch to be
> > a little easier to understand.
> 
> It all lies in the transfer of ownership comment. With that expressed,
> it is no longer an object residing on two lists that we must untangle,
> but a temporary list that holds the lookups which we convert into
> eb_vma. It is clear then we only need to clean up the temporary list
> upon failure.
> 
> > My real gripe is, I had already sent off my patch to be tested by QA -
> > and they give me about a 2d turnaround (not including weekends), which
> > means the soonest I could get this tested and get results is next Wed.
> > 
> > So if there is no improvement, I'd really appreciate this as a cleanup
> > on top of my patch.
> 
> Your changelog belies why not.

Picked up for -fixes (with the comment bikeshed as discussd on irc),
thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch