On Mon, 16 Nov 2020 12:00:23 -0600, Dr. Greg <greg@xxxxxxxxxxxx> wrote:
On Thu, Nov 12, 2020 at 02:41:00PM -0800, Andy Lutomirski wrote:
Good morning, I hope the week is starting well for everyone.
On Thu, Nov 12, 2020 at 1:31 PM Dave Hansen <dave.hansen@xxxxxxxxx>
wrote:
>
> On 11/12/20 12:58 PM, Dr. Greg wrote:
> > @@ -270,11 +270,10 @@ static int sgx_vma_mprotect(struct
vm_area_struct *vma,
> > struct vm_area_struct **pprev, unsigned
long start,
> > unsigned long end, unsigned long newflags)
> > {
> > - int ret;
> > + struct sgx_encl *encl = vma->vm_private_data;
> >
> > - ret = sgx_encl_may_map(vma->vm_private_data, start, end,
newflags);
> > - if (ret)
> > - return ret;
> > + if ( test_bit(SGX_ENCL_INITIALIZED, &encl->flags) )
> > + return -EACCES;
> >
> > return mprotect_fixup(vma, pprev, start, end, newflags);
> > }
>
> This rules out mprotect() on running enclaves. Does that break any
> expectations from enclave authors, or take away capabilities that
folks
> need?
It certainly prevents any scheme in which an enclave coordinates
with the outside world to do W-and-then-X JIT inside. I'm also not
convinced it has any real effect at all unless there's some magic I
missed to prevent someone from using mmap(2) to effectively change
permissions.
The patch that I posted yesterday addresses the security issue for
both mmap and mprotect by trapping the permission change request at
the level of the sgx_encl_may_map() function.
With respect to the W-and-then-X JIT issue, the stated purpose of the
driver is to implement basic SGX functionality, which is SGX1
semantics, it has been stated formally for a year by the developers
themselves that they are not entertaining a driver that addresses any
of the issues associated with non-static memory permissions.
The JIT issue is applicable even to SGX1 platforms. We can do EADD with
EPCM.RWX in sec_info and with PTE.RW, EINIT, then mprotect to set PTE.RX
when JIT is done.
Haitao
