[Intel-gfx] [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap

[Bloomfield, Jon]

> -----Original Message-----
> From: Chris Wilson <chris at chris-wilson.co.uk>
> Sent: Thursday, June 14, 2018 8:00 AM
> To: Bloomfield, Jon <jon.bloomfield at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>; Matthew Auld
> <matthew.william.auld at gmail.com>
> Subject: RE: [PATCH 3/5] drm/i915: Prevent writing into a read-only object via
> a GGTT mmap
> 
> Quoting Bloomfield, Jon (2018-06-14 15:53:13)
> > > -----Original Message-----
> > > From: Chris Wilson <chris at chris-wilson.co.uk>
> > > Sent: Thursday, June 14, 2018 5:00 AM
> > > To: intel-gfx at lists.freedesktop.org
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>; Bloomfield, Jon
> > > <jon.bloomfield at intel.com>; Joonas Lahtinen
> > > <joonas.lahtinen at linux.intel.com>; Matthew Auld
> > > <matthew.william.auld at gmail.com>
> > > Subject: [PATCH 3/5] 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 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!)
> > Are you sure this is necessary? I assumed you would just create a ro IA
> > mapping to the page, irrespective of the ability of ggtt.
> 
> You are thinking of the CPU mmap? The GTT mmap offers a linear view of
> the tiled object. It would be very wrong for us to bypass the PROT_READ
> protection of a user page by accessing it via the GTT.
No, I was thinking of gtt mmap. That requires both GTT and IA PTE mappings
right? One to map the object into the GTT, and then a second to point the
IA at the aperture. Why wouldn't marking the IA mapping RO protect the
object if the GT cannot reach the GTT mapping (from user batches).

> 
> > It feels wrong to
> > disallow mapping a read-only object to the CPU as read-only. With ppgtt
> > the presence of an unprotected mapping in the ggtt should be immune
> > from tampering in the GT, so only the cpu mapping should really matter.
> 
> And the CPU mapping has its protection bits on the IA PTE.
> -Chris