All of
 help / color / mirror / Atom feed
* [PATCH v8] x86/sgx: Maintain encl->refcount for each encl->mm_list entry
@ 2021-02-07 22:14 Jarkko Sakkinen
  2021-02-07 22:37 ` Dave Hansen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-02-07 22:14 UTC (permalink / raw)
  To: linux-sgx
  Cc: dave.hansen, Jarkko Sakkinen, Haitao Huang, Dave Hansen,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Sean Christopherson, Jethro Beekman,

This has been shown in tests:

[  +0.000008] WARNING: CPU: 3 PID: 7620 at kernel/rcu/srcutree.c:374 cleanup_srcu_struct+0xed/0x100

This is essentially a use-after free, although SRCU notices it as
an SRCU cleanup in an invalid context.

== Background ==

SGX has a data structure (struct sgx_encl_mm) which keeps per-mm SGX
metadata.  This is separate from 'struct sgx_encl' because, in theory,
an enclave can be mapped from more than one mm.  sgx_encl_mm includes
a pointer back to the sgx_encl.

This means that sgx_encl must have a longer lifetime than all of the
sgx_encl_mm's that point to it.  That's usually the case: sgx_encl_mm
is freed only after the mmu_notifier is unregistered in sgx_release().

However, there's a race.  If the process is exiting,
sgx_mmu_notifier_release() can be called in parallel with sgx_release()
instead of being called *by* it.  The mmu_notifier path keeps encl_mm
alive past when sgx_encl can be freed.  This inverts the lifetime rules
and means that sgx_mmu_notifier_release() can access a freed sgx_encl.

== Fix ==

Increase encl->refcount when encl_mm->encl is established. Release
this reference encl_mm is freed.  This ensures that 'encl' outlives

Fixes: 1728ab54b4be ("x86/sgx: Add a page reclaimer")
Cc: Dave Hansen <
Reported-by: Haitao Huang <>
Signed-off-by: Jarkko Sakkinen <>
- Slight adjustments on call sites suggested by Dave, to make things
  more clear and obvious. Otherwise, semantically same as v7:
  Contains also long description written by Dave.
- No changes from v6. Resend of
- Maintain refcount for each encl->mm_list entry.
- To make sure that the instance does not get deleted use kref_get()
  kref_put(). This also removes the need for additional
- Rewrite the commit message.
- Just change the call order. *_expedited() is out of scope for this
  bug fix.
v3: Fine-tuned tags, and added missing change log for v2.
v2: Switch to synchronize_srcu_expedited().

 arch/x86/kernel/cpu/sgx/driver.c | 3 +++
 arch/x86/kernel/cpu/sgx/encl.c   | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index f2eac41bb4ff..8ce6d8371cfb 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -72,6 +72,9 @@ static int sgx_release(struct inode *inode, struct file *file)
 		mmu_notifier_unregister(&encl_mm->mmu_notifier, encl_mm->mm);
+		/* 'encl_mm' is gone, put encl_mm->encl reference: */
+		kref_put(&encl->refcount, sgx_encl_release);
 	kref_put(&encl->refcount, sgx_encl_release);
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 20a2dd5ba2b4..7449ef33f081 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -473,6 +473,9 @@ static void sgx_mmu_notifier_free(struct mmu_notifier *mn)
 	struct sgx_encl_mm *encl_mm = container_of(mn, struct sgx_encl_mm, mmu_notifier);
+	/* 'encl_mm' is going away, put encl_mm->encl reference: */
+	kref_put(&encl_mm->encl->refcount, sgx_encl_release);
@@ -526,6 +529,8 @@ int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm)
 	if (!encl_mm)
 		return -ENOMEM;
+	/* Grab a refcount for the encl_mm->encl reference: */
+	kref_get(&encl->refcount);
 	encl_mm->encl = encl;
 	encl_mm->mm = mm;
 	encl_mm->mmu_notifier.ops = &sgx_mmu_notifier_ops;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-14 16:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-07 22:14 [PATCH v8] x86/sgx: Maintain encl->refcount for each encl->mm_list entry Jarkko Sakkinen
2021-02-07 22:37 ` Dave Hansen
2021-02-08 18:20 ` [tip: x86/urgent] " tip-bot2 for Jarkko Sakkinen
2021-04-14  1:15 ` [PATCH v8] " Haitao Huang
2021-04-14 15:51   ` Sean Christopherson
2021-04-14 16:58     ` Dave Hansen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.