1:43 p.m.
Abort sgx_pin_mm if mm_users==0 after acquiring mmap_sem to avoid
racing with do_exit during allocation and evictions flows. Remove
the same check from sgx_process_add_page_req as it is now redundant.
Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com>
---
drivers/platform/x86/intel_sgx_ioctl.c | 6 ------
drivers/platform/x86/intel_sgx_util.c | 11 ++++++++++-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/intel_sgx_ioctl.c
b/drivers/platform/x86/intel_sgx_ioctl.c
index 6c8962f..43aaaad 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -262,12 +262,6 @@ static bool sgx_process_add_page_req(struct sgx_add_page_req *req)
if (IS_ERR(backing))
goto out;
- /* Do not race with do_exit() */
- if (!atomic_read(&encl->mm->mm_users)) {
- sgx_put_backing(backing, 0);
- goto out;
- }
-
ret = vm_insert_pfn(vma, encl_page->addr, PFN_DOWN(epc_page->pa));
if (ret)
goto out;
diff --git a/drivers/platform/x86/intel_sgx_util.c
b/drivers/platform/x86/intel_sgx_util.c
index d1c4c71..d076d77 100644
--- a/drivers/platform/x86/intel_sgx_util.c
+++ b/drivers/platform/x86/intel_sgx_util.c
@@ -149,7 +149,16 @@ bool sgx_pin_mm(struct sgx_encl *encl)
down_read(&encl->mm->mmap_sem);
- if (!encl->vma_cnt) {
+ /* Check both vma_cnt and mm_users after acquiring mmap_sem
+ * to avoid racing with the owning process exiting. mm_users
+ * needs to be checked as do_exit->exit_mmap tears down VMAs
+ * and PTEs without holding any MM locks (once mm_users==0).
+ * mm_count only guarantees the MM's kernel objects will not
+ * be freed, it doesn't protect the VMAs or PTEs. Allowing
+ * EPC page eviction to race with the PTEs being dismantled
+ * can result in PTEs being left in use when the MM is freed.
+ */
+ if (!encl->vma_cnt || !atomic_read(&encl->mm->mm_users)) {
sgx_unpin_mm(encl);
return false;
}
--
2.7.4
2:33 p.m.
On 11/14/2016 12:43 PM, Sean Christopherson wrote:
--- a/drivers/platform/x86/intel_sgx_util.c
+++ b/drivers/platform/x86/intel_sgx_util.c
@@ -149,7 +149,16 @@ bool sgx_pin_mm(struct sgx_encl *encl)
down_read(&encl->mm->mmap_sem);
- if (!encl->vma_cnt) {
+ /* Check both vma_cnt and mm_users after acquiring mmap_sem
+ * to avoid racing with the owning process exiting. mm_users
+ * needs to be checked as do_exit->exit_mmap tears down VMAs
+ * and PTEs without holding any MM locks (once mm_users==0).
+ * mm_count only guarantees the MM's kernel objects will not
+ * be freed, it doesn't protect the VMAs or PTEs. Allowing
+ * EPC page eviction to race with the PTEs being dismantled
+ * can result in PTEs being left in use when the MM is freed.
+ */
+ if (!encl->vma_cnt || !atomic_read(&encl->mm->mm_users)) {
sgx_unpin_mm(encl);
return false;
}
Isn't this still fundamentally racy? What prevents someone from doing
an atomic_dec() (and tearing down the mm) immediately after the
atomic_read() check?
3:14 p.m.
Somewhere in the do_exit flow mmap_sem is guaranteed to be acquired for write after
mm_users==0 and before the problematic PTE teardown begins. I don't remember the
exact code off the top of my head but IIRC it was non-obvious. I'll track down the
code and update with details.
-----Original Message-----
From: Hansen, Dave
Sent: Monday, November 14, 2016 1:33 PM
To: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On 11/14/2016 12:43 PM, Sean Christopherson wrote:
--- a/drivers/platform/x86/intel_sgx_util.c
+++ b/drivers/platform/x86/intel_sgx_util.c
@@ -149,7 +149,16 @@ bool sgx_pin_mm(struct sgx_encl *encl)
down_read(&encl->mm->mmap_sem);
- if (!encl->vma_cnt) {
+ /* Check both vma_cnt and mm_users after acquiring mmap_sem
+ * to avoid racing with the owning process exiting. mm_users
+ * needs to be checked as do_exit->exit_mmap tears down VMAs
+ * and PTEs without holding any MM locks (once mm_users==0).
+ * mm_count only guarantees the MM's kernel objects will not
+ * be freed, it doesn't protect the VMAs or PTEs. Allowing
+ * EPC page eviction to race with the PTEs being dismantled
+ * can result in PTEs being left in use when the MM is freed.
+ */
+ if (!encl->vma_cnt || !atomic_read(&encl->mm->mm_users)) {
sgx_unpin_mm(encl);
return false;
}
Isn't this still fundamentally racy? What prevents someone from doing an atomic_dec()
(and tearing down the mm) immediately after the
atomic_read() check?
9:13 a.m.
Apparently the guaranteed down_write was a figment of my imagination. Dave is correct,
this approach is still racy.
I think the correct way to eliminate the race with do_exit is to add a mmu_notifier for
each enclave and mark the enclave as exiting in the mmu_notifier_release callback, with
appropriate locking. The only possible downside of this approach is that it will force
CONFIG_MMU_NOTIFIER when SGX is enabled, but I don't see a way to avoid that
dependency.
-----Original Message-----
From: intel-sgx-kernel-dev [mailto:intel-sgx-kernel-dev-bounces@lists.01.org] On Behalf Of
Christopherson, Sean J
Sent: Monday, November 14, 2016 2:14 PM
To: Hansen, Dave <dave.hansen(a)intel.com>; intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
Somewhere in the do_exit flow mmap_sem is guaranteed to be acquired for write after
mm_users==0 and before the problematic PTE teardown begins. I don't remember the
exact code off the top of my head but IIRC it was non-obvious. I'll track down the
code and update with details.
-----Original Message-----
From: Hansen, Dave
Sent: Monday, November 14, 2016 1:33 PM
To: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On 11/14/2016 12:43 PM, Sean Christopherson wrote:
--- a/drivers/platform/x86/intel_sgx_util.c
+++ b/drivers/platform/x86/intel_sgx_util.c
@@ -149,7 +149,16 @@ bool sgx_pin_mm(struct sgx_encl *encl)
down_read(&encl->mm->mmap_sem);
- if (!encl->vma_cnt) {
+ /* Check both vma_cnt and mm_users after acquiring mmap_sem
+ * to avoid racing with the owning process exiting. mm_users
+ * needs to be checked as do_exit->exit_mmap tears down VMAs
+ * and PTEs without holding any MM locks (once mm_users==0).
+ * mm_count only guarantees the MM's kernel objects will not
+ * be freed, it doesn't protect the VMAs or PTEs. Allowing
+ * EPC page eviction to race with the PTEs being dismantled
+ * can result in PTEs being left in use when the MM is freed.
+ */
+ if (!encl->vma_cnt || !atomic_read(&encl->mm->mm_users)) {
sgx_unpin_mm(encl);
return false;
}
Isn't this still fundamentally racy? What prevents someone from doing an atomic_dec()
(and tearing down the mm) immediately after the
atomic_read() check?
_______________________________________________
intel-sgx-kernel-dev mailing list
intel-sgx-kernel-dev(a)lists.01.org
https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
9:35 a.m.
On 11/15/2016 08:13 AM, Christopherson, Sean J wrote:
I think the correct way to eliminate the race with do_exit is to add
a mmu_notifier for each enclave and mark the enclave as exiting in
the mmu_notifier_release callback, with appropriate locking. The
only possible downside of this approach is that it will force
CONFIG_MMU_NOTIFIER when SGX is enabled, but I don't see a way to
avoid that dependency.
What's the race, again? You hold mmap_sem for read, so the VMAs can't
change. Page table _walks_ can be done without *any* locking in some
circumstances. Modifying page tables themselves is fine under mmap_sem
for read, like MADV_DONTNEED.
I'm not seeing the race.
9:46 a.m.
In the do_exit case, exit_mmap runs without holding mmap_sem. Because the driver does not
increment mm_users, exit_mmap can be invoked via do_exit while the driver is evicting EPC
pages. Ultimately this manifests as a warning in check_mm, e.g. "BUG: non-zero
nr_ptes on freeing mm: 1".
-----Original Message-----
From: Hansen, Dave
Sent: Tuesday, November 15, 2016 8:35 AM
To: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On 11/15/2016 08:13 AM, Christopherson, Sean J wrote:
I think the correct way to eliminate the race with do_exit is to add
a
mmu_notifier for each enclave and mark the enclave as exiting in the
mmu_notifier_release callback, with appropriate locking. The only
possible downside of this approach is that it will force
CONFIG_MMU_NOTIFIER when SGX is enabled, but I don't see a way to
avoid that dependency.
What's the race, again? You hold mmap_sem for read, so the VMAs can't change.
Page table _walks_ can be done without *any* locking in some circumstances. Modifying
page tables themselves is fine under mmap_sem for read, like MADV_DONTNEED.
I'm not seeing the race.
11:02 a.m.
On 11/15/2016 08:46 AM, Christopherson, Sean J wrote:
In the do_exit case, exit_mmap runs without holding mmap_sem.
Because the driver does not increment mm_users, exit_mmap can be
invoked via do_exit while the driver is evicting EPC pages.
Ultimately this manifests as a warning in check_mm, e.g. "BUG:
non-zero nr_ptes on freeing mm: 1".
Is it just pte teardown from mm exit? Or, do you have problems with pte
teardown in general? For instance, what do you do if someone calls
madvise(MADV_DONTNEED) on your SGX area? Will that just leak SGX pages?
11:26 a.m.
The only problematic flow that I am aware of is do_exit. Modifications to the mm via
syscalls, e.g. madvise or munmap, acquire mmap_sem for write and can't run
concurrently with SGX eviction, which acquires mmap_sem for read.
-----Original Message-----
From: Hansen, Dave
Sent: Tuesday, November 15, 2016 10:03 AM
To: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On 11/15/2016 08:46 AM, Christopherson, Sean J wrote:
In the do_exit case, exit_mmap runs without holding mmap_sem.
Because the driver does not increment mm_users, exit_mmap can be
invoked via do_exit while the driver is evicting EPC pages.
Ultimately this manifests as a warning in check_mm, e.g. "BUG:
non-zero nr_ptes on freeing mm: 1".
Is it just pte teardown from mm exit? Or, do you have problems with pte teardown in
general? For instance, what do you do if someone calls
madvise(MADV_DONTNEED) on your SGX area? Will that just leak SGX pages?
11:31 a.m.
On 11/15/2016 10:26 AM, Christopherson, Sean J wrote:
The only problematic flow that I am aware of is do_exit.
Modifications to the mm via syscalls, e.g. madvise or munmap, acquire
mmap_sem for write and can't run concurrently with SGX eviction,
which acquires mmap_sem for read.
Nope. madvise(MADV_DONTNEED) only acquires mmap_sem for read.
11:49 a.m.
Whoops, inverted the return of madvise_need_mmap_write.
I have not tested the driver with madvise(MADV_DONTNEED), but based on the code for
madvise_dontneed() I think that call will always fail due to sgx_mmap setting VM_PFNMAP on
the enclave's VMAs.
-----Original Message-----
From: Hansen, Dave
Sent: Tuesday, November 15, 2016 10:31 AM
To: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On 11/15/2016 10:26 AM, Christopherson, Sean J wrote:
The only problematic flow that I am aware of is do_exit.
Modifications to the mm via syscalls, e.g. madvise or munmap, acquire
mmap_sem for write and can't run concurrently with SGX eviction, which
acquires mmap_sem for read.
Nope. madvise(MADV_DONTNEED) only acquires mmap_sem for read.
12:12 p.m.
On 11/15/2016 10:49 AM, Christopherson, Sean J wrote:
I have not tested the driver with madvise(MADV_DONTNEED), but based
on the code for madvise_dontneed() I think that call will always fail
due to sgx_mmap setting VM_PFNMAP on the enclave's VMAs.
OK, so you're expecting that VM_PFNMAP will continue to ensure that any
teardown of your PTEs will acquire mmap_sem for write? You're expecting
this behavior to continue in perpetuity in the kernel? That seems like
a rather fragile assumption, especially since you already found
do_exit() to be violating this.
I think the mmu notifiers are probably the right way to go, btw...
12:29 p.m.
I'll defer to Jarkko regarding assumptions around VM_PFNMAP and mmap_sem, at this
point I am just trying to resolve a bug I encountered in the current code base and
don't know the full history or design details of the SGX driver.
-----Original Message-----
From: Hansen, Dave
Sent: Tuesday, November 15, 2016 11:12 AM
To: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On 11/15/2016 10:49 AM, Christopherson, Sean J wrote:
I have not tested the driver with madvise(MADV_DONTNEED), but based
on
the code for madvise_dontneed() I think that call will always fail due
to sgx_mmap setting VM_PFNMAP on the enclave's VMAs.
OK, so you're expecting that VM_PFNMAP will continue to ensure that any teardown of
your PTEs will acquire mmap_sem for write? You're expecting this behavior to continue
in perpetuity in the kernel? That seems like a rather fragile assumption, especially
since you already found
do_exit() to be violating this.
I think the mmu notifiers are probably the right way to go, btw...
1:57 p.m.
On Mon, Nov 14, 2016 at 12:43:29PM -0800, Sean Christopherson wrote:
Abort sgx_pin_mm if mm_users==0 after acquiring mmap_sem to avoid
racing with do_exit during allocation and evictions flows. Remove
the same check from sgx_process_add_page_req as it is now redundant.
Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen(a)linux.intel.com>
with some remarks
- I've seen the error message that is emitted by check_mm. I'll append
it to the commit message for clarity.
- Dave is right in that one could still decrement mm_users. However,
this improves the situation a bit since eviction code will also check
mm_users.
- Making this more rare is fine for short term since this is not a fatal
error.
- I replace the comment that you put with something shorter.
/Jarkko
---
drivers/platform/x86/intel_sgx_ioctl.c | 6 ------
drivers/platform/x86/intel_sgx_util.c | 11 ++++++++++-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/intel_sgx_ioctl.c
b/drivers/platform/x86/intel_sgx_ioctl.c
index 6c8962f..43aaaad 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -262,12 +262,6 @@ static bool sgx_process_add_page_req(struct sgx_add_page_req *req)
if (IS_ERR(backing))
goto out;
- /* Do not race with do_exit() */
- if (!atomic_read(&encl->mm->mm_users)) {
- sgx_put_backing(backing, 0);
- goto out;
- }
-
ret = vm_insert_pfn(vma, encl_page->addr, PFN_DOWN(epc_page->pa));
if (ret)
goto out;
diff --git a/drivers/platform/x86/intel_sgx_util.c
b/drivers/platform/x86/intel_sgx_util.c
index d1c4c71..d076d77 100644
--- a/drivers/platform/x86/intel_sgx_util.c
+++ b/drivers/platform/x86/intel_sgx_util.c
@@ -149,7 +149,16 @@ bool sgx_pin_mm(struct sgx_encl *encl)
down_read(&encl->mm->mmap_sem);
- if (!encl->vma_cnt) {
+ /* Check both vma_cnt and mm_users after acquiring mmap_sem
+ * to avoid racing with the owning process exiting. mm_users
+ * needs to be checked as do_exit->exit_mmap tears down VMAs
+ * and PTEs without holding any MM locks (once mm_users==0).
+ * mm_count only guarantees the MM's kernel objects will not
+ * be freed, it doesn't protect the VMAs or PTEs. Allowing
+ * EPC page eviction to race with the PTEs being dismantled
+ * can result in PTEs being left in use when the MM is freed.
+ */
+ if (!encl->vma_cnt || !atomic_read(&encl->mm->mm_users)) {
sgx_unpin_mm(encl);
return false;
}
--
2.7.4
_______________________________________________
intel-sgx-kernel-dev mailing list
intel-sgx-kernel-dev(a)lists.01.org
https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
2:07 p.m.
On 11/15/2016 12:57 PM, Jarkko Sakkinen wrote:
- Dave is right in that one could still decrement mm_users. However,
this improves the situation a bit since eviction code will also check
mm_users.
Could you just make a note that this is something that's got to get
rectified before it gets submitted upstream?
2:10 p.m.
On Tue, Nov 15, 2016 at 01:07:33PM -0800, Dave Hansen wrote:
On 11/15/2016 12:57 PM, Jarkko Sakkinen wrote:
> - Dave is right in that one could still decrement mm_users. However,
> this improves the situation a bit since eviction code will also check
> mm_users.
Could you just make a note that this is something that's got to get
rectified before it gets submitted upstream?
Certainly. I'll add a FIXME comment so it won't go unnoticed. I'm doing
some feature work that uses mmu_notifier. I can cut the right pieces
from that work and create a more proper fix.
/Jarkko
2:16 p.m.
I have a preliminary fix using mmu_notifier and have verified it resolves the warnings.
Do you want me to prep a patch, or would you prefer to handle it on your own so as not to
interfere with the feature work?
-----Original Message-----
From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
Sent: Tuesday, November 15, 2016 1:10 PM
To: Hansen, Dave <dave.hansen(a)intel.com>
Cc: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On Tue, Nov 15, 2016 at 01:07:33PM -0800, Dave Hansen wrote:
On 11/15/2016 12:57 PM, Jarkko Sakkinen wrote:
> - Dave is right in that one could still decrement mm_users. However,
> this improves the situation a bit since eviction code will also check
> mm_users.
Could you just make a note that this is something that's got to get
rectified before it gets submitted upstream?
Certainly. I'll add a FIXME comment so it won't go unnoticed. I'm doing some
feature work that uses mmu_notifier. I can cut the right pieces from that work and create
a more proper fix.
/Jarkko
3:58 p.m.
On Tue, Nov 15, 2016 at 09:16:30PM +0000, Christopherson, Sean J wrote:
I have a preliminary fix using mmu_notifier and have verified it
resolves the warnings. Do you want me to prep a patch, or would you
prefer to handle it on your own so as not to interfere with the
feature work?
You can send it if you want.
/Jarkko
-----Original Message-----
From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
Sent: Tuesday, November 15, 2016 1:10 PM
To: Hansen, Dave <dave.hansen(a)intel.com>
Cc: Christopherson, Sean J <sean.j.christopherson(a)intel.com>;
intel-sgx-kernel-dev(a)lists.01.org
Subject: Re: [intel-sgx-kernel-dev] [PATCH] Check mm_users!=0 when pinning enclave's
MM
On Tue, Nov 15, 2016 at 01:07:33PM -0800, Dave Hansen wrote:
> On 11/15/2016 12:57 PM, Jarkko Sakkinen wrote:
> > - Dave is right in that one could still decrement mm_users. However,
> > this improves the situation a bit since eviction code will also check
> > mm_users.
>
> Could you just make a note that this is something that's got to get
> rectified before it gets submitted upstream?
Certainly. I'll add a FIXME comment so it won't go unnoticed. I'm doing some
feature work that uses mmu_notifier. I can cut the right pieces from that work and create
a more proper fix.
/Jarkko
6:47 p.m.
New subject: munmap is slow in certain cases
My loader works as follows. It allocates an enclave using mmap with
PROT_NONE. Then, as pages are EADDed, it uses mprotect to adjust each
page's permissions to match those in SECINFO (except for TCS pages which
get set to PROT_READ|PROT_WRITE). Later, when destroying the enclave, I
use a single munmap for the whole enclave range. When using the
following enclave, this syscall takes about 25 seconds on my system:
0- 40ffff Reg r-x
410000- 53afff Reg r--
53b000- 5affff Reg rw-
5b0000- 5b1fff Reg rw-
5b2000-45b1fff Reg rw-
45b2000-45c1fff (not mapped)
45c2000-45d1fff Reg rw-
45d2000-45d2fff Reg rw-
45d3000-45d3fff Tcs ---
45d4000-45d5fff Reg rw-
45d6000-45e5fff (not mapped)
45e6000-45f5fff Reg rw-
45f6000-45f6fff Reg rw-
45f7000-45f7fff Tcs ---
45f8000-45f9fff Reg rw-
45fa000-4609fff (not mapped)
460a000-4619fff Reg rw-
461a000-461afff Reg rw-
461b000-461bfff Tcs ---
461c000-461dfff Reg rw-
461e000-462dfff (not mapped)
462e000-463dfff Reg rw-
463e000-463efff Reg rw-
463f000-463ffff Tcs ---
4640000-4641fff Reg rw-
4642000-7ffffff (not mapped)
I'm using this driver:
https://github.com/01org/linux-sgx-driver/tree/sgx_driver_1.7
Jethro Beekman | Fortanix
9:21 a.m.
New subject: munmap is slow in certain cases
Jethro Beekman <jethro(a)fortanix.com> writes:
My loader works as follows. It allocates an enclave using mmap with
PROT_NONE. Then, as pages are EADDed, it uses mprotect to adjust each page's
permissions to match those in SECINFO (except for TCS pages which get set to
PROT_READ|PROT_WRITE). Later, when destroying the enclave, I use a single
munmap for the whole enclave range. When using the following enclave, this
syscall takes about 25 seconds on my system:
0- 40ffff Reg r-x
410000- 53afff Reg r--
53b000- 5affff Reg rw-
5b0000- 5b1fff Reg rw-
5b2000-45b1fff Reg rw-
45b2000-45c1fff (not mapped)
45c2000-45d1fff Reg rw-
45d2000-45d2fff Reg rw-
45d3000-45d3fff Tcs ---
45d4000-45d5fff Reg rw-
45d6000-45e5fff (not mapped)
45e6000-45f5fff Reg rw-
45f6000-45f6fff Reg rw-
45f7000-45f7fff Tcs ---
45f8000-45f9fff Reg rw-
45fa000-4609fff (not mapped)
460a000-4619fff Reg rw-
461a000-461afff Reg rw-
461b000-461bfff Tcs ---
461c000-461dfff Reg rw-
461e000-462dfff (not mapped)
462e000-463dfff Reg rw-
463e000-463efff Reg rw-
463f000-463ffff Tcs ---
4640000-4641fff Reg rw-
4642000-7ffffff (not mapped)
I'm using this driver:
https://github.com/01org/linux-sgx-driver/tree/sgx_driver_1.7
Jethro Beekman | Fortanix
Is there any way you can provide the files needed to reproduce this issue? Reproducing
this issue from scratch is likely going to be quite difficult.
Are there any active threads in the enclave when munmap is called? Does the slowdown
occur if munmap is called immediately after creating the enclave, i.e. before executing
EENTER on any thread? I assume more than just the enclave layout is causing the
slowdown.
Can you try to reproduce the issue using the most recent version of the driver from
http://git.infradead.org/users/jjs/linux-isgx.git?
3:38 p.m.
New subject: munmap is slow in certain cases
On 2017-01-05 08:21, Christopherson, Sean J wrote:
Are there any active threads in the enclave when munmap is called?
Does the slowdown occur if munmap is called immediately after
creating the enclave, i.e. before executing EENTER on any thread?
I assume more than just the enclave layout is causing the slowdown.
I wrote a simple test case that sets up only empty RW pages and fails
EINIT. Even then there is the slowdown on munmap. I can make this test
case publicly available, but see below.
Can you try to reproduce the issue using the most recent version of
the driver from http://git.infradead.org/users/jjs/linux-isgx.git?
I think this particular issue may have been fixed. However, this driver
is very unstable with my actual workload:
[ 1885.331180] INFO: rcu_sched self-detected stall on CPU
[ 1885.331189] 3-...: (5250 ticks this GP) idle=bfd/140000000000001/0
softirq=33251/33251 fqs=2555
[ 1885.331192] (t=5250 jiffies g=25305 c=25304 q=3743)
[ 1885.331196] Task dump for CPU 3:
[ 1885.331198] enclave-runner R running task 0 3199 1
0x0000000c
[ 1885.331203] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
ffff8bf6a14d9c80
[ 1885.331207] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
00000000ffffffff
[ 1885.331211] ffffffff9dce8950 0000000089ac6218 ffff8bf5ad53d700
0000000000000e9f
[ 1885.331214] Call Trace:
[ 1885.331216] <IRQ> [<ffffffff9dd883b4>] ? rcu_dump_cpu_stacks+0x91/0xaa
[ 1885.331227] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
[ 1885.331231] [<ffffffff9dcffae0>] ? tick_sched_handle.isra.13+0x50/0x50
[ 1885.331234] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
[ 1885.331237] [<ffffffff9dcffab0>] ? tick_sched_handle.isra.13+0x20/0x50
[ 1885.331239] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
[ 1885.331243] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
[ 1885.331246] [<ffffffff9dcf7abd>] ?
ktime_get_update_offsets_now+0x4d/0x100
[ 1885.331250] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
[ 1885.331254] [<ffffffff9e342ae9>] ? smp_apic_timer_interrupt+0x39/0x50
[ 1885.331257] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
[ 1885.331258] <EOI> [<ffffffffc0c620a1>] ? sgx_zap_tcs_ptes+0x21/0x60
[intel_sgx]
[ 1885.331270] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50 [intel_sgx]
[ 1885.331274] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0 [intel_sgx]
[ 1885.331278] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
[ 1885.331282] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
[ 1885.331285] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
[ 1885.331288] [<ffffffff9e3402a5>] ? entry_SYSCALL64_slow_path+0x25/0x25
[ 1916.297912] INFO: rcu_sched self-detected stall on CPU
[ 1916.297925] 0-...: (5250 ticks this GP) idle=bd9/140000000000001/0
softirq=40635/40635 fqs=2538
[ 1916.297928] (t=5251 jiffies g=25310 c=25309 q=377)
[ 1916.297932] Task dump for CPU 0:
[ 1916.297934] enclave-runner R running task 0 3199 1
0x0000000c
[ 1916.297939] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
ffff8bf6a1419c80
[ 1916.297943] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
00000000ffffffff
[ 1916.297947] ffffffff9dce8950 0000000089ac69d8 ffff8bf5ad53d700
0000000000000179
[ 1916.297951] Call Trace:
[ 1916.297952] <IRQ> [<ffffffff9dd883b4>] ? rcu_dump_cpu_stacks+0x91/0xaa
[ 1916.297964] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
[ 1916.297968] [<ffffffff9dcffae0>] ? tick_sched_handle.isra.13+0x50/0x50
[ 1916.297972] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
[ 1916.297975] [<ffffffff9dcffab0>] ? tick_sched_handle.isra.13+0x20/0x50
[ 1916.297977] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
[ 1916.297981] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
[ 1916.297988] [<ffffffff9dcf7abd>] ?
ktime_get_update_offsets_now+0x4d/0x100
[ 1916.297992] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
[ 1916.297996] [<ffffffff9e342ae9>] ? smp_apic_timer_interrupt+0x39/0x50
[ 1916.297998] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
[ 1916.297999] <EOI> [<ffffffffc0c620a1>] ? sgx_zap_tcs_ptes+0x21/0x60
[intel_sgx]
[ 1916.298012] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50 [intel_sgx]
[ 1916.298015] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0 [intel_sgx]
[ 1916.298019] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
[ 1916.298023] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
[ 1916.298027] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
[ 1916.298029] [<ffffffff9e3402a5>] ? entry_SYSCALL64_slow_path+0x25/0x25
This is followed within the next minutes by further messages in iwlwifi
and i915 and a system hang. I have unfortunately not been able to
produce a simple test case that reproduces this.
Is there any way you can provide the files needed to reproduce this
issue? Reproducing this issue from scratch is likely going to be
quite difficult.
As I said, I can make available the test case for the slow munmap with
the sgx_driver_1.7. However that may no longer be necessary. See my
off-list message regarding the test case for the stalling with the
latest driver.
Jethro Beekman | Fortanix
4:44 p.m.
New subject: munmap is slow in certain cases
On 2017-01-05 08:21, Christopherson, Sean J wrote:
> Are there any active threads in the enclave when munmap is called?
> Does the slowdown occur if munmap is called immediately after
> creating the enclave, i.e. before executing EENTER on any thread?
> I assume more than just the enclave layout is causing the slowdown.
I wrote a simple test case that sets up only empty RW pages and fails
EINIT. Even then there is the slowdown on munmap. I can make this test
case publicly available, but see below.
> Can you try to reproduce the issue using the most recent version of
> the driver from http://git.infradead.org/users/jjs/linux-isgx.git?
I think this particular issue may have been fixed. However, this driver
is very unstable with my actual workload:
[ 1885.331180] INFO: rcu_sched self-detected stall on CPU
[ 1885.331189] 3-...: (5250 ticks this GP) idle=bfd/140000000000001/0
softirq=33251/33251 fqs=2555
[ 1885.331192] (t=5250 jiffies g=25305 c=25304 q=3743)
[ 1885.331196] Task dump for CPU 3:
[ 1885.331198] enclave-runner R running task 0 3199 1
0x0000000c
[ 1885.331203] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
ffff8bf6a14d9c80
[ 1885.331207] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
00000000ffffffff
[ 1885.331211] ffffffff9dce8950 0000000089ac6218 ffff8bf5ad53d700
0000000000000e9f
[ 1885.331214] Call Trace:
[ 1885.331216] <IRQ> [<ffffffff9dd883b4>] ? rcu_dump_cpu_stacks+0x91/0xaa
[ 1885.331227] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
[ 1885.331231] [<ffffffff9dcffae0>] ? tick_sched_handle.isra.13+0x50/0x50
[ 1885.331234] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
[ 1885.331237] [<ffffffff9dcffab0>] ? tick_sched_handle.isra.13+0x20/0x50
[ 1885.331239] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
[ 1885.331243] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
[ 1885.331246] [<ffffffff9dcf7abd>] ?
ktime_get_update_offsets_now+0x4d/0x100
[ 1885.331250] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
[ 1885.331254] [<ffffffff9e342ae9>] ? smp_apic_timer_interrupt+0x39/0x50
[ 1885.331257] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
[ 1885.331258] <EOI> [<ffffffffc0c620a1>] ? sgx_zap_tcs_ptes+0x21/0x60
[intel_sgx]
[ 1885.331270] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50 [intel_sgx]
[ 1885.331274] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0 [intel_sgx]
[ 1885.331278] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
[ 1885.331282] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
[ 1885.331285] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
[ 1885.331288] [<ffffffff9e3402a5>] ? entry_SYSCALL64_slow_path+0x25/0x25
[ 1916.297912] INFO: rcu_sched self-detected stall on CPU
[ 1916.297925] 0-...: (5250 ticks this GP) idle=bd9/140000000000001/0
softirq=40635/40635 fqs=2538
[ 1916.297928] (t=5251 jiffies g=25310 c=25309 q=377)
[ 1916.297932] Task dump for CPU 0:
[ 1916.297934] enclave-runner R running task 0 3199 1
0x0000000c
[ 1916.297939] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
ffff8bf6a1419c80
[ 1916.297943] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
00000000ffffffff
[ 1916.297947] ffffffff9dce8950 0000000089ac69d8 ffff8bf5ad53d700
0000000000000179
[ 1916.297951] Call Trace:
[ 1916.297952] <IRQ> [<ffffffff9dd883b4>] ? rcu_dump_cpu_stacks+0x91/0xaa
[ 1916.297964] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
[ 1916.297968] [<ffffffff9dcffae0>] ? tick_sched_handle.isra.13+0x50/0x50
[ 1916.297972] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
[ 1916.297975] [<ffffffff9dcffab0>] ? tick_sched_handle.isra.13+0x20/0x50
[ 1916.297977] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
[ 1916.297981] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
[ 1916.297988] [<ffffffff9dcf7abd>] ?
ktime_get_update_offsets_now+0x4d/0x100
[ 1916.297992] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
[ 1916.297996] [<ffffffff9e342ae9>] ? smp_apic_timer_interrupt+0x39/0x50
[ 1916.297998] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
[ 1916.297999] <EOI> [<ffffffffc0c620a1>] ? sgx_zap_tcs_ptes+0x21/0x60
[intel_sgx]
[ 1916.298012] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50 [intel_sgx]
[ 1916.298015] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0 [intel_sgx]
[ 1916.298019] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
[ 1916.298023] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
[ 1916.298027] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
[ 1916.298029] [<ffffffff9e3402a5>] ? entry_SYSCALL64_slow_path+0x25/0x25
This is followed within the next minutes by further messages in iwlwifi
and i915 and a system hang. I have unfortunately not been able to
produce a simple test case that reproduces this.
> Is there any way you can provide the files needed to reproduce this
> issue? Reproducing this issue from scratch is likely going to be
> quite difficult.
As I said, I can make available the test case for the slow munmap with
the sgx_driver_1.7. However that may no longer be necessary. See my
off-list message regarding the test case for the stalling with the
latest driver.
Jethro Beekman | Fortanix
Jarkko, why does the driver invalidate the enclave if the owning process is forked?
Zapping the child's PTEs makes perfect sense, but I don't understand why the
parent's enclave needs to be killed.
5:52 p.m.
New subject: munmap is slow in certain cases
On 2017-01-05 15:44, Christopherson, Sean J wrote:
> I think this particular issue may have been fixed. However, this
driver
> is very unstable with my actual workload:
>
> [ 1885.331180] INFO: rcu_sched self-detected stall on CPU
> [ 1885.331189] 3-...: (5250 ticks this GP) idle=bfd/140000000000001/0
> softirq=33251/33251 fqs=2555
> [ 1885.331192] (t=5250 jiffies g=25305 c=25304 q=3743)
> [ 1885.331196] Task dump for CPU 3:
> [ 1885.331198] enclave-runner R running task 0 3199 1
> 0x0000000c
> [ 1885.331203] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
> ffff8bf6a14d9c80
> [ 1885.331207] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
> 00000000ffffffff
> [ 1885.331211] ffffffff9dce8950 0000000089ac6218 ffff8bf5ad53d700
> 0000000000000e9f
> [ 1885.331214] Call Trace:
> [ 1885.331216] <IRQ> [<ffffffff9dd883b4>] ?
rcu_dump_cpu_stacks+0x91/0xaa
> [ 1885.331227] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
> [ 1885.331231] [<ffffffff9dcffae0>] ? tick_sched_handle.isra.13+0x50/0x50
> [ 1885.331234] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
> [ 1885.331237] [<ffffffff9dcffab0>] ? tick_sched_handle.isra.13+0x20/0x50
> [ 1885.331239] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
> [ 1885.331243] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
> [ 1885.331246] [<ffffffff9dcf7abd>] ?
> ktime_get_update_offsets_now+0x4d/0x100
> [ 1885.331250] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
> [ 1885.331254] [<ffffffff9e342ae9>] ? smp_apic_timer_interrupt+0x39/0x50
> [ 1885.331257] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
> [ 1885.331258] <EOI> [<ffffffffc0c620a1>] ? sgx_zap_tcs_ptes+0x21/0x60
> [intel_sgx]
> [ 1885.331270] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50 [intel_sgx]
> [ 1885.331274] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0 [intel_sgx]
> [ 1885.331278] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
> [ 1885.331282] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
> [ 1885.331285] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
> [ 1885.331288] [<ffffffff9e3402a5>] ? entry_SYSCALL64_slow_path+0x25/0x25
> [ 1916.297912] INFO: rcu_sched self-detected stall on CPU
> [ 1916.297925] 0-...: (5250 ticks this GP) idle=bd9/140000000000001/0
> softirq=40635/40635 fqs=2538
> [ 1916.297928] (t=5251 jiffies g=25310 c=25309 q=377)
> [ 1916.297932] Task dump for CPU 0:
> [ 1916.297934] enclave-runner R running task 0 3199 1
> 0x0000000c
> [ 1916.297939] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
> ffff8bf6a1419c80
> [ 1916.297943] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
> 00000000ffffffff
> [ 1916.297947] ffffffff9dce8950 0000000089ac69d8 ffff8bf5ad53d700
> 0000000000000179
> [ 1916.297951] Call Trace:
> [ 1916.297952] <IRQ> [<ffffffff9dd883b4>] ?
rcu_dump_cpu_stacks+0x91/0xaa
> [ 1916.297964] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
> [ 1916.297968] [<ffffffff9dcffae0>] ? tick_sched_handle.isra.13+0x50/0x50
> [ 1916.297972] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
> [ 1916.297975] [<ffffffff9dcffab0>] ? tick_sched_handle.isra.13+0x20/0x50
> [ 1916.297977] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
> [ 1916.297981] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
> [ 1916.297988] [<ffffffff9dcf7abd>] ?
> ktime_get_update_offsets_now+0x4d/0x100
> [ 1916.297992] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
> [ 1916.297996] [<ffffffff9e342ae9>] ? smp_apic_timer_interrupt+0x39/0x50
> [ 1916.297998] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
> [ 1916.297999] <EOI> [<ffffffffc0c620a1>] ? sgx_zap_tcs_ptes+0x21/0x60
> [intel_sgx]
> [ 1916.298012] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50 [intel_sgx]
> [ 1916.298015] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0 [intel_sgx]
> [ 1916.298019] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
> [ 1916.298023] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
> [ 1916.298027] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
> [ 1916.298029] [<ffffffff9e3402a5>] ? entry_SYSCALL64_slow_path+0x25/0x25
Jarkko, why does the driver invalidate the enclave if the owning process is forked?
Zapping the child's PTEs makes perfect sense, but I don't understand why the
parent's enclave needs to be killed.
Your comment led me to do some more debugging. Forking/threading seems
totally broken in this driver.
1. If I fork from a new thread at any time before loading an enclave
(before or after opening the SGX device), ioctl(EADD) fails with EINVAL.
2. If I fork from a new thread at any time after loading an enclave, I
get SIGBUS in the original process when entering the enclave (or if the
enclave was already running).
3. If the enclave is large (say 64MB mapped pages), in case #2, instead
of SIGBUS, I get the hang described above.
Jethro Beekman | Fortanix
4:32 p.m.
New subject: munmap is slow in certain cases
On Thu, Jan 05, 2017 at 04:52:38PM -0800, Jethro Beekman wrote:
On 2017-01-05 15:44, Christopherson, Sean J wrote:
> > I think this particular issue may have been fixed. However, this driver
> > is very unstable with my actual workload:
> >
> > [ 1885.331180] INFO: rcu_sched self-detected stall on CPU
> > [ 1885.331189] 3-...: (5250 ticks this GP) idle=bfd/140000000000001/0
> > softirq=33251/33251 fqs=2555
> > [ 1885.331192] (t=5250 jiffies g=25305 c=25304 q=3743)
> > [ 1885.331196] Task dump for CPU 3:
> > [ 1885.331198] enclave-runner R running task 0 3199 1
> > 0x0000000c
> > [ 1885.331203] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
> > ffff8bf6a14d9c80
> > [ 1885.331207] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
> > 00000000ffffffff
> > [ 1885.331211] ffffffff9dce8950 0000000089ac6218 ffff8bf5ad53d700
> > 0000000000000e9f
> > [ 1885.331214] Call Trace:
> > [ 1885.331216] <IRQ> [<ffffffff9dd883b4>] ?
rcu_dump_cpu_stacks+0x91/0xaa
> > [ 1885.331227] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
> > [ 1885.331231] [<ffffffff9dcffae0>] ?
tick_sched_handle.isra.13+0x50/0x50
> > [ 1885.331234] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
> > [ 1885.331237] [<ffffffff9dcffab0>] ?
tick_sched_handle.isra.13+0x20/0x50
> > [ 1885.331239] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
> > [ 1885.331243] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
> > [ 1885.331246] [<ffffffff9dcf7abd>] ?
> > ktime_get_update_offsets_now+0x4d/0x100
> > [ 1885.331250] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
> > [ 1885.331254] [<ffffffff9e342ae9>] ?
smp_apic_timer_interrupt+0x39/0x50
> > [ 1885.331257] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
> > [ 1885.331258] <EOI> [<ffffffffc0c620a1>] ?
sgx_zap_tcs_ptes+0x21/0x60
> > [intel_sgx]
> > [ 1885.331270] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50
[intel_sgx]
> > [ 1885.331274] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0
[intel_sgx]
> > [ 1885.331278] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
> > [ 1885.331282] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
> > [ 1885.331285] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
> > [ 1885.331288] [<ffffffff9e3402a5>] ?
entry_SYSCALL64_slow_path+0x25/0x25
> > [ 1916.297912] INFO: rcu_sched self-detected stall on CPU
> > [ 1916.297925] 0-...: (5250 ticks this GP) idle=bd9/140000000000001/0
> > softirq=40635/40635 fqs=2538
> > [ 1916.297928] (t=5251 jiffies g=25310 c=25309 q=377)
> > [ 1916.297932] Task dump for CPU 0:
> > [ 1916.297934] enclave-runner R running task 0 3199 1
> > 0x0000000c
> > [ 1916.297939] ffffffff9e861480 000000002b7723e0 ffffffff9dd883b4
> > ffff8bf6a1419c80
> > [ 1916.297943] ffffffff9e861480 0000000000000000 ffffffff9e97b8c0
> > 00000000ffffffff
> > [ 1916.297947] ffffffff9dce8950 0000000089ac69d8 ffff8bf5ad53d700
> > 0000000000000179
> > [ 1916.297951] Call Trace:
> > [ 1916.297952] <IRQ> [<ffffffff9dd883b4>] ?
rcu_dump_cpu_stacks+0x91/0xaa
> > [ 1916.297964] [<ffffffff9dce8950>] ? rcu_check_callbacks+0x7f0/0x940
> > [ 1916.297968] [<ffffffff9dcffae0>] ?
tick_sched_handle.isra.13+0x50/0x50
> > [ 1916.297972] [<ffffffff9dcef9c8>] ? update_process_times+0x28/0x50
> > [ 1916.297975] [<ffffffff9dcffab0>] ?
tick_sched_handle.isra.13+0x20/0x50
> > [ 1916.297977] [<ffffffff9dcffb18>] ? tick_sched_timer+0x38/0x70
> > [ 1916.297981] [<ffffffff9dcf033a>] ? __hrtimer_run_queues+0xea/0x330
> > [ 1916.297988] [<ffffffff9dcf7abd>] ?
> > ktime_get_update_offsets_now+0x4d/0x100
> > [ 1916.297992] [<ffffffff9dcf0d1c>] ? hrtimer_interrupt+0x9c/0x1a0
> > [ 1916.297996] [<ffffffff9e342ae9>] ?
smp_apic_timer_interrupt+0x39/0x50
> > [ 1916.297998] [<ffffffff9e341e02>] ? apic_timer_interrupt+0x82/0x90
> > [ 1916.297999] <EOI> [<ffffffffc0c620a1>] ?
sgx_zap_tcs_ptes+0x21/0x60
> > [intel_sgx]
> > [ 1916.298012] [<ffffffffc0c621b7>] ? sgx_invalidate+0x27/0x50
[intel_sgx]
> > [ 1916.298015] [<ffffffffc0c62a86>] ? sgx_vma_open+0x36/0xb0
[intel_sgx]
> > [ 1916.298019] [<ffffffff9dc7db38>] ? copy_process.part.31+0xce8/0x1b30
> > [ 1916.298023] [<ffffffff9dc7eb5d>] ? _do_fork+0xed/0x3d0
> > [ 1916.298027] [<ffffffff9dc03b99>] ? do_syscall_64+0x59/0xb0
> > [ 1916.298029] [<ffffffff9e3402a5>] ?
entry_SYSCALL64_slow_path+0x25/0x25
>
> Jarkko, why does the driver invalidate the enclave if the owning process is forked?
Zapping the child's PTEs makes perfect sense, but I don't understand why the
parent's enclave needs to be killed.
Your comment led me to do some more debugging. Forking/threading seems
totally broken in this driver.
1. If I fork from a new thread at any time before loading an enclave (before
or after opening the SGX device), ioctl(EADD) fails with EINVAL.
2. If I fork from a new thread at any time after loading an enclave, I get
SIGBUS in the original process when entering the enclave (or if the enclave
was already running).
3. If the enclave is large (say 64MB mapped pages), in case #2, instead of
SIGBUS, I get the hang described above.
Jethro Beekman | Fortanix
Friday was a holiday in Finland and today I've been struggling to get
through email thread that I started [1]. Once I get the TPM stuff
somewhat under control I'll investigate your responses (which with a quick
grasp somewhat make sense to me).
Some dirty parts are just kind of "emerged" because of historical
reasons. This was an internal driver for a long time and it really
shows in some places although I think it is getting much better now.
I'll have to check interdependencies with the SDK. Naturally SDK should
not dictate how the driver is done but it still makes sense to get a
picture why things are the way things are.
Most of the code is written by me (or rewritten based on the PoC code by
Suresh) but for some parts I've simply forgotten some of the reasoning.
There might have been like 6 months breaks of not touching the code at
all and doing some SGX-unrelated wor while it was still internal.
[1] https://lkml.org/lkml/2017/1/2/234
/Jarkko
4:36 p.m.
New subject: munmap is slow in certain cases
On 01/09/2017 03:32 PM, Jarkko Sakkinen wrote:
Your comment led me to do some more debugging. Forking/threading
seems
totally broken in this driver.
Do you set VM_DONTCOPY on the VMA? That seems like a rather easy way to
sidestep having to handle all the fork() behavior.
1:23 a.m.
New subject: munmap is slow in certain cases
On Mon, Jan 09, 2017 at 03:36:23PM -0800, Dave Hansen wrote:
On 01/09/2017 03:32 PM, Jarkko Sakkinen wrote:
> Your comment led me to do some more debugging. Forking/threading seems
> totally broken in this driver.
Do you set VM_DONTCOPY on the VMA? That seems like a rather easy way to
sidestep having to handle all the fork() behavior.
I did use orinally VM_DONTCOPY flag. Now I have to check why it was
changes but I do remember this was requested by the SDK team. For the
mainline kernel code we can of course ignore the SDK if it technically
does not make sense but I still need to check the reasoning.
/Jarkko
9:17 a.m.
New subject: munmap is slow in certain cases
Jarkko Sakkinen <jarkko.sakkinen(a)linux.intel.com> wrote:
On Mon, Jan 09, 2017 at 03:36:23PM -0800, Dave Hansen wrote:
> On 01/09/2017 03:32 PM, Jarkko Sakkinen wrote:
> > Your comment led me to do some more debugging. Forking/threading
> > seems totally broken in this driver.
>
> Do you set VM_DONTCOPY on the VMA? That seems like a rather easy way
> to sidestep having to handle all the fork() behavior.
I did use orinally VM_DONTCOPY flag. Now I have to check why it was changes
but I do remember this was requested by the SDK team. For the mainline kernel
code we can of course ignore the SDK if it technically does not make sense
but I still need to check the reasoning.
I did a small amount of testing with VM_DONTCOPY + fork() and did not observe any
unexpected behavior. I also found a blurb in the SGX SDK developer's guide which
implies that the SDK expects that the enclave is still usable in the parent process after
fork(). VM_DONTCOPY seems like the correct approach unless there is a technical reason
for effectively disallowing fork.
SDK blurb:
0x0004 SGX_ERROR_ENCLAVE_LOST The enclave is lost after power transition or used in child
process created by fork()
1996
days inactive
2054
days old
intel-sgx-kernel-dev@lists.01.org
25 comments
5 participants
participants (5)
-
Christopherson, Sean J -
Dave Hansen -
Jarkko Sakkinen -
Jethro Beekman -
Sean Christopherson