> From: Andy Lutomirski [mailto:luto@xxxxxxxxxx] > Sent: Monday, July 01, 2019 10:58 AM > > On Mon, Jul 1, 2019 at 10:11 AM Xing, Cedric <cedric.xing@xxxxxxxxx> > wrote: > > > > Hi Andy, > > > > > From: Andy Lutomirski [mailto:luto@xxxxxxxxxx] > > > Sent: Saturday, June 29, 2019 4:47 PM > > > > > > Just on a very cursory review, this seems like it's creating a bunch > > > of complexity (a whole new library and data structure), and I'm not > > > convinced the result is any better than Sean's version. > > > > The new EMA data structure is to track enclave pages by range. Yes, > Sean avoided that by storing similar information in the existing > encl_page structure inside SGX subsystem. But as I pointed out, his code > has to iterate through *every* page in range so mprotect() will be very > slow if the range is large. So he would end up introducing something > similar to achieve the same performance. > > It seems odd to stick it in security/ if it only has one user, though. > Also, if it wasn't in security/, then the security folks would stop > complaining :) That's where I started. EMA (though named differently in my v1) was buried inside and used only by SELinux. But Stephen thought it useful for other LSMs as well, as it could be expected that other LSMs would also need to track enclave pages and end up duplicating what's done inside SELinux. I'm ok either way, though I do agree with Stephen's assessment. > > > > > > And that's not the most important point. The major problem in his > patch lies in SGX2 support, as #PF driven EAUG cannot be supported (or > he'd have to amend his code accordingly, which will add complexity and > tip your scale). > > > > Why can't it be? Let me take it back. It's important as it is where LSM folks are divided. I intended to say the major reason I objected Sean's approach was its inability to support SGX2 smoothly - as #PF driven EAUG requires non-existent pages to be mmap()'ed, otherwise vm_ops->fault wouldn't be dispatched so EAUG couldn't be issued in response to #PF.
