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

Chris Wilson
2013-12-04T17:37:25Z

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.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre