| Message ID | 20190619222401.14942-3-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | security: x86/sgx: SGX vs. LSM | expand |
On Wed, Jun 19, 2019 at 03:23:51PM -0700, Sean Christopherson wrote: > SGX enclaves have an associated Enclave Linear Range (ELRANGE) that is > tracked and enforced by the CPU using a base+mask approach, similar to > how hardware range registers such as the variable MTRRs. As a result, > the ELRANGE must be naturally sized and aligned. > > To reduce boilerplate code that would be needed in every userspace > enclave loader, the SGX driver naturally aligns the mmap() address and > also requires the range to be naturally sized. Unfortunately, SGX fails > to grant a waiver to the MAP_FIXED case, e.g. incorrectly rejects mmap() > if userspace is attempting to map a small slice of an existing enclave. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kernel/cpu/sgx/driver/main.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c > index 07aa5f91b2dd..29384cdd0842 100644 > --- a/arch/x86/kernel/cpu/sgx/driver/main.c > +++ b/arch/x86/kernel/cpu/sgx/driver/main.c > @@ -115,7 +115,13 @@ static unsigned long sgx_get_unmapped_area(struct file *file, > unsigned long pgoff, > unsigned long flags) > { > - if (len < 2 * PAGE_SIZE || len & (len - 1) || flags & MAP_PRIVATE) > + if (flags & MAP_PRIVATE) > + return -EINVAL; > + > + if (flags & MAP_FIXED) > + return addr; > + > + if (len < 2 * PAGE_SIZE || len & (len - 1)) > return -EINVAL; Why the last check is needed given that mmap() no longer does not associate with the layout of the enclave? I'd just wipe it away. /Jarkko
On Fri, Jun 21, 2019 at 12:08:51AM +0300, Jarkko Sakkinen wrote: > On Wed, Jun 19, 2019 at 03:23:51PM -0700, Sean Christopherson wrote: > > SGX enclaves have an associated Enclave Linear Range (ELRANGE) that is > > tracked and enforced by the CPU using a base+mask approach, similar to > > how hardware range registers such as the variable MTRRs. As a result, > > the ELRANGE must be naturally sized and aligned. > > > > To reduce boilerplate code that would be needed in every userspace > > enclave loader, the SGX driver naturally aligns the mmap() address and > > also requires the range to be naturally sized. Unfortunately, SGX fails > > to grant a waiver to the MAP_FIXED case, e.g. incorrectly rejects mmap() > > if userspace is attempting to map a small slice of an existing enclave. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > arch/x86/kernel/cpu/sgx/driver/main.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c > > index 07aa5f91b2dd..29384cdd0842 100644 > > --- a/arch/x86/kernel/cpu/sgx/driver/main.c > > +++ b/arch/x86/kernel/cpu/sgx/driver/main.c > > @@ -115,7 +115,13 @@ static unsigned long sgx_get_unmapped_area(struct file *file, > > unsigned long pgoff, > > unsigned long flags) > > { > > - if (len < 2 * PAGE_SIZE || len & (len - 1) || flags & MAP_PRIVATE) > > + if (flags & MAP_PRIVATE) > > + return -EINVAL; > > + > > + if (flags & MAP_FIXED) > > + return addr; > > + > > + if (len < 2 * PAGE_SIZE || len & (len - 1)) > > return -EINVAL; > > Why the last check is needed given that mmap() no longer does not > associate with the layout of the enclave? I'd just wipe it away. In any case, I squashed this one. /Jarkko
diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c index 07aa5f91b2dd..29384cdd0842 100644 --- a/arch/x86/kernel/cpu/sgx/driver/main.c +++ b/arch/x86/kernel/cpu/sgx/driver/main.c @@ -115,7 +115,13 @@ static unsigned long sgx_get_unmapped_area(struct file *file, unsigned long pgoff, unsigned long flags) { - if (len < 2 * PAGE_SIZE || len & (len - 1) || flags & MAP_PRIVATE) + if (flags & MAP_PRIVATE) + return -EINVAL; + + if (flags & MAP_FIXED) + return addr; + + if (len < 2 * PAGE_SIZE || len & (len - 1)) return -EINVAL; addr = current->mm->get_unmapped_area(file, addr, 2 * len, pgoff,
SGX enclaves have an associated Enclave Linear Range (ELRANGE) that is tracked and enforced by the CPU using a base+mask approach, similar to how hardware range registers such as the variable MTRRs. As a result, the ELRANGE must be naturally sized and aligned. To reduce boilerplate code that would be needed in every userspace enclave loader, the SGX driver naturally aligns the mmap() address and also requires the range to be naturally sized. Unfortunately, SGX fails to grant a waiver to the MAP_FIXED case, e.g. incorrectly rejects mmap() if userspace is attempting to map a small slice of an existing enclave. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kernel/cpu/sgx/driver/main.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)