| Message ID | 20180531113552.13152-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2018-05-31 at 12:35 +0100, Chris Wilson wrote: > 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 backup in the > fault handler. This is a little draconian as we could blatantly ignore > the write protection on the pages, but it is far simply to keep the > readonly object pure. (It is easier to lift a restriction than to impose > it later!) > > 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> As discussed in IRC, this is no change to previous uAPI as read-only objects have not been available previously. And even afterwards, only GTT_MMAP on RO userptr will be rejected for platforms that don't have RO pages, so that's part of the newly implemented RO userptr. Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 530d6d0109b4..e55278fadf9c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2005,6 +2005,10 @@ int i915_gem_fault(struct vm_fault *vmf) unsigned int flags; int ret; + /* Sanity check that we allow writing into this object */ + if (obj->gt_ro && (write || !ggtt->base.has_read_only)) + 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; @@ -2291,10 +2295,17 @@ i915_gem_mmap_gtt(struct drm_file *file, if (!obj) return -ENOENT; + /* If we will not be able to create the GGTT vma, reject it early. */ + if (obj->gt_ro && !to_i915(dev)->ggtt.base.has_read_only) { + ret = -ENODEV; + goto out; + } + ret = i915_gem_object_create_mmap_offset(obj); if (ret == 0) *offset = drm_vma_node_offset_addr(&obj->base.vma_node); +out: i915_gem_object_put(obj); return ret; }
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 backup in the fault handler. This is a little draconian as we could blatantly ignore the write protection on the pages, but it is far simply to keep the readonly object pure. (It is easier to lift a restriction than to impose it later!) 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> --- drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++ 1 file changed, 11 insertions(+)