On Tue, Feb 03, 2015 at 03:48:17PM +0100, Michał Winiarski wrote:
> It's possible for invalidate_range_start mmu notifier callback to race
> against userptr object release. If the gem object was released prior to
> obtaining the spinlock in invalidate_range_start we're hitting null
> pointer dereference.
> 
> Testcase: igt/gem_userptr_blits/stress-mm-invalidate-close*
> Cc: Chris Wilson <[email protected]>
> Signed-off-by: Michał Winiarski <[email protected]>
Reviewed-by: Chris Wilson <[email protected]>

Though I would personally remove the extra newline and add an extra comment 
instead:

> ---
>  drivers/gpu/drm/i915/i915_gem_userptr.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
> b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index d182058..64b8802 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -113,7 +113,10 @@ restart:
>                       continue;
>  
>               obj = mo->obj;
> -             drm_gem_object_reference(&obj->base);
> +             if (!kref_get_unless_zero(&obj->base.refcount))
> +                     continue;
> +
>               spin_unlock(&mn->lock);
>  
>               cancel_userptr(obj);
> @@ -149,7 +152,13 @@ static void 
> i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
>                       it = interval_tree_iter_first(&mn->objects, start, end);
>               if (it != NULL) {
>                       obj = container_of(it, struct i915_mmu_object, it)->obj;
> -                     drm_gem_object_reference(&obj->base);
                        /* The mmu_object is released late when
                         * destroying the GEM object so it is entirely
                         * possible to gain a reference on an object
                         * in the process of being freed since our
                         * serialisation is via the spinlock and not the
                         * struct_mutex - and consequently use it
                         * after it is freed and then double free it.
                         */
> +                     if (!kref_get_unless_zero(&obj->base.refcount)) {
> +                             spin_unlock(&mn->lock);
> +                             serial = 0;
> +                             continue;
> +                     }
> +
>                       serial = mn->serial;
>               }
>               spin_unlock(&mn->lock);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to