Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks

["Xing, Cedric" <cedric.xing@intel.com>]

On 7/8/2019 6:52 PM, Sean Christopherson wrote:
> On Mon, Jul 08, 2019 at 05:02:00PM -0700, Casey Schaufler wrote:
>> On 7/7/2019 6:30 AM, Dr. Greg wrote:
>>> On Wed, Jul 03, 2019 at 08:32:10AM -0700, Casey Schaufler wrote:
>>>
>>> Good morning, I hope the weekend has been enjoyable for everyone.
>>>
>>>>>> On 7/2/2019 12:42 AM, Xing, Cedric wrote:
>>>>>>> ...
>>>>>>> Guess this discussion will never end if we don't get into
>>>>>>> code. Guess it'd be more productive to talk over phone then come back
>>>>>>> to this thread with a conclusion. Will that be ok with you?
>>>>>> I don't think that a phone call is going to help. Talking code
>>>>>> issues tends to muddle them in my brain. If you can give me a few
>>>>>> days I will propose a rough version of how I think your code should
>>>>>> be integrated into the LSM environment. I'm spending more time
>>>>>> trying (unsuccessfully :( ) to discribe the issues in English than
>>>>>> it will probably take in C.
>>>>> While Casey is off writing his rosetta stone,
>>>> I'd hardly call it that. More of an effort to round the corners on
>>>> the square peg. And Cedric has some ideas on how to approach that.
>>> Should we infer from this comment that, of the two competing
>>> strategies, Cedric's is the favored architecture?
>>
>> With Cedric's latest patches I'd say there's only one
>> strategy. There's still some refinement to do, but we're
>> getting there.
> 
> Dynamic tracking has an unsolvable race condition.  If process A maps a
> page W and process B maps the same page X, then the result of W^X checks
> depends on the order of mprotect() calls between A and B.

I don't quite understand where the term "dynamic tracking" came from. 
What's done in the patch is just to track which page was contributed by 
which file. It's been done for all file mappings in Linux.

Back to the "race condition". A similar situation already exists in 
SELinux, between EXECMOD and EXECMEM. Say a file does *not* have EXECMOD 
but the calling process has EXECMEM. Then WX could be granted to a fresh 
private mapping (because of EXECMEM). However, once the mapping has been 
written to, X should have been revoked (because of lack of EXECMOD) but 
will still be retained until dropped by an explicit mprotect(). 
Afterwards, mprotect(X) will be denied. That's not the same situation as 
in this enclave case but they do share one thing in common - X should 
have been revoked from an existing mapping but it isn't, which is just a 
policy choice.

Nothing is unsolvable. Here are 2 options.
(1) We argue that it doesn't matter, similar to the EXECMOD/EXECMEM case 
on regular file mappings described above; or
(2) EXECMOD is required for both W->X and X->W transitions, hence W 
requested by the 2nd process will be denied because X has been granted 
to the 1st process while EXECMOD is absent.

Please note that #2 is effectively the same concept as 
PROCESS2__ENCLAVE_EXECDIRTY in Sean's patch, except that EXECMOD is per 
file while ENCLAVE_EXECDIRTY is per process.