[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