[Intel-gfx] [PATCH v2] drm/i915: Fixup userptr mmu notifier registration error handling

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-10-17 16:09:08)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Avoid dereferencing the error pointer and also avoid returning NULL
> from i915_mmu_notifier_find since the callers do not expect that.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
> Fixes: 7741b547b6e0 ("drm/i915: Preallocate our mmu notifier workequeu to unbreak cpu hotplug deadlock")
> Cc: Dan Carpenter <dan.carpenter at oracle.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_userptr.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index cdc9be799eee..e26b23171b56 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -221,15 +221,17 @@ i915_mmu_notifier_find(struct i915_mm_struct *mm)
>                         /* Protected by mm_lock */
>                         mm->mn = fetch_and_zero(&mn);
>                 }
> -       } else {
> -               /* someone else raced and successfully installed the mmu
> -                * notifier, we can cancel our own errors */
> +       } else if (mm->mn) {
> +               /*
> +                * Someone else raced and successfully installed the mmu
> +                * notifier, we can cancel our own errors.
> +                */
>                 err = 0;

residual else branch => err != 0 (i.e. mn is an ERR_PTR)

(Note that we could do
	if (mm->mn) {
	} else if (!IS_ERR(mn)) {
	} else {
		err = ERR_PTR(mn);
	}
just to have slightly more balanced branches; and is how I verified you
have all bases covered.)

>         }
>         mutex_unlock(&mm->i915->mm_lock);
>         up_write(&mm->mm->mmap_sem);
>  
> -       if (mn) {
> +       if (mn && !IS_ERR(mn)) {

Ok.
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris