[Intel-gfx] [bug report] drm/i915: Use ttm mmap handling for ttm bo's.

[Dan Carpenter]

Hello Maarten Lankhorst,

This is a semi-automatic email about new static checker warnings.

The patch cf3e3e86d779: "drm/i915: Use ttm mmap handling for ttm 
bo's." from Jun 10, 2021, leads to the following Smatch complaint:

    ./drivers/gpu/drm/i915/gem/i915_gem_mman.c:1008 i915_gem_mmap()
    error: we previously assumed 'node' could be null (see line 953)

./drivers/gpu/drm/i915/gem/i915_gem_mman.c
   949          drm_vma_offset_lock_lookup(dev->vma_offset_manager);
   950          node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
   951                                                    vma->vm_pgoff,
   952							  vma_pages(vma));
   953		if (node && drm_vma_node_is_allowed(node, priv)) {
                    ^^^^
Lots of NULL checking

   954			/*
   955			 * Skip 0-refcnted objects as it is in the process of being
   956			 * destroyed and will be invalid when the vma manager lock
   957			 * is released.
   958			 */
   959			if (!node->driver_private) {
   960				mmo = container_of(node, struct i915_mmap_offset, vma_node);
   961				obj = i915_gem_object_get_rcu(mmo->obj);
   962	
   963				GEM_BUG_ON(obj && obj->ops->mmap_ops);
   964			} else {
   965				obj = i915_gem_object_get_rcu
   966					(container_of(node, struct drm_i915_gem_object,
   967						      base.vma_node));
   968	
   969				GEM_BUG_ON(obj && !obj->ops->mmap_ops);
   970			}
   971		}
   972		drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
   973		rcu_read_unlock();
   974		if (!obj)
   975			return node ? -EACCES : -EINVAL;
                               ^^^^
Checked

   976	
   977		if (i915_gem_object_is_readonly(obj)) {
   978			if (vma->vm_flags & VM_WRITE) {
   979				i915_gem_object_put(obj);
   980				return -EINVAL;
   981			}
   982			vm_flags_clear(vma, VM_MAYWRITE);
   983		}
   984	
   985		anon = mmap_singleton(to_i915(dev));
   986		if (IS_ERR(anon)) {
   987			i915_gem_object_put(obj);
   988			return PTR_ERR(anon);
   989		}
   990	
   991		vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP | VM_IO);
   992	
   993		/*
   994		 * We keep the ref on mmo->obj, not vm_file, but we require
   995		 * vma->vm_file->f_mapping, see vma_link(), for later revocation.
   996		 * Our userspace is accustomed to having per-file resource cleanup
   997		 * (i.e. contexts, objects and requests) on their close(fd), which
   998		 * requires avoiding extraneous references to their filp, hence why
   999		 * we prefer to use an anonymous file for their mmaps.
  1000		 */
  1001		vma_set_file(vma, anon);
  1002		/* Drop the initial creation reference, the vma is now holding one. */
  1003		fput(anon);
  1004	
  1005		if (obj->ops->mmap_ops) {
  1006			vma->vm_page_prot = pgprot_decrypted(vm_get_page_prot(vma->vm_flags));
  1007			vma->vm_ops = obj->ops->mmap_ops;
  1008			vma->vm_private_data = node->driver_private;
                                               ^^^^^^^^^^^^^^^^^^^^
Patch adds unchecked dereference.

  1009			return 0;
  1010		}

regards,
dan carpenter