On Thu, 2019-08-22 at 19:31 +0300, Jarkko Sakkinen wrote: > On Wed, 2019-08-21 at 20:55 -0700, Sean Christopherson wrote: > > Why are we validating the TCS protection bits? Hardware ignores them, so > > why do we care? sgx_ioc_enclave_add_page() sets the internal protection > > bits so there's no danger of putting the wrong thing in the page tables. > > I think that in this commit I got it wrong but I think this is awkward: > > /* > * TCS pages must always RW set for CPU access while the SECINFO > * permissions are *always* zero - the CPU ignores the user provided > * values and silently overwrites with zero permissions. > */ > if ((secinfo.flags & SGX_SECINFO_PAGE_TYPE_MASK) == SGX_SECINFO_TCS) > prot |= PROT_READ | PROT_WRITE; > > In my opinion the right thing to do would be check that SECINFO has *at > minimum* RW and return -EINVAL if not. > > I don't like the SGX silently adjusting permissions like this. For me any sane solution goes where we don't have that kind of tweaking probably my "minimum RW" is more sane than my earlier proposal. /Jarkko
