Re: [Intel-gfx] [PATCH] drm/i915: Fix vm use-after-free in vma destruction

From: Andi Shyti <andi.shyti@linux.intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	Matthew Auld <matthew.auld@intel.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix vm use-after-free in vma destruction
Date: Thu, 19 May 2022 23:46:38 +0200	[thread overview]
Message-ID: <Yoa6vqN/BOfoF53j@intel.intel> (raw)
In-Reply-To: <20220512094045.792373-1-thomas.hellstrom@linux.intel.com>

Hi Thomas,

On Thu, May 12, 2022 at 11:40:45AM +0200, Thomas Hellström wrote:
> In vma destruction, the following race may occur:
> 
> Thread 1:	    		  Thread 2:
> i915_vma_destroy();
> 
>   ...
>   list_del_init(vma->vm_link);
>   ...
>   mutex_unlock(vma->vm->mutex);
> 				  __i915_vm_release();
> release_references();
> 
> And in release_reference() we dereference vma->vm to get to the
> vm gt pointer, leadin go a use-after free.

leading to

[...]

> -static void release_references(struct i915_vma *vma, bool vm_ddestroy)
> +static void release_references(struct i915_vma *vma, struct intel_gt *gt,
> +			       bool vm_ddestroy)
>  {
>  	struct drm_i915_gem_object *obj = vma->obj;
> -	struct intel_gt *gt = vma->vm->gt;
>  
>  	GEM_BUG_ON(i915_vma_is_active(vma));

but then we have

	if (vm_ddestroy)
		i915_vm_resv_put(vma->vm);

were we reference to a freed vm, right? Do we need to check it
here, as well?

Andi