drm/i915: Prevent writing into a read-only object via a GGTT mmap

commit 3e977ac6179b39faa3c0eda5fce4f00663ae298d upstream.

If the user has created a read-only object, they should not be allowed
to circumvent the write protection by using a GGTT mmapping. Deny it.

Also most machines do not support read-only GGTT PTEs, so again we have
to reject attempted writes. Fortunately, this is known a priori, so we
can at least reject in the call to create the mmap (with a sanity check
in the fault handler).

v2: Check the vma->vm_flags during mmap() to allow readonly access.
v3: Remove VM_MAYWRITE to curtail mprotect()

Testcase: igt/gem_userptr_blits/readonly_mmap*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180712185315.3288-4-chris@chris-wilson.co.uk
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 48e99ab525c363dbbf78f55b540d49d17bbd3769..ae5c0952a7a3b6b84f5283d0514df4bbd5bde08d 100644 (file)
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -996,6 +996,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
                return -EACCES;
        }
 
+       if (node->readonly) {
+               if (vma->vm_flags & VM_WRITE) {
+                       drm_gem_object_unreference_unlocked(obj);
+                       return -EINVAL;
+               }
+
+               vma->vm_flags &= ~VM_MAYWRITE;
+       }
+
        ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
                               vma);
 

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 54b768618beae9f7f36cbcd59290db58739cfae5..eb740997f143a0fbeba5278e809c36b102b84733 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2339,6 +2339,18 @@ i915_gem_object_put_unlocked(struct drm_i915_gem_object *obj)
 __deprecated
 extern void drm_gem_object_unreference_unlocked(struct drm_gem_object *);
 
+static inline void
+i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
+{
+       obj->base.vma_node.readonly = true;
+}
+
+static inline bool
+i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
+{
+       return obj->base.vma_node.readonly;
+}
+
 static inline bool
 i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
 {

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 26c4befcd234252294663c035a1a4fad5ed7d2ac..f1c4e95baa28da6c848cb3293d6652c369f2ee1c 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1773,6 +1773,10 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
        unsigned int flags;
        int ret;
 
+       /* Sanity check that we allow writing into this object */
+       if (i915_gem_object_is_readonly(obj) && write)
+               return VM_FAULT_SIGBUS;
+
        /* We don't use vmf->pgoff since that has the fake offset */
        page_offset = ((unsigned long)vmf->virtual_address - area->vm_start) >>
                PAGE_SHIFT;

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 83c7497e9aa7714043312dbbd5217995bc661037..bfe40d9e5ac98b754ab77e12e6ec416c2e5f6c8d 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -178,7 +178,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
        vma->pages = vma->obj->pages;
 
        /* Applicable to VLV, and gen8+ */
-       if (vma->obj->gt_ro)
+       if (i915_gem_object_is_readonly(vma->obj))
                pte_flags |= PTE_READ_ONLY;
 
        vma->vm->insert_entries(vma->vm, vma->pages, vma->node.start,
@@ -2360,6 +2360,11 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
 
        rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv);
 
+       /*
+        * Note that we ignore PTE_READ_ONLY here. The caller must be careful
+        * not to allow the user to override access to a read only page.
+        */
+
        gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm + (start >> PAGE_SHIFT);
 
        for_each_sgt_dma(addr, sgt_iter, st) {
@@ -2620,7 +2625,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
                return ret;
 
        /* Applicable to VLV (gen8+ do not support RO in the GGTT) */
-       if (obj->gt_ro)
+       if (i915_gem_object_is_readonly(obj))
                pte_flags |= PTE_READ_ONLY;
 
        vma->vm->insert_entries(vma->vm, vma->pages, vma->node.start,
@@ -2649,7 +2654,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
 
        /* Currently applicable only to VLV */
        pte_flags = 0;
-       if (vma->obj->gt_ro)
+       if (i915_gem_object_is_readonly(vma->obj))
                pte_flags |= PTE_READ_ONLY;
 
 

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 418d1d142067a9770f80edc66e485669e40cd9eb..29c3123840aea2fffe3f99a8e025566f8f635c8d 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1966,7 +1966,7 @@ intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
         * if supported by the platform's GGTT.
         */
        if (vm->has_read_only)
-               obj->gt_ro = 1;
+               i915_gem_object_set_readonly(obj);
 
        vma = i915_vma_create(obj, vm, NULL);
        if (IS_ERR(vma))

diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 9c03895dc479dae56b84b8dd2f912bd499709eef..72bf408f887fc9de8e597bad49e766f45b94cd0f 100644 (file)
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -42,6 +42,7 @@ struct drm_vma_offset_node {
        rwlock_t vm_lock;
        struct drm_mm_node vm_node;
        struct rb_root vm_files;
+       bool readonly:1;
 };
 
 struct drm_vma_offset_manager {