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

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: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180712185315.3288-4-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4a16d7b26c89c1a350e5720c46d917f29532e416..bf90625df3c5bf4f34f1a1d187064f46c3599aa1 100644 (file)
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1036,6 +1036,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_put_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_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ed2be33ec58a3540190f679257d703e15d43c297..1910c66f48e28f37545d554d26b76d940031a352 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2012,6 +2012,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
        pgoff_t page_offset;
        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 = (vmf->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 86a9618bab24832fb36bdd862ba9cd51337d660e..3d75f2bb5623390fc42fedb4090f537a03f977cc 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -206,7 +206,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
 
        /* Applicable to VLV, and gen8+ */
        pte_flags = 0;
-       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, cache_level, pte_flags);
@@ -2491,8 +2491,10 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
        const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
        dma_addr_t addr;
 
-       /* The GTT does not support read-only mappings */
-       GEM_BUG_ON(flags & PTE_READ_ONLY);
+       /*
+        * 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;
        gtt_entries += vma->node.start >> PAGE_SHIFT;
@@ -2731,7 +2733,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
 
        /* Applicable to VLV (gen8+ do not support RO in the GGTT) */
        pte_flags = 0;
-       if (obj->gt_ro)
+       if (i915_gem_object_is_readonly(obj))
                pte_flags |= PTE_READ_ONLY;
 
        intel_runtime_pm_get(i915);
@@ -2769,7 +2771,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;
 
        if (flags & I915_VMA_LOCAL_BIND) {

diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index c3c6f2e588fbe12a06fd380492d874377b90007b..56e9f00d2c4c1facd7f182e86f549e5bb16f5dc0 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -141,7 +141,6 @@ struct drm_i915_gem_object {
         * Is the object to be mapped as read-only to the GPU
         * Only honoured if hardware has relevant pte bit
         */
-       unsigned long gt_ro:1;
        unsigned int cache_level:3;
        unsigned int cache_coherent:2;
 #define I915_BO_CACHE_COHERENT_FOR_READ BIT(0)
@@ -358,6 +357,18 @@ static inline void i915_gem_object_unlock(struct drm_i915_gem_object *obj)
        reservation_object_unlock(obj->resv);
 }
 
+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/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6de88add508a4b2a4a6f88b0d0a87244f2598c12..69bd7f697f6daa6b4f50183df60159cbb47daf16 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1100,7 +1100,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_instance(obj, vm, NULL);
        if (IS_ERR(vma))

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 2e2a3c44b8e92b810c8e4f3c0e5c97cefb099e58..1c92560d35da6553cea677aaec9d07ed11333a7e 100644 (file)
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -487,7 +487,8 @@ static int igt_ctx_readonly(void *arg)
                                        goto out_unlock;
                                }
 
-                               obj->gt_ro = prandom_u32_state(&prng);
+                               if (prandom_u32_state(&prng) & 1)
+                                       i915_gem_object_set_readonly(obj);
                        }
 
                        intel_runtime_pm_get(i915);
@@ -518,7 +519,7 @@ static int igt_ctx_readonly(void *arg)
                unsigned int num_writes;
 
                num_writes = rem;
-               if (obj->gt_ro)
+               if (i915_gem_object_is_readonly(obj))
                        num_writes = 0;
 
                err = cpu_check(obj, num_writes);

diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 8758df94e9a0f765831a8cea162e2e824b507abf..c7987daeaed0856542e0be04caea76ff96ddf3ed 100644 (file)
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -41,6 +41,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 {