Messages in this thread | | | Subject | Re: [PATCH v40 10/24] mm: Add 'mprotect' hook to struct vm_operations_struct | Date | Wed, 18 Nov 2020 19:39:50 -0600 | From | "Haitao Huang" <> |
| |
On Mon, 16 Nov 2020 12:00:23 -0600, Dr. Greg <greg@enjellic.com> 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@intel.com> >> 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
| |