Commit 2933803b authored by Chris Wilson's avatar Chris Wilson Committed by Jani Nikula

drm/i915/gem: Tighten checks and acquiring the mmap object

Make sure we hold the rcu lock as we acquire the rcu protected reference
of the object when looking it up from the associated mmap vma.

Closes: https://gitlab.freedesktop.org/drm/intel/issues/1083
Fixes: cc662126 ("drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200130143931.1906301-1-chris@chris-wilson.co.uk
(cherry picked from commit 280d14a6)
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent 52144db1
...@@ -807,60 +807,43 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) ...@@ -807,60 +807,43 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
struct drm_vma_offset_node *node; struct drm_vma_offset_node *node;
struct drm_file *priv = filp->private_data; struct drm_file *priv = filp->private_data;
struct drm_device *dev = priv->minor->dev; struct drm_device *dev = priv->minor->dev;
struct drm_i915_gem_object *obj = NULL;
struct i915_mmap_offset *mmo = NULL; struct i915_mmap_offset *mmo = NULL;
struct drm_gem_object *obj = NULL;
struct file *anon; struct file *anon;
if (drm_dev_is_unplugged(dev)) if (drm_dev_is_unplugged(dev))
return -ENODEV; return -ENODEV;
rcu_read_lock();
drm_vma_offset_lock_lookup(dev->vma_offset_manager); drm_vma_offset_lock_lookup(dev->vma_offset_manager);
node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager, node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
vma->vm_pgoff, vma->vm_pgoff,
vma_pages(vma)); vma_pages(vma));
if (likely(node)) { if (node && drm_vma_node_is_allowed(node, priv)) {
mmo = container_of(node, struct i915_mmap_offset,
vma_node);
/*
* In our dependency chain, the drm_vma_offset_node
* depends on the validity of the mmo, which depends on
* the gem object. However the only reference we have
* at this point is the mmo (as the parent of the node).
* Try to check if the gem object was at least cleared.
*/
if (!mmo || !mmo->obj) {
drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
return -EINVAL;
}
/* /*
* Skip 0-refcnted objects as it is in the process of being * Skip 0-refcnted objects as it is in the process of being
* destroyed and will be invalid when the vma manager lock * destroyed and will be invalid when the vma manager lock
* is released. * is released.
*/ */
obj = &mmo->obj->base; mmo = container_of(node, struct i915_mmap_offset, vma_node);
if (!kref_get_unless_zero(&obj->refcount)) obj = i915_gem_object_get_rcu(mmo->obj);
obj = NULL;
} }
drm_vma_offset_unlock_lookup(dev->vma_offset_manager); drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
rcu_read_unlock();
if (!obj) if (!obj)
return -EINVAL; return node ? -EACCES : -EINVAL;
if (!drm_vma_node_is_allowed(node, priv)) {
drm_gem_object_put_unlocked(obj);
return -EACCES;
}
if (i915_gem_object_is_readonly(to_intel_bo(obj))) { if (i915_gem_object_is_readonly(obj)) {
if (vma->vm_flags & VM_WRITE) { if (vma->vm_flags & VM_WRITE) {
drm_gem_object_put_unlocked(obj); i915_gem_object_put(obj);
return -EINVAL; return -EINVAL;
} }
vma->vm_flags &= ~VM_MAYWRITE; vma->vm_flags &= ~VM_MAYWRITE;
} }
anon = mmap_singleton(to_i915(obj->dev)); anon = mmap_singleton(to_i915(dev));
if (IS_ERR(anon)) { if (IS_ERR(anon)) {
drm_gem_object_put_unlocked(obj); i915_gem_object_put(obj);
return PTR_ERR(anon); return PTR_ERR(anon);
} }
......
...@@ -69,6 +69,15 @@ i915_gem_object_lookup_rcu(struct drm_file *file, u32 handle) ...@@ -69,6 +69,15 @@ i915_gem_object_lookup_rcu(struct drm_file *file, u32 handle)
return idr_find(&file->object_idr, handle); return idr_find(&file->object_idr, handle);
} }
static inline struct drm_i915_gem_object *
i915_gem_object_get_rcu(struct drm_i915_gem_object *obj)
{
if (obj && !kref_get_unless_zero(&obj->base.refcount))
obj = NULL;
return obj;
}
static inline struct drm_i915_gem_object * static inline struct drm_i915_gem_object *
i915_gem_object_lookup(struct drm_file *file, u32 handle) i915_gem_object_lookup(struct drm_file *file, u32 handle)
{ {
...@@ -76,8 +85,7 @@ i915_gem_object_lookup(struct drm_file *file, u32 handle) ...@@ -76,8 +85,7 @@ i915_gem_object_lookup(struct drm_file *file, u32 handle)
rcu_read_lock(); rcu_read_lock();
obj = i915_gem_object_lookup_rcu(file, handle); obj = i915_gem_object_lookup_rcu(file, handle);
if (obj && !kref_get_unless_zero(&obj->base.refcount)) obj = i915_gem_object_get_rcu(obj);
obj = NULL;
rcu_read_unlock(); rcu_read_unlock();
return obj; return obj;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment